Skip to content

Commit 0f56ce0

Browse files
jmorsetstellar
jmorse
authored and
tstellar
committed
[DebugInfo][InstrRef] Avoid a crash from mixed variable location modes
Variable locations now come in two modes, instruction referencing and DBG_VALUE. At -O0 we pick DBG_VALUE to allow fast construction of variable information. Unfortunately, SelectionDAG edits the optimisation level in the presence of opt-bisect-limit, meaning different passes have different views of what variable location mode we should use. That causes assertions when they're mixed. This patch plumbs through a boolean in SelectionDAG from start to instruction emission, so that we don't rely on the current optimisation level for correctness. Differential Revision: https://reviews.llvm.org/D123033 (cherry picked from commit fb6596f)
1 parent e8f03f2 commit 0f56ce0

File tree

11 files changed

+158
-11
lines changed

11 files changed

+158
-11
lines changed

‎llvm/include/llvm/CodeGen/FastISel.h

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -212,6 +212,7 @@ class FastISel {
212212
const TargetRegisterInfo &TRI;
213213
const TargetLibraryInfo *LibInfo;
214214
bool SkipTargetIndependentISel;
215+
bool UseInstrRefDebugInfo = false;
215216

216217
/// The position of the last instruction for materializing constants
217218
/// for use in the current block. It resets to EmitStartPt when it makes sense
@@ -318,6 +319,12 @@ class FastISel {
318319
/// Reset InsertPt to the given old insert position.
319320
void leaveLocalValueArea(SavePoint Old);
320321

322+
/// Signal whether instruction referencing variable locations are desired for
323+
/// this function's debug-info.
324+
void useInstrRefDebugInfo(bool Flag) {
325+
UseInstrRefDebugInfo = Flag;
326+
}
327+
321328
protected:
322329
explicit FastISel(FunctionLoweringInfo &FuncInfo,
323330
const TargetLibraryInfo *LibInfo,

‎llvm/include/llvm/CodeGen/SelectionDAG.h

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -278,6 +278,9 @@ class SelectionDAG {
278278

279279
uint16_t NextPersistentId = 0;
280280

281+
/// Are instruction referencing variable locations desired for this function?
282+
bool UseInstrRefDebugInfo = false;
283+
281284
public:
282285
/// Clients of various APIs that cause global effects on
283286
/// the DAG can optionally implement this interface. This allows the clients
@@ -1702,6 +1705,16 @@ class SelectionDAG {
17021705
/// function mirrors \c llvm::salvageDebugInfo.
17031706
void salvageDebugInfo(SDNode &N);
17041707

1708+
/// Signal whether instruction referencing variable locations are desired for
1709+
/// this function's debug-info.
1710+
void useInstrRefDebugInfo(bool Flag) {
1711+
UseInstrRefDebugInfo = Flag;
1712+
}
1713+
1714+
bool getUseInstrRefDebugInfo() const {
1715+
return UseInstrRefDebugInfo;
1716+
}
1717+
17051718
void dump() const;
17061719

17071720
/// In most cases this function returns the ABI alignment for a given type,

‎llvm/include/llvm/CodeGen/SelectionDAGISel.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,7 @@ class SelectionDAGISel : public MachineFunctionPass {
5353
const TargetLowering *TLI;
5454
bool FastISelFailed;
5555
SmallPtrSet<const Instruction *, 4> ElidedArgCopyInstrs;
56+
bool UseInstrRefDebugInfo = false;
5657

5758
/// Current optimization remark emitter.
5859
/// Used to report things like combines and FastISel failures.

‎llvm/lib/CodeGen/MachineFunction.cpp

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1181,9 +1181,6 @@ void MachineFunction::finalizeDebugInstrRefs() {
11811181
MI.getOperand(1).ChangeToRegister(0, false);
11821182
};
11831183

1184-
if (!useDebugInstrRef())
1185-
return;
1186-
11871184
for (auto &MBB : *this) {
11881185
for (auto &MI : MBB) {
11891186
if (!MI.isDebugRef() || !MI.getOperand(0).isReg())

‎llvm/lib/CodeGen/SelectionDAG/FastISel.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1265,7 +1265,7 @@ bool FastISel::selectIntrinsicCall(const IntrinsicInst *II) {
12651265
// If using instruction referencing, mutate this into a DBG_INSTR_REF,
12661266
// to be later patched up by finalizeDebugInstrRefs. Tack a deref onto
12671267
// the expression, we don't have an "indirect" flag in DBG_INSTR_REF.
1268-
if (FuncInfo.MF->useDebugInstrRef() && Op->isReg()) {
1268+
if (UseInstrRefDebugInfo && Op->isReg()) {
12691269
Builder->setDesc(TII.get(TargetOpcode::DBG_INSTR_REF));
12701270
Builder->getOperand(1).ChangeToImmediate(0);
12711271
auto *NewExpr =
@@ -1324,7 +1324,7 @@ bool FastISel::selectIntrinsicCall(const IntrinsicInst *II) {
13241324

13251325
// If using instruction referencing, mutate this into a DBG_INSTR_REF,
13261326
// to be later patched up by finalizeDebugInstrRefs.
1327-
if (FuncInfo.MF->useDebugInstrRef()) {
1327+
if (UseInstrRefDebugInfo) {
13281328
Builder->setDesc(TII.get(TargetOpcode::DBG_INSTR_REF));
13291329
Builder->getOperand(1).ChangeToImmediate(0);
13301330
}

‎llvm/lib/CodeGen/SelectionDAG/InstrEmitter.cpp

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1341,11 +1341,12 @@ EmitSpecialNode(SDNode *Node, bool IsClone, bool IsCloned,
13411341
/// InstrEmitter - Construct an InstrEmitter and set it to start inserting
13421342
/// at the given position in the given block.
13431343
InstrEmitter::InstrEmitter(const TargetMachine &TM, MachineBasicBlock *mbb,
1344-
MachineBasicBlock::iterator insertpos)
1344+
MachineBasicBlock::iterator insertpos,
1345+
bool UseInstrRefDebugInfo)
13451346
: MF(mbb->getParent()), MRI(&MF->getRegInfo()),
13461347
TII(MF->getSubtarget().getInstrInfo()),
13471348
TRI(MF->getSubtarget().getRegisterInfo()),
13481349
TLI(MF->getSubtarget().getTargetLowering()), MBB(mbb),
13491350
InsertPos(insertpos) {
1350-
EmitDebugInstrRefs = MF->useDebugInstrRef();
1351+
EmitDebugInstrRefs = UseInstrRefDebugInfo;
13511352
}

‎llvm/lib/CodeGen/SelectionDAG/InstrEmitter.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -154,7 +154,8 @@ class LLVM_LIBRARY_VISIBILITY InstrEmitter {
154154
/// InstrEmitter - Construct an InstrEmitter and set it to start inserting
155155
/// at the given position in the given block.
156156
InstrEmitter(const TargetMachine &TM, MachineBasicBlock *mbb,
157-
MachineBasicBlock::iterator insertpos);
157+
MachineBasicBlock::iterator insertpos,
158+
bool UseInstrRefDebugInfo);
158159

159160
private:
160161
void EmitMachineNode(SDNode *Node, bool IsClone, bool IsCloned,

‎llvm/lib/CodeGen/SelectionDAG/ScheduleDAGFast.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -758,7 +758,8 @@ void ScheduleDAGLinearize::Schedule() {
758758

759759
MachineBasicBlock*
760760
ScheduleDAGLinearize::EmitSchedule(MachineBasicBlock::iterator &InsertPos) {
761-
InstrEmitter Emitter(DAG->getTarget(), BB, InsertPos);
761+
InstrEmitter Emitter(DAG->getTarget(), BB, InsertPos,
762+
DAG->getUseInstrRefDebugInfo());
762763
DenseMap<SDValue, Register> VRBaseMap;
763764

764765
LLVM_DEBUG({ dbgs() << "\n*** Final schedule ***\n"; });

‎llvm/lib/CodeGen/SelectionDAG/ScheduleDAGSDNodes.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -843,7 +843,8 @@ EmitPhysRegCopy(SUnit *SU, DenseMap<SUnit*, Register> &VRBaseMap,
843843
/// not necessarily refer to returned BB. The emitter may split blocks.
844844
MachineBasicBlock *ScheduleDAGSDNodes::
845845
EmitSchedule(MachineBasicBlock::iterator &InsertPos) {
846-
InstrEmitter Emitter(DAG->getTarget(), BB, InsertPos);
846+
InstrEmitter Emitter(DAG->getTarget(), BB, InsertPos,
847+
DAG->getUseInstrRefDebugInfo());
847848
DenseMap<SDValue, Register> VRBaseMap;
848849
DenseMap<SUnit*, Register> CopyVRBaseMap;
849850
SmallVector<std::pair<unsigned, MachineInstr*>, 32> Orders;

‎llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -425,6 +425,11 @@ bool SelectionDAGISel::runOnMachineFunction(MachineFunction &mf) {
425425
const Function &Fn = mf.getFunction();
426426
MF = &mf;
427427

428+
// Decide what flavour of variable location debug-info will be used, before
429+
// we change the optimisation level.
430+
UseInstrRefDebugInfo = mf.useDebugInstrRef();
431+
CurDAG->useInstrRefDebugInfo(UseInstrRefDebugInfo);
432+
428433
// Reset the target options before resetting the optimization
429434
// level below.
430435
// FIXME: This is a horrible hack and should be processed via
@@ -654,7 +659,8 @@ bool SelectionDAGISel::runOnMachineFunction(MachineFunction &mf) {
654659

655660
// For debug-info, in instruction referencing mode, we need to perform some
656661
// post-isel maintenence.
657-
MF->finalizeDebugInstrRefs();
662+
if (UseInstrRefDebugInfo)
663+
MF->finalizeDebugInstrRefs();
658664

659665
// Determine if there are any calls in this machine function.
660666
MachineFrameInfo &MFI = MF->getFrameInfo();
@@ -1380,6 +1386,8 @@ void SelectionDAGISel::SelectAllBasicBlocks(const Function &Fn) {
13801386
if (TM.Options.EnableFastISel) {
13811387
LLVM_DEBUG(dbgs() << "Enabling fast-isel\n");
13821388
FastIS = TLI->createFastISel(*FuncInfo, LibInfo);
1389+
if (FastIS)
1390+
FastIS->useInstrRefDebugInfo(UseInstrRefDebugInfo);
13831391
}
13841392

13851393
ReversePostOrderTraversal<const Function*> RPOT(&Fn);

0 commit comments

Comments
 (0)