[PATCH] D25243: [Statepoints] Reuse stack slots more than once within a basic block

[Philip Reames via Phabricator via llvm-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL289509: [Statepoints] Reuse stack slots more than once within a basic block (authored by reames).

Changed prior to commit:
  https://reviews.llvm.org/D25243?vs=73498&id=81166#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D25243

Files:
  llvm/trunk/lib/CodeGen/SelectionDAG/StatepointLowering.cpp
  llvm/trunk/test/CodeGen/X86/statepoint-stack-usage.ll


Index: llvm/trunk/lib/CodeGen/SelectionDAG/StatepointLowering.cpp
===================================================================
--- llvm/trunk/lib/CodeGen/SelectionDAG/StatepointLowering.cpp
+++ llvm/trunk/lib/CodeGen/SelectionDAG/StatepointLowering.cpp
@@ -55,7 +55,8 @@
   NextSlotToAllocate = 0;
   // Need to resize this on each safepoint - we need the two to stay in sync and
   // the clear patterns of a SelectionDAGBuilder have no relation to
-  // FunctionLoweringInfo.  SmallBitVector::reset initializes all bits to false.
+  // FunctionLoweringInfo.  Also need to ensure used bits get cleared.
+  AllocatedStackSlots.clear();
   AllocatedStackSlots.resize(Builder.FuncInfo.StatepointStackSlots.size());
 }
 
@@ -82,16 +83,16 @@
   const size_t NumSlots = AllocatedStackSlots.size();
   assert(NextSlotToAllocate <= NumSlots && "Broken invariant");
 
-  // The stack slots in StatepointStackSlots beyond the first NumSlots were
-  // added in this instance of StatepointLoweringState, and cannot be re-used.
-  assert(NumSlots <= Builder.FuncInfo.StatepointStackSlots.size() &&
+  assert(AllocatedStackSlots.size() ==
+         Builder.FuncInfo.StatepointStackSlots.size() &&
          "Broken invariant");
 
   for (; NextSlotToAllocate < NumSlots; NextSlotToAllocate++) {
     if (!AllocatedStackSlots.test(NextSlotToAllocate)) {
       const int FI = Builder.FuncInfo.StatepointStackSlots[NextSlotToAllocate];
       if (MFI.getObjectSize(FI) == SpillSize) {
         AllocatedStackSlots.set(NextSlotToAllocate);
+        // TODO: Is ValueType the right thing to use here?
         return Builder.DAG.getFrameIndex(FI, ValueType);
       }
     }
@@ -104,6 +105,10 @@
   MFI.markAsStatepointSpillSlotObjectIndex(FI);
 
   Builder.FuncInfo.StatepointStackSlots.push_back(FI);
+  AllocatedStackSlots.resize(AllocatedStackSlots.size()+1, true);
+  assert(AllocatedStackSlots.size() ==
+         Builder.FuncInfo.StatepointStackSlots.size() &&
+         "Broken invariant");
 
   StatepointMaxSlotsRequired = std::max<unsigned long>(
       StatepointMaxSlotsRequired, Builder.FuncInfo.StatepointStackSlots.size());
Index: llvm/trunk/test/CodeGen/X86/statepoint-stack-usage.ll
===================================================================
--- llvm/trunk/test/CodeGen/X86/statepoint-stack-usage.ll
+++ llvm/trunk/test/CodeGen/X86/statepoint-stack-usage.ll
@@ -54,6 +54,36 @@
   ret i32 1
 }
 
+; Check that we reuse the same stack slot across multiple calls.  The use of
+; more than two calls here is critical.  We've had a bug which allowed reuse
+; exactly once which went undetected for a long time.
+define i32 @back_to_back_deopt(i32 %a, i32 %b, i32 %c) #1 
+  gc "statepoint-example" {
+; CHECK-LABEL: back_to_back_deopt
+; The exact stores don't matter, but there need to be three stack slots created
+; CHECK: movl	%ebx, 12(%rsp)
+; CHECK: movl	%ebp, 8(%rsp)
+; CHECK: movl	%r14d, 4(%rsp)
+; CHECK: callq
+; CHECK: movl	%ebx, 12(%rsp)
+; CHECK: movl	%ebp, 8(%rsp)
+; CHECK: movl	%r14d, 4(%rsp)
+; CHECK: callq
+; CHECK: movl	%ebx, 12(%rsp)
+; CHECK: movl	%ebp, 8(%rsp)
+; CHECK: movl	%r14d, 4(%rsp)
+; CHECK: callq
+; CHECK: movl	%ebx, 12(%rsp)
+; CHECK: movl	%ebp, 8(%rsp)
+; CHECK: movl	%r14d, 4(%rsp)
+; CHECK: callq
+  call token (i64, i32, void ()*, i32, i32, ...) @llvm.experimental.gc.statepoint.p0f_isVoidf(i64 0, i32 0, void ()* undef, i32 0, i32 0, i32 0, i32 3, i32 %a, i32 %b, i32 %c)
+call token (i64, i32, void ()*, i32, i32, ...) @llvm.experimental.gc.statepoint.p0f_isVoidf(i64 0, i32 0, void ()* undef, i32 0, i32 0, i32 0, i32 3, i32 %a, i32 %b, i32 %c)
+call token (i64, i32, void ()*, i32, i32, ...) @llvm.experimental.gc.statepoint.p0f_isVoidf(i64 0, i32 0, void ()* undef, i32 0, i32 0, i32 0, i32 3, i32 %a, i32 %b, i32 %c)
+call token (i64, i32, void ()*, i32, i32, ...) @llvm.experimental.gc.statepoint.p0f_isVoidf(i64 0, i32 0, void ()* undef, i32 0, i32 0, i32 0, i32 3, i32 %a, i32 %b, i32 %c)
+  ret i32 1
+}
+
 ; Test that stack slots are reused for invokes
 define i32 @back_to_back_invokes(i32 addrspace(1)* %a, i32 addrspace(1)* %b, i32 addrspace(1)* %c) #1 gc "statepoint-example" personality i32 ()* @"personality_function" {
 ; CHECK-LABEL: back_to_back_invokes


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D25243.81166.patch
Type: text/x-patch
Size: 4227 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20161213/2f0b5278/attachment.bin>