Skip to content

Commit

Permalink
[Statepoints] Use Indirect operands for spill slots
Browse files Browse the repository at this point in the history 
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

llvm-svn: 256352
  • Loading branch information
@preames
preames committed Dec 23, 2015
1 parent 8e7d3b9 commit cb0f947
Show file tree
Hide file tree
Showing 4 changed files with 88 additions and 39 deletions.
23 changes: 22 additions & 1 deletion llvm/include/llvm/CodeGen/MachineFrameInfo.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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.
Expand Down Expand Up @@ -547,6 +555,12 @@ class MachineFrameInfo {
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() &&
Expand All @@ -562,6 +576,13 @@ class MachineFrameInfo {
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,
Expand Down
4 changes: 4 additions & 0 deletions llvm/lib/CodeGen/SelectionDAG/StatepointLowering.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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;
Expand Down
36 changes: 31 additions & 5 deletions llvm/lib/CodeGen/TargetLoweringBase.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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));
Expand All @@ -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;
Expand Down
64 changes: 31 additions & 33 deletions llvm/test/CodeGen/X86/statepoint-stackmap-format.ll
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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)
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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
Expand Down

0 comments on commit cb0f947

Please sign in to comment.