RPCS3/llvm
1
0
Fork 0
mirror of https://github.com/RPCS3/llvm.git synced 2025-03-03 16:18:37 +00:00
Code Issues Actions Packages Projects Releases Wiki Activity

[StatepointLowering] Fix bug in allocateStackSlot

Browse Source 
allocateStackSlot did not consider the size of the value to be spilled
before deciding to re-use a spill slot.  This was originally okay (since
originally we'd only ever spill pointers), but it became not okay when
we changed our scheme to directly spill vectors of pointers.

While this change fixes the bug pointed out, it has two performance
caveats:

 - It matches spill slot and spillee size exactly, while in theory we
   can spill, e.g., an 8 byte pointer into a 16 byte slot.  This is
   slightly complicated to fix since in the stackmaps section, we report
   the size of the spill slot as the size of the "indirect value"; and
   if they're no longer equivalent, we'll have to keep track of the
   (indirect) value size separately from the stack slot size.

 - It will "spuriously run out" of reusable slots, since we now have an
   second check in the search loop in addition to the availablity
   check (e.g. you had two free scalar slots, and you first ask for a
   vector slot followed by a scalar slot).  I'll fix this in a later
   commit.

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@261336 91177308-0d34-0410-b5e6-96231b3b80d8
This commit is contained in:
Sanjoy Das 2016-02-19 17:15:22 +00:00
parent f08995153b
commit fffaef16a2
2 changed files with 58 additions and 2 deletions
lib/CodeGen/SelectionDAG
StatepointLowering.cpp
test/CodeGen/X86
statepoint-vector-bad-spill.ll

21
lib/CodeGen/SelectionDAG/StatepointLowering.cpp

@ -73,6 +73,10 @@ SDValue
StatepointLoweringState::allocateStackSlot(EVT ValueType,
SelectionDAGBuilder &Builder) {
NumSlotsAllocatedForStatepoints++;
auto *MFI = Builder.DAG.getMachineFunction().getFrameInfo();
unsigned SpillSize = ValueType.getSizeInBits() / 8;
assert((SpillSize * 8) == ValueType.getSizeInBits() && "Size not in bytes?");
// First look for a previously created stack slot which is not in
// use (accounting for the fact arbitrary slots may already be
@ -82,7 +86,8 @@ StatepointLoweringState::allocateStackSlot(EVT ValueType,
assert(NextSlotToAllocate <= NumSlots && "Broken invariant");
for (; NextSlotToAllocate < NumSlots; NextSlotToAllocate++) {
if (!AllocatedStackSlots[NextSlotToAllocate]) {
if (!AllocatedStackSlots[NextSlotToAllocate] &&
MFI->getObjectSize(NextSlotToAllocate) == SpillSize) {
const int FI = Builder.FuncInfo.StatepointStackSlots[NextSlotToAllocate];
AllocatedStackSlots[NextSlotToAllocate] = true;
return Builder.DAG.getFrameIndex(FI, ValueType);
@ -96,7 +101,6 @@ StatepointLoweringState::allocateStackSlot(EVT ValueType,
SDValue SpillSlot = Builder.DAG.CreateStackTemporary(ValueType);
const unsigned FI = cast<FrameIndexSDNode>(SpillSlot)->getIndex();
auto *MFI = Builder.DAG.getMachineFunction().getFrameInfo();
MFI->markAsStatepointSpillSlotObjectIndex(FI);
Builder.FuncInfo.StatepointStackSlots.push_back(FI);
@ -424,6 +428,19 @@ spillIncomingStatepointValue(SDValue Incoming, SDValue Chain,
// TODO: We can create TokenFactor node instead of
// chaining stores one after another, this may allow
// a bit more optimal scheduling for them
#ifndef NDEBUG
// Right now we always allocate spill slots that are of the same
// size as the value we're about to spill (the size of spillee can
// vary since we spill vectors of pointers too). At some point we
// can consider allowing spills of smaller values to larger slots
// (i.e. change the '==' in the assert below to a '>=').
auto *MFI = Builder.DAG.getMachineFunction().getFrameInfo();
assert((MFI->getObjectSize(Index) * 8) ==
Incoming.getValueType().getSizeInBits() &&
"Bad spill: stack slot does not match!");
#endif
Chain = Builder.DAG.getStore(Chain, Builder.getCurSDLoc(), Incoming, Loc,
MachinePointerInfo::getFixedStack(
Builder.DAG.getMachineFunction(), Index),

39
test/CodeGen/X86/statepoint-vector-bad-spill.ll Normal file

@ -0,0 +1,39 @@
; RUN: llc -O3 < %s | FileCheck %s
; This is checking for a crash.
target triple = "x86_64-pc-linux-gnu"
define <2 x i8 addrspace(1)*> @test0(i8 addrspace(1)* %el, <2 x i8 addrspace(1)*>* %vec_ptr) gc "statepoint-example" {
; CHECK-LABEL: test0:
entry:
%tok0 = call token (i64, i32, void ()*, i32, i32, ...) @llvm.experimental.gc.statepoint.p0f_isVoidf(i64 0, i32 0, void ()* @do_safepoint, i32 0, i32 0, i32 0, i32 0, i8 addrspace(1)* %el)
%el.relocated = call i8 addrspace(1)* @llvm.experimental.gc.relocate.p1i8(token %tok0, i32 7, i32 7)
%obj.pre = load <2 x i8 addrspace(1)*>, <2 x i8 addrspace(1)*>* %vec_ptr
%obj = insertelement <2 x i8 addrspace(1)*> %obj.pre, i8 addrspace(1)* %el.relocated, i32 0 ; No real objective here, except to use %el
%tok1 = call token (i64, i32, void ()*, i32, i32, ...) @llvm.experimental.gc.statepoint.p0f_isVoidf(i64 0, i32 0, void ()* @do_safepoint, i32 0, i32 0, i32 0, i32 0, <2 x i8 addrspace(1)*> %obj)
%obj.relocated = call <2 x i8 addrspace(1)*> @llvm.experimental.gc.relocate.v2p1i8(token %tok1, i32 7, i32 7)
ret <2 x i8 addrspace(1)*> %obj.relocated
}
define i8 addrspace(1)* @test1(<2 x i8 addrspace(1)*> %obj) gc "statepoint-example" {
; CHECK-LABEL: test1:
entry:
%tok1 = call token (i64, i32, void ()*, i32, i32, ...) @llvm.experimental.gc.statepoint.p0f_isVoidf(i64 0, i32 0, void ()* @do_safepoint, i32 0, i32 0, i32 0, i32 0, <2 x i8 addrspace(1)*> %obj)
%obj.relocated = call <2 x i8 addrspace(1)*> @llvm.experimental.gc.relocate.v2p1i8(token %tok1, i32 7, i32 7)
%el = extractelement <2 x i8 addrspace(1)*> %obj.relocated, i32 0
%tok0 = call token (i64, i32, void ()*, i32, i32, ...) @llvm.experimental.gc.statepoint.p0f_isVoidf(i64 0, i32 0, void ()* @do_safepoint, i32 0, i32 0, i32 0, i32 0, i8 addrspace(1)* %el)
%el.relocated = call i8 addrspace(1)* @llvm.experimental.gc.relocate.p1i8(token %tok0, i32 7, i32 7)
ret i8 addrspace(1)* %el.relocated
}
declare void @do_safepoint()
declare token @llvm.experimental.gc.statepoint.p0f_isVoidf(i64, i32, void ()*, i32, i32, ...)
declare i8 addrspace(1)* @llvm.experimental.gc.relocate.p1i8(token, i32, i32)
declare <2 x i8 addrspace(1)*> @llvm.experimental.gc.relocate.v2p1i8(token, i32, i32)