[Statepoints] Use Indirect operands for spill slots

Teach the statepoint lowering code to emit Indirect stackmap entries for spill inserted by StatepointLowering (i.e. SelectionDAG), but Direct stackmap entries for in-IR allocas which represent manual stack slots. This is what the docs call for (http://llvm.org/docs/StackMaps.html#stack-map-format), but we've been emitting both as Direct. This was pointed out recently on the mailing list as a bug. It also blocks http://reviews.llvm.org/D15632 which extends the lowering to handle vector-of-pointers since only Indirect references can encode a variable sized slot.

To implement this, I introduced a new flag on the StackObject class used to maintian information about stack slots. I original considered (and prototyped in http://reviews.llvm.org/D15632), the idea of using the existing isSpillSlot flag, but end up deciding that was a bit too risky and that the cost of adding a new flag was low. Having the new flag will also allow us - in the future - to emit better comments in verbose assembly which indicate where a particular stack spill around a call comes from. (deopt, gc, regalloc).

Differential Revision: http://reviews.llvm.org/D15759

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@256352 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/include/llvm/CodeGen/MachineFrameInfo.h b/include/llvm/CodeGen/MachineFrameInfo.h
index f5a688458d2e65621aa7b36b34101d70d6ede0bc..48e8ca75052e4ae2598b1b78edf36c3447ee8843 100644 (file)
--- a/include/llvm/CodeGen/MachineFrameInfo.h
+++ b/include/llvm/CodeGen/MachineFrameInfo.h
@@ -101,6 +101,13 @@ class MachineFrameInfo {
     // cannot alias any other memory objects.
     bool isSpillSlot;
 
+    /// If true, this stack slot is used to spill a value (could be deopt
+    /// and/or GC related) over a statepoint. We know that the address of the
+    /// slot can't alias any LLVM IR value.  This is very similiar to a Spill
+    /// Slot, but is created by statepoint lowering is SelectionDAG, not the
+    /// register allocator. 
+    bool isStatepointSpillSlot;
+
     /// If this stack object is originated from an Alloca instruction
     /// this value saves the original IR allocation. Can be NULL.
     const AllocaInst *Alloca;
@@ -118,7 +125,8 @@ class MachineFrameInfo {
     StackObject(uint64_t Sz, unsigned Al, int64_t SP, bool IM,
                 bool isSS, const AllocaInst *Val, bool A)
       : SPOffset(SP), Size(Sz), Alignment(Al), isImmutable(IM),
-        isSpillSlot(isSS), Alloca(Val), PreAllocated(false), isAliased(A) {}
+        isSpillSlot(isSS), isStatepointSpillSlot(false), Alloca(Val),
+        PreAllocated(false), isAliased(A) {}
   };
 
   /// The alignment of the stack.
@@ -547,6 +555,12 @@ public:
     return Objects[ObjectIdx+NumFixedObjects].isSpillSlot;
   }
 
+  bool isStatepointSpillSlotObjectIndex(int ObjectIdx) const {
+    assert(unsigned(ObjectIdx+NumFixedObjects) < Objects.size() &&
+           "Invalid Object Idx!");
+    return Objects[ObjectIdx+NumFixedObjects].isStatepointSpillSlot;
+  }
+
   /// Returns true if the specified index corresponds to a dead object.
   bool isDeadObjectIndex(int ObjectIdx) const {
     assert(unsigned(ObjectIdx+NumFixedObjects) < Objects.size() &&
@@ -562,6 +576,13 @@ public:
     return Objects[ObjectIdx + NumFixedObjects].Size == 0;
   }
 
+  void markAsStatepointSpillSlotObjectIndex(int ObjectIdx) {
+    assert(unsigned(ObjectIdx+NumFixedObjects) < Objects.size() &&
+           "Invalid Object Idx!");
+    Objects[ObjectIdx+NumFixedObjects].isStatepointSpillSlot = true;
+    assert(isStatepointSpillSlotObjectIndex(ObjectIdx) && "inconsistent");
+  }
+
   /// Create a new statically sized stack object, returning
   /// a nonnegative identifier to represent it.
   int CreateStackObject(uint64_t Size, unsigned Alignment, bool isSS,

diff --git a/lib/CodeGen/SelectionDAG/StatepointLowering.cpp b/lib/CodeGen/SelectionDAG/StatepointLowering.cpp
index 87373154c87eca8516ffe5c85b68dd313e82f88d..050ec2116c5d8b2e783fe0200ff271d33c00e9ec 100644 (file)
--- a/lib/CodeGen/SelectionDAG/StatepointLowering.cpp
+++ b/lib/CodeGen/SelectionDAG/StatepointLowering.cpp
@@ -17,6 +17,7 @@
 #include "llvm/ADT/SmallSet.h"
 #include "llvm/ADT/Statistic.h"
 #include "llvm/CodeGen/FunctionLoweringInfo.h"
+#include "llvm/CodeGen/MachineFrameInfo.h"
 #include "llvm/CodeGen/GCMetadata.h"
 #include "llvm/CodeGen/GCStrategy.h"
 #include "llvm/CodeGen/SelectionDAG.h"
@@ -95,6 +96,9 @@ 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);
       AllocatedStackSlots.push_back(true);
       return SpillSlot;

diff --git a/lib/CodeGen/TargetLoweringBase.cpp b/lib/CodeGen/TargetLoweringBase.cpp
index 5422df4f1c081132073a29c2bb65511a57a499ee..36a31c9d6461dc62d4784c6db96cec1ffb86ef8d 100644 (file)
--- a/lib/CodeGen/TargetLoweringBase.cpp
+++ b/lib/CodeGen/TargetLoweringBase.cpp
@@ -1094,6 +1094,19 @@ MachineBasicBlock*
 TargetLoweringBase::emitPatchPoint(MachineInstr *MI,
                                    MachineBasicBlock *MBB) const {
   MachineFunction &MF = *MI->getParent()->getParent();
+  MachineFrameInfo &MFI = *MF.getFrameInfo();
+
+  // We're handling multiple types of operands here:
+  // PATCHPOINT MetaArgs - live-in, read only, direct
+  // STATEPOINT Deopt Spill - live-through, read only, indirect
+  // STATEPOINT Deopt Alloca - live-through, read only, direct
+  // (We're currently conservative and mark the deopt slots read/write in
+  // practice.) 
+  // STATEPOINT GC Spill - live-through, read/write, indirect
+  // STATEPOINT GC Alloca - live-through, read/write, direct
+  // The live-in vs live-through is handled already (the live through ones are
+  // all stack slots), but we need to handle the different type of stackmap
+  // operands and memory effects here.
 
   // MI changes inside this loop as we grow operands.
   for(unsigned OperIdx = 0; OperIdx != MI->getNumOperands(); ++OperIdx) {
@@ -1109,10 +1122,24 @@ TargetLoweringBase::emitPatchPoint(MachineInstr *MI,
     // Copy operands before the frame-index.
     for (unsigned i = 0; i < OperIdx; ++i)
       MIB.addOperand(MI->getOperand(i));
-    // Add frame index operands: direct-mem-ref tag, #FI, offset.
-    MIB.addImm(StackMaps::DirectMemRefOp);
-    MIB.addOperand(MI->getOperand(OperIdx));
-    MIB.addImm(0);
+    // Add frame index operands recognized by stackmaps.cpp
+    if (MFI.isStatepointSpillSlotObjectIndex(FI)) {
+      // indirect-mem-ref tag, size, #FI, offset.
+      // Used for spills inserted by StatepointLowering.  This codepath is not
+      // used for patchpoints/stackmaps at all, for these spilling is done via
+      // foldMemoryOperand callback only.
+      assert(MI->getOpcode() == TargetOpcode::STATEPOINT && "sanity");
+      MIB.addImm(StackMaps::IndirectMemRefOp);
+      MIB.addImm(MFI.getObjectSize(FI));
+      MIB.addOperand(MI->getOperand(OperIdx));
+      MIB.addImm(0);
+    } else {
+      // direct-mem-ref tag, #FI, offset.
+      // Used by patchpoint, and direct alloca arguments to statepoints
+      MIB.addImm(StackMaps::DirectMemRefOp);
+      MIB.addOperand(MI->getOperand(OperIdx));
+      MIB.addImm(0);
+    }
     // Copy the operands after the frame index.
     for (unsigned i = OperIdx + 1; i != MI->getNumOperands(); ++i)
       MIB.addOperand(MI->getOperand(i));
@@ -1122,7 +1149,6 @@ TargetLoweringBase::emitPatchPoint(MachineInstr *MI,
     assert(MIB->mayLoad() && "Folded a stackmap use to a non-load!");
 
     // Add a new memory operand for this FI.
-    const MachineFrameInfo &MFI = *MF.getFrameInfo();
     assert(MFI.getObjectOffset(FI) != -1);
 
     unsigned Flags = MachineMemOperand::MOLoad;

diff --git a/test/CodeGen/X86/statepoint-stackmap-format.ll b/test/CodeGen/X86/statepoint-stackmap-format.ll
index 2b7f077a4b293c8de526bb335ada3ca5653d77fc..5c9f54f89ff0e4dfa15060b6e9782b6d382d5b86 100644 (file)
--- a/test/CodeGen/X86/statepoint-stackmap-format.ll
+++ b/test/CodeGen/X86/statepoint-stackmap-format.ll
@@ -11,7 +11,7 @@ declare zeroext i1 @return_i1()
 
 define i1 @test(i32 addrspace(1)* %ptr_base, i32 %arg)
   gc "statepoint-example" {
-; CHECK-LABEL: test
+; CHECK-LABEL: test:
 ; Do we see two spills for the local values and the store to the
 ; alloca?
 ; CHECK: subq  $40, %rsp
@@ -94,18 +94,19 @@ declare i32 addrspace(1)* @llvm.experimental.gc.relocate.p1i32(i32, i32, i32) #3
 ; CHECK-NEXT:   .quad 40
 ; CHECK-NEXT:   .quad test_derived_arg
 ; CHECK-NEXT:   .quad 40
+; CHECK-NEXT:   .quad test_id
+; CHECK-NEXT:   .quad 8
 
 ;
 ; test
 ;
 
-; Large Constants
-; Statepoint ID only
-; CHECK: .quad 0
+; Statepoint ID
+; CHECK-NEXT: .quad    0
 
 ; Callsites
 ; Constant arguments
-; CHECK: .long .Ltmp1-test
+; CHECK-NEXT: .long    .Ltmp1-test
 ; CHECK: .short        0
 ; CHECK: .short        11
 ; SmallConstant (0)
@@ -123,8 +124,8 @@ declare i32 addrspace(1)* @llvm.experimental.gc.relocate.p1i32(i32, i32, i32) #3
 ; CHECK: .byte 8
 ; CHECK: .short        0
 ; CHECK: .long 2
-; Direct Spill Slot [RSP+0]
-; CHECK: .byte 2
+; Indirect Spill Slot [RSP+0]
+; CHECK: .byte 3
 ; CHECK: .byte 8
 ; CHECK: .short        7
 ; CHECK: .long 16
@@ -143,23 +144,23 @@ declare i32 addrspace(1)* @llvm.experimental.gc.relocate.p1i32(i32, i32, i32) #3
 ; CHECK: .byte 8
 ; CHECK: .short        0
 ; CHECK: .long 0
-; Direct Spill Slot [RSP+16]
-; CHECK: .byte 2
+; Indirect Spill Slot [RSP+16]
+; CHECK: .byte 3
 ; CHECK: .byte 8
 ; CHECK: .short        7
 ; CHECK: .long 16
-; Direct Spill Slot [RSP+8]
-; CHECK: .byte 2
+; Indirect Spill Slot [RSP+8]
+; CHECK: .byte 3
 ; CHECK: .byte 8
 ; CHECK: .short        7
 ; CHECK: .long 8
-; Direct Spill Slot [RSP+16]
-; CHECK: .byte 2
+; Indirect Spill Slot [RSP+16]
+; CHECK: .byte 3
 ; CHECK: .byte 8
 ; CHECK: .short        7
 ; CHECK: .long 16
-; Direct Spill Slot [RSP+16]
-; CHECK: .byte 2
+; Indirect Spill Slot [RSP+16]
+; CHECK: .byte 3
 ; CHECK: .byte 8
 ; CHECK: .short        7
 ; CHECK: .long 16
@@ -171,15 +172,13 @@ declare i32 addrspace(1)* @llvm.experimental.gc.relocate.p1i32(i32, i32, i32) #3
 
 ;
 ; test_derived_arg
-;
 
-; Large Constants
-; Statepoint ID only
-; CHECK: .quad 0
+; Statepoint ID
+; CHECK-NEXT: .quad    0
 
 ; Callsites
 ; Constant arguments
-; CHECK: .long .Ltmp3-test_derived_arg
+; CHECK-NEXT: .long    .Ltmp3-test_derived_arg
 ; CHECK: .short        0
 ; CHECK: .short        11
 ; SmallConstant (0)
@@ -192,8 +191,8 @@ declare i32 addrspace(1)* @llvm.experimental.gc.relocate.p1i32(i32, i32, i32) #3
 ; CHECK: .byte 8
 ; CHECK: .short        0
 ; CHECK: .long 2
-; Direct Spill Slot [RSP+0]
-; CHECK: .byte 2
+; Indirect Spill Slot [RSP+0]
+; CHECK: .byte 3
 ; CHECK: .byte 8
 ; CHECK: .short        7
 ; CHECK: .long 16
@@ -212,23 +211,23 @@ declare i32 addrspace(1)* @llvm.experimental.gc.relocate.p1i32(i32, i32, i32) #3
 ; CHECK: .byte 8
 ; CHECK: .short        0
 ; CHECK: .long 0
-; Direct Spill Slot [RSP+16]
-; CHECK: .byte 2
+; Indirect Spill Slot [RSP+16]
+; CHECK: .byte 3
 ; CHECK: .byte 8
 ; CHECK: .short        7
 ; CHECK: .long 16
-; Direct Spill Slot [RSP+8]
-; CHECK: .byte 2
+; Indirect Spill Slot [RSP+8]
+; CHECK: .byte 3
 ; CHECK: .byte 8
 ; CHECK: .short        7
 ; CHECK: .long 8
-; Direct Spill Slot [RSP+16]
-; CHECK: .byte 2
+; Indirect Spill Slot [RSP+16]
+; CHECK: .byte 3
 ; CHECK: .byte 8
 ; CHECK: .short        7
 ; CHECK: .long 16
-; Direct Spill Slot [RSP+16]
-; CHECK: .byte 2
+; Indirect Spill Slot [RSP+16]
+; CHECK: .byte 3
 ; CHECK: .byte 8
 ; CHECK: .short        7
 ; CHECK: .long 16
@@ -239,13 +238,12 @@ declare i32 addrspace(1)* @llvm.experimental.gc.relocate.p1i32(i32, i32, i32) #3
 ; CHECK: .align        8
 
 ; Records for the test_id function:
-; No large constants
 
 ; The Statepoint ID:
-; CHECK: .quad 237
+; CHECK-NEXT: .quad    237
 
 ; Instruction Offset
-; CHECK: .long .Ltmp5-test_id
+; CHECK-NEXT: .long    .Ltmp5-test_id
 
 ; Reserved:
 ; CHECK: .short        0