From da801219ba8d6d2a8663d4dd3c14e8e3fca35ba5 Mon Sep 17 00:00:00 2001 From: Philip Reames Date: Wed, 23 Dec 2015 23:44:28 +0000 Subject: [PATCH] [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 --- include/llvm/CodeGen/MachineFrameInfo.h | 23 ++++++- .../SelectionDAG/StatepointLowering.cpp | 4 ++ lib/CodeGen/TargetLoweringBase.cpp | 36 +++++++++-- .../CodeGen/X86/statepoint-stackmap-format.ll | 64 +++++++++---------- 4 files changed, 88 insertions(+), 39 deletions(-) diff --git a/include/llvm/CodeGen/MachineFrameInfo.h b/include/llvm/CodeGen/MachineFrameInfo.h index f5a688458d2..48e8ca75052 100644 --- 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 87373154c87..050ec2116c5 100644 --- 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(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 5422df4f1c0..36a31c9d646 100644 --- 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 2b7f077a4b2..5c9f54f89ff 100644 --- 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 -- 2.34.1