[llvm] r261336 - [StatepointLowering] Fix bug in allocateStackSlot
Sanjoy Das via llvm-commits
llvm-commits at lists.llvm.org
Fri Feb 19 09:15:23 PST 2016
Author: sanjoy
Date: Fri Feb 19 11:15:22 2016
New Revision: 261336
URL: http://llvm.org/viewvc/llvm-project?rev=261336&view=rev
Log:
[StatepointLowering] Fix bug in allocateStackSlot
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.
Added:
llvm/trunk/test/CodeGen/X86/statepoint-vector-bad-spill.ll
Modified:
llvm/trunk/lib/CodeGen/SelectionDAG/StatepointLowering.cpp
Modified: llvm/trunk/lib/CodeGen/SelectionDAG/StatepointLowering.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/SelectionDAG/StatepointLowering.cpp?rev=261336&r1=261335&r2=261336&view=diff
==============================================================================
--- llvm/trunk/lib/CodeGen/SelectionDAG/StatepointLowering.cpp (original)
+++ llvm/trunk/lib/CodeGen/SelectionDAG/StatepointLowering.cpp Fri Feb 19 11:15:22 2016
@@ -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::allocateStackSl
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::allocateStackSl
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 Inc
// 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),
Added: llvm/trunk/test/CodeGen/X86/statepoint-vector-bad-spill.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/X86/statepoint-vector-bad-spill.ll?rev=261336&view=auto
==============================================================================
--- llvm/trunk/test/CodeGen/X86/statepoint-vector-bad-spill.ll (added)
+++ llvm/trunk/test/CodeGen/X86/statepoint-vector-bad-spill.ll Fri Feb 19 11:15:22 2016
@@ -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)
More information about the llvm-commits
mailing list