From d7cf7abb78635f67d7e3225e393ade9bf2faa523 Mon Sep 17 00:00:00 2001 From: Jeremy Morse Date: Wed, 2 Jun 2021 15:58:03 +0100 Subject: [PATCH] [DebugInfo][InstrRef][4/4] Support DBG_INSTR_REF through all backend passes This is a cleanup patch -- we're now able to support all flavours of variable location in instruction referencing mode. This patch updates various tests for debug instructions to be broader: numerous code paths try to ignore debug isntructions, and they now have to ignore the additional DBG_PHI and DBG_INSTR_REFs that we can generate. A small amount of rework happens for LiveDebugVariables: as we don't need to track live intervals through regalloc any more, we can get away with unlinking debug instructions before regalloc, then re-inserting them after. Note that this isn't (yet) true of DBG_VALUE_LISTs, they still have to go through live interval tracking. In SelectionDAG, add a helper lambda that emits half-formed DBG_INSTR_REFs for arguments in instr-ref mode, DBG_VALUE otherwise. This is one of the final locations where DBG_VALUEs are emitted for vreg arguments. X86InstrInfo now un-sets the debug instr number on SUB instructions that get mutated into CMP instructions. As the instruction no longer computes a subtraction, we can't use it for variable locations. Differential Revision: https://reviews.llvm.org/D88898 --- lib/CodeGen/LiveDebugVariables.cpp | 157 ++++++++++++------ lib/CodeGen/LiveIntervals.cpp | 2 +- lib/CodeGen/MachineSink.cpp | 2 +- lib/CodeGen/RegisterCoalescer.cpp | 12 +- lib/CodeGen/RegisterPressure.cpp | 2 +- lib/CodeGen/ScheduleDAGInstrs.cpp | 2 +- .../SelectionDAG/SelectionDAGBuilder.cpp | 50 ++++-- lib/Target/X86/X86InstrInfo.cpp | 2 + .../MIR/InstrRef/phi-coalesce-subreg.mir | 10 +- .../DebugInfo/MIR/InstrRef/phi-coalescing.mir | 7 +- .../MIR/InstrRef/phi-regallocd-to-stack.mir | 4 - .../MIR/InstrRef/phi-through-regalloc.mir | 3 - .../MIR/InstrRef/x86-drop-compare-inst.mir | 95 +++++++++++ 13 files changed, 257 insertions(+), 91 deletions(-) create mode 100644 test/DebugInfo/MIR/InstrRef/x86-drop-compare-inst.mir diff --git a/lib/CodeGen/LiveDebugVariables.cpp b/lib/CodeGen/LiveDebugVariables.cpp index 0244f452d64..54058a54792 100644 --- a/lib/CodeGen/LiveDebugVariables.cpp +++ b/lib/CodeGen/LiveDebugVariables.cpp @@ -38,9 +38,11 @@ #include "llvm/CodeGen/MachineInstrBuilder.h" #include "llvm/CodeGen/MachineOperand.h" #include "llvm/CodeGen/MachineRegisterInfo.h" +#include "llvm/CodeGen/Passes.h" #include "llvm/CodeGen/SlotIndexes.h" #include "llvm/CodeGen/TargetInstrInfo.h" #include "llvm/CodeGen/TargetOpcodes.h" +#include "llvm/CodeGen/TargetPassConfig.h" #include "llvm/CodeGen/TargetRegisterInfo.h" #include "llvm/CodeGen/TargetSubtargetInfo.h" #include "llvm/CodeGen/VirtRegMap.h" @@ -56,6 +58,7 @@ #include "llvm/Support/CommandLine.h" #include "llvm/Support/Debug.h" #include "llvm/Support/raw_ostream.h" +#include "llvm/Target/TargetMachine.h" #include #include #include @@ -535,10 +538,6 @@ class LDVImpl { LiveIntervals *LIS; const TargetRegisterInfo *TRI; - using StashedInstrRef = - std::tuple; - /// Position and VReg of a PHI instruction during register allocation. struct PHIValPos { SlotIndex SI; /// Slot where this PHI occurs. @@ -553,7 +552,17 @@ class LDVImpl { /// at different positions. DenseMap> RegToPHIIdx; - std::map> StashedInstrReferences; + /// Record for any debug instructions unlinked from their blocks during + /// regalloc. Stores the instr and it's location, so that they can be + /// re-inserted after regalloc is over. + struct InstrPos { + MachineInstr *MI; ///< Debug instruction, unlinked from it's block. + SlotIndex Idx; ///< Slot position where MI should be re-inserted. + MachineBasicBlock *MBB; ///< Block that MI was in. + }; + + /// Collection of stored debug instructions, preserved until after regalloc. + SmallVector StashedDebugInstrs; /// Whether emitDebugValues is called. bool EmitDone = false; @@ -591,15 +600,18 @@ class LDVImpl { /// \returns True if the DBG_VALUE instruction should be deleted. bool handleDebugValue(MachineInstr &MI, SlotIndex Idx); - /// Track a DBG_INSTR_REF. This needs to be removed from the MachineFunction - /// during regalloc -- but there's no need to maintain live ranges, as we - /// refer to a value rather than a location. + /// Track variable location debug instructions while using the instruction + /// referencing implementation. Such debug instructions do not need to be + /// updated during regalloc because they identify instructions rather than + /// register locations. However, they needs to be removed from the + /// MachineFunction during regalloc, then re-inserted later, to avoid + /// disrupting the allocator. /// - /// \param MI DBG_INSTR_REF instruction + /// \param MI Any DBG_VALUE / DBG_INSTR_REF / DBG_PHI instruction /// \param Idx Last valid SlotIndex before instruction /// - /// \returns True if the DBG_VALUE instruction should be deleted. - bool handleDebugInstrRef(MachineInstr &MI, SlotIndex Idx); + /// \returns Iterator to continue processing from after unlinking. + MachineBasicBlock::iterator handleDebugInstr(MachineInstr &MI, SlotIndex Idx); /// Add DBG_LABEL instruction to UserLabel. /// @@ -613,9 +625,11 @@ class LDVImpl { /// for each instruction. /// /// \param mf MachineFunction to be scanned. + /// \param InstrRef Whether to operate in instruction referencing mode. If + /// true, most of LiveDebugVariables doesn't run. /// /// \returns True if any debug values were found. - bool collectDebugValues(MachineFunction &mf); + bool collectDebugValues(MachineFunction &mf, bool InstrRef); /// Compute the live intervals of all user values after collecting all /// their def points. @@ -624,14 +638,14 @@ class LDVImpl { public: LDVImpl(LiveDebugVariables *ps) : pass(*ps) {} - bool runOnMachineFunction(MachineFunction &mf); + bool runOnMachineFunction(MachineFunction &mf, bool InstrRef); /// Release all memory. void clear() { MF = nullptr; PHIValToPos.clear(); RegToPHIIdx.clear(); - StashedInstrReferences.clear(); + StashedDebugInstrs.clear(); userValues.clear(); userLabels.clear(); virtRegToEqClass.clear(); @@ -864,17 +878,21 @@ bool LDVImpl::handleDebugValue(MachineInstr &MI, SlotIndex Idx) { return true; } -bool LDVImpl::handleDebugInstrRef(MachineInstr &MI, SlotIndex Idx) { - assert(MI.isDebugRef()); - unsigned InstrNum = MI.getOperand(0).getImm(); - unsigned OperandNum = MI.getOperand(1).getImm(); - auto *Var = MI.getDebugVariable(); - auto *Expr = MI.getDebugExpression(); - auto &DL = MI.getDebugLoc(); - StashedInstrRef Stashed = - std::make_tuple(InstrNum, OperandNum, Var, Expr, DL); - StashedInstrReferences[Idx].push_back(Stashed); - return true; +MachineBasicBlock::iterator LDVImpl::handleDebugInstr(MachineInstr &MI, + SlotIndex Idx) { + assert(MI.isDebugValue() || MI.isDebugRef() || MI.isDebugPHI()); + + // In instruction referencing mode, there should be no DBG_VALUE instructions + // that refer to virtual registers. They might still refer to constants. + if (MI.isDebugValue()) + assert(!MI.getOperand(0).isReg() || !MI.getOperand(0).getReg().isVirtual()); + + // Unlink the instruction, store it in the debug instructions collection. + auto NextInst = std::next(MI.getIterator()); + auto *MBB = MI.getParent(); + MI.removeFromParent(); + StashedDebugInstrs.push_back({&MI, Idx, MBB}); + return NextInst; } bool LDVImpl::handleDebugLabel(MachineInstr &MI, SlotIndex Idx) { @@ -900,7 +918,7 @@ bool LDVImpl::handleDebugLabel(MachineInstr &MI, SlotIndex Idx) { return true; } -bool LDVImpl::collectDebugValues(MachineFunction &mf) { +bool LDVImpl::collectDebugValues(MachineFunction &mf, bool InstrRef) { bool Changed = false; for (MachineBasicBlock &MBB : mf) { for (MachineBasicBlock::iterator MBBI = MBB.begin(), MBBE = MBB.end(); @@ -919,11 +937,17 @@ bool LDVImpl::collectDebugValues(MachineFunction &mf) { : LIS->getInstructionIndex(*std::prev(MBBI)).getRegSlot(); // Handle consecutive debug instructions with the same slot index. do { - // Only handle DBG_VALUE in handleDebugValue(). Skip all other - // kinds of debug instructions. - if ((MBBI->isDebugValue() && handleDebugValue(*MBBI, Idx)) || - (MBBI->isDebugRef() && handleDebugInstrRef(*MBBI, Idx)) || - (MBBI->isDebugLabel() && handleDebugLabel(*MBBI, Idx))) { + // In instruction referencing mode, pass each instr to handleDebugInstr + // to be unlinked. Ignore DBG_VALUE_LISTs -- they refer to vregs, and + // need to go through the normal live interval splitting process. + if (InstrRef && (MBBI->isNonListDebugValue() || MBBI->isDebugPHI() || + MBBI->isDebugRef())) { + MBBI = handleDebugInstr(*MBBI, Idx); + Changed = true; + // In normal debug mode, use the dedicated DBG_VALUE / DBG_LABEL handler + // to track things through register allocation, and erase the instr. + } else if ((MBBI->isDebugValue() && handleDebugValue(*MBBI, Idx)) || + (MBBI->isDebugLabel() && handleDebugLabel(*MBBI, Idx))) { MBBI = MBB.erase(MBBI); Changed = true; } else @@ -1238,7 +1262,7 @@ void LDVImpl::computeIntervals() { } } -bool LDVImpl::runOnMachineFunction(MachineFunction &mf) { +bool LDVImpl::runOnMachineFunction(MachineFunction &mf, bool InstrRef) { clear(); MF = &mf; LIS = &pass.getAnalysis(); @@ -1246,7 +1270,7 @@ bool LDVImpl::runOnMachineFunction(MachineFunction &mf) { LLVM_DEBUG(dbgs() << "********** COMPUTING LIVE DEBUG VARIABLES: " << mf.getName() << " **********\n"); - bool Changed = collectDebugValues(mf); + bool Changed = collectDebugValues(mf, InstrRef); computeIntervals(); LLVM_DEBUG(print(dbgs())); @@ -1287,9 +1311,19 @@ bool LiveDebugVariables::runOnMachineFunction(MachineFunction &mf) { removeDebugInstrs(mf); return false; } + + // Have we been asked to track variable locations using instruction + // referencing? + bool InstrRef = false; + auto *TPC = getAnalysisIfAvailable(); + if (TPC) { + auto &TM = TPC->getTM(); + InstrRef = TM.Options.ValueTrackingVariableLocations; + } + if (!pImpl) pImpl = new LDVImpl(this); - return static_cast(pImpl)->runOnMachineFunction(mf); + return static_cast(pImpl)->runOnMachineFunction(mf, InstrRef); } void LiveDebugVariables::releaseMemory() { @@ -1850,22 +1884,45 @@ void LDVImpl::emitDebugValues(VirtRegMap *VRM) { LLVM_DEBUG(dbgs() << "********** EMITTING INSTR REFERENCES **********\n"); - // Re-insert any DBG_INSTR_REFs back in the position they were. Ordering - // is preserved by vector. - const MCInstrDesc &RefII = TII->get(TargetOpcode::DBG_INSTR_REF); - for (auto &P : StashedInstrReferences) { - const SlotIndex &Idx = P.first; - auto *MBB = Slots->getMBBFromIndex(Idx); - MachineBasicBlock::iterator insertPos = - findInsertLocation(MBB, Idx, *LIS, BBSkipInstsMap); - for (auto &Stashed : P.second) { - auto MIB = BuildMI(*MF, std::get<4>(Stashed), RefII); - MIB.addImm(std::get<0>(Stashed)); - MIB.addImm(std::get<1>(Stashed)); - MIB.addMetadata(std::get<2>(Stashed)); - MIB.addMetadata(std::get<3>(Stashed)); - MachineInstr *New = MIB; - MBB->insert(insertPos, New); + // Re-insert any debug instrs back in the position they were. Ordering + // is preserved by vector. We must re-insert in the same order to ensure that + // debug instructions don't swap, which could re-order assignments. + for (auto &P : StashedDebugInstrs) { + SlotIndex Idx = P.Idx; + + // Start block index: find the first non-debug instr in the block, and + // insert before it. + if (Idx == Slots->getMBBStartIdx(P.MBB)) { + MachineBasicBlock::iterator InsertPos = + findInsertLocation(P.MBB, Idx, *LIS, BBSkipInstsMap); + P.MBB->insert(InsertPos, P.MI); + continue; + } + + if (MachineInstr *Pos = Slots->getInstructionFromIndex(Idx)) { + // Insert at the end of any debug instructions. + auto PostDebug = std::next(Pos->getIterator()); + PostDebug = skipDebugInstructionsForward(PostDebug, P.MBB->instr_end()); + P.MBB->insert(PostDebug, P.MI); + } else { + // Insert position disappeared; walk forwards through slots until we + // find a new one. + SlotIndex End = Slots->getMBBEndIdx(P.MBB); + for (; Idx < End; Idx = Slots->getNextNonNullIndex(Idx)) { + Pos = Slots->getInstructionFromIndex(Idx); + if (Pos) { + P.MBB->insert(Pos->getIterator(), P.MI); + break; + } + } + + // We have reached the end of the block and didn't find anywhere to + // insert! It's not safe to discard any debug instructions; place them + // in front of the first terminator, or in front of end(). + if (Idx >= End) { + auto TermIt = P.MBB->getFirstTerminator(); + P.MBB->insert(TermIt, P.MI); + } } } diff --git a/lib/CodeGen/LiveIntervals.cpp b/lib/CodeGen/LiveIntervals.cpp index c038ba94301..6075f64b2f2 100644 --- a/lib/CodeGen/LiveIntervals.cpp +++ b/lib/CodeGen/LiveIntervals.cpp @@ -475,7 +475,7 @@ bool LiveIntervals::shrinkToUses(LiveInterval *li, // Visit all instructions reading li->reg(). Register Reg = li->reg(); for (MachineInstr &UseMI : MRI->reg_instructions(Reg)) { - if (UseMI.isDebugValue() || !UseMI.readsVirtualRegister(Reg)) + if (UseMI.isDebugInstr() || !UseMI.readsVirtualRegister(Reg)) continue; SlotIndex Idx = getInstructionIndex(UseMI).getRegSlot(); LiveQueryResult LRQ = li->Query(Idx); diff --git a/lib/CodeGen/MachineSink.cpp b/lib/CodeGen/MachineSink.cpp index 41fa3a22c9c..ec98394dca7 100644 --- a/lib/CodeGen/MachineSink.cpp +++ b/lib/CodeGen/MachineSink.cpp @@ -718,7 +718,7 @@ MachineSinking::getBBRegisterPressure(MachineBasicBlock &MBB) { MIE = MBB.instr_begin(); MII != MIE; --MII) { MachineInstr &MI = *std::prev(MII); - if (MI.isDebugValue() || MI.isDebugLabel() || MI.isPseudoProbe()) + if (MI.isDebugInstr() || MI.isPseudoProbe()) continue; RegisterOperands RegOpers; RegOpers.collect(MI, *TRI, *MRI, false, false); diff --git a/lib/CodeGen/RegisterCoalescer.cpp b/lib/CodeGen/RegisterCoalescer.cpp index 0248115dd99..3b75e0e4a20 100644 --- a/lib/CodeGen/RegisterCoalescer.cpp +++ b/lib/CodeGen/RegisterCoalescer.cpp @@ -928,7 +928,7 @@ RegisterCoalescer::removeCopyByCommutingDef(const CoalescerPair &CP, if (UseMO.isUndef()) continue; MachineInstr *UseMI = UseMO.getParent(); - if (UseMI->isDebugValue()) { + if (UseMI->isDebugInstr()) { // FIXME These don't have an instruction index. Not clear we have enough // info to decide whether to do this replacement or not. For now do it. UseMO.setReg(NewReg); @@ -1561,7 +1561,7 @@ bool RegisterCoalescer::reMaterializeTrivialDef(const CoalescerPair &CP, UI != MRI->use_end();) { MachineOperand &UseMO = *UI++; MachineInstr *UseMI = UseMO.getParent(); - if (UseMI->isDebugValue()) { + if (UseMI->isDebugInstr()) { if (Register::isPhysicalRegister(DstReg)) UseMO.substPhysReg(DstReg, *TRI); else @@ -1742,7 +1742,7 @@ void RegisterCoalescer::updateRegDefsUses(Register SrcReg, Register DstReg, if (SubReg == 0 || MO.isUndef()) continue; MachineInstr &MI = *MO.getParent(); - if (MI.isDebugValue()) + if (MI.isDebugInstr()) continue; SlotIndex UseIdx = LIS->getInstructionIndex(MI).getRegSlot(true); addUndefFlag(*DstInt, UseIdx, MO, SubReg); @@ -1769,7 +1769,7 @@ void RegisterCoalescer::updateRegDefsUses(Register SrcReg, Register DstReg, // If SrcReg wasn't read, it may still be the case that DstReg is live-in // because SrcReg is a sub-register. - if (DstInt && !Reads && SubIdx && !UseMI->isDebugValue()) + if (DstInt && !Reads && SubIdx && !UseMI->isDebugInstr()) Reads = DstInt->liveAt(LIS->getInstructionIndex(*UseMI)); // Replace SrcReg with DstReg in all UseMI operands. @@ -1799,7 +1799,7 @@ void RegisterCoalescer::updateRegDefsUses(Register SrcReg, Register DstReg, // unused lanes. This may happen with rematerialization. DstInt->createSubRange(Allocator, UnusedLanes); } - SlotIndex MIIdx = UseMI->isDebugValue() + SlotIndex MIIdx = UseMI->isDebugInstr() ? LIS->getSlotIndexes()->getIndexBefore(*UseMI) : LIS->getInstructionIndex(*UseMI); SlotIndex UseIdx = MIIdx.getRegSlot(true); @@ -1815,7 +1815,7 @@ void RegisterCoalescer::updateRegDefsUses(Register SrcReg, Register DstReg, LLVM_DEBUG({ dbgs() << "\t\tupdated: "; - if (!UseMI->isDebugValue()) + if (!UseMI->isDebugInstr()) dbgs() << LIS->getInstructionIndex(*UseMI) << "\t"; dbgs() << *UseMI; }); diff --git a/lib/CodeGen/RegisterPressure.cpp b/lib/CodeGen/RegisterPressure.cpp index fae1ecfeb73..62a459fca61 100644 --- a/lib/CodeGen/RegisterPressure.cpp +++ b/lib/CodeGen/RegisterPressure.cpp @@ -873,7 +873,7 @@ void RegPressureTracker::recedeSkipDebugValues() { void RegPressureTracker::recede(SmallVectorImpl *LiveUses) { recedeSkipDebugValues(); - if (CurrPos->isDebugValue() || CurrPos->isPseudoProbe()) { + if (CurrPos->isDebugInstr() || CurrPos->isPseudoProbe()) { // It's possible to only have debug_value and pseudo probe instructions and // hit the start of the block. assert(CurrPos == MBB->begin()); diff --git a/lib/CodeGen/ScheduleDAGInstrs.cpp b/lib/CodeGen/ScheduleDAGInstrs.cpp index 62948fff861..2a4b0476b8b 100644 --- a/lib/CodeGen/ScheduleDAGInstrs.cpp +++ b/lib/CodeGen/ScheduleDAGInstrs.cpp @@ -807,7 +807,7 @@ void ScheduleDAGInstrs::buildSchedGraph(AAResults *AA, DbgMI = nullptr; } - if (MI.isDebugValue() || MI.isDebugRef()) { + if (MI.isDebugValue() || MI.isDebugRef() || MI.isDebugPHI()) { DbgMI = &MI; continue; } diff --git a/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp b/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp index baef5e7c4a7..5827a5f7fd4 100644 --- a/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp +++ b/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp @@ -5503,6 +5503,35 @@ bool SelectionDAGBuilder::EmitFuncArgumentDbgValue( if (!Arg) return false; + MachineFunction &MF = DAG.getMachineFunction(); + const TargetInstrInfo *TII = DAG.getSubtarget().getInstrInfo(); + + // Helper to create DBG_INSTR_REFs or DBG_VALUEs, depending on what kind + // we've been asked to pursue. + auto MakeVRegDbgValue = [&](Register Reg, DIExpression *FragExpr, + bool Indirect) { + if (Reg.isVirtual() && TM.Options.ValueTrackingVariableLocations) { + // For VRegs, in instruction referencing mode, create a DBG_INSTR_REF + // pointing at the VReg, which will be patched up later. + auto &Inst = TII->get(TargetOpcode::DBG_INSTR_REF); + auto MIB = BuildMI(MF, DL, Inst); + MIB.addReg(Reg, RegState::Debug); + MIB.addImm(0); + MIB.addMetadata(Variable); + auto *NewDIExpr = FragExpr; + // We don't have an "Indirect" field in DBG_INSTR_REF, fold that into + // the DIExpression. + if (Indirect) + NewDIExpr = DIExpression::prepend(FragExpr, DIExpression::DerefBefore); + MIB.addMetadata(NewDIExpr); + return MIB; + } else { + // Create a completely standard DBG_VALUE. + auto &Inst = TII->get(TargetOpcode::DBG_VALUE); + return BuildMI(MF, DL, Inst, Indirect, Reg, Variable, FragExpr); + } + }; + if (!IsDbgDeclare) { // ArgDbgValues are hoisted to the beginning of the entry block. So we // should only emit as ArgDbgValue if the dbg.value intrinsic is found in @@ -5568,9 +5597,6 @@ bool SelectionDAGBuilder::EmitFuncArgumentDbgValue( } } - MachineFunction &MF = DAG.getMachineFunction(); - const TargetInstrInfo *TII = DAG.getSubtarget().getInstrInfo(); - bool IsIndirect = false; Optional Op; // Some arguments' frame index is recorded during argument lowering. @@ -5640,9 +5666,9 @@ bool SelectionDAGBuilder::EmitFuncArgumentDbgValue( DAG.AddDbgValue(SDV, false); continue; } - FuncInfo.ArgDbgValues.push_back( - BuildMI(MF, DL, TII->get(TargetOpcode::DBG_VALUE), IsDbgDeclare, - RegAndSize.first, Variable, *FragmentExpr)); + MachineInstr *NewMI = + MakeVRegDbgValue(RegAndSize.first, *FragmentExpr, IsDbgDeclare); + FuncInfo.ArgDbgValues.push_back(NewMI); } }; @@ -5673,11 +5699,15 @@ bool SelectionDAGBuilder::EmitFuncArgumentDbgValue( assert(Variable->isValidLocationForIntrinsic(DL) && "Expected inlined-at fields to agree"); - IsIndirect = (Op->isReg()) ? IsIndirect : true; - FuncInfo.ArgDbgValues.push_back( - BuildMI(MF, DL, TII->get(TargetOpcode::DBG_VALUE), IsIndirect, - *Op, Variable, Expr)); + MachineInstr *NewMI = nullptr; + if (Op->isReg()) + NewMI = MakeVRegDbgValue(Op->getReg(), Expr, IsIndirect); + else + NewMI = BuildMI(MF, DL, TII->get(TargetOpcode::DBG_VALUE), true, *Op, + Variable, Expr); + + FuncInfo.ArgDbgValues.push_back(NewMI); return true; } diff --git a/lib/Target/X86/X86InstrInfo.cpp b/lib/Target/X86/X86InstrInfo.cpp index bf7a0bd9803..74e0dff568c 100644 --- a/lib/Target/X86/X86InstrInfo.cpp +++ b/lib/Target/X86/X86InstrInfo.cpp @@ -4182,6 +4182,8 @@ bool X86InstrInfo::optimizeCompareInstr(MachineInstr &CmpInstr, Register SrcReg, } CmpInstr.setDesc(get(NewOpcode)); CmpInstr.RemoveOperand(0); + // Mutating this instruction invalidates any debug data associated with it. + CmpInstr.setDebugInstrNum(0); // Fall through to optimize Cmp if Cmp is CMPrr or CMPri. if (NewOpcode == X86::CMP64rm || NewOpcode == X86::CMP32rm || NewOpcode == X86::CMP16rm || NewOpcode == X86::CMP8rm) diff --git a/test/DebugInfo/MIR/InstrRef/phi-coalesce-subreg.mir b/test/DebugInfo/MIR/InstrRef/phi-coalesce-subreg.mir index 035dd321bbd..b18f57f4d91 100644 --- a/test/DebugInfo/MIR/InstrRef/phi-coalesce-subreg.mir +++ b/test/DebugInfo/MIR/InstrRef/phi-coalesce-subreg.mir @@ -106,8 +106,6 @@ body: | %3:gr32 = COPY $edi %6:gr16 = COPY %4.sub_16bit %5:gr16 = COPY %3.sub_16bit - DBG_VALUE %5, $noreg, !12, !DIExpression(), debug-location !13 - DBG_VALUE %6, $noreg, !14, !DIExpression(), debug-location !13 ADJCALLSTACKDOWN64 0, 0, 0, implicit-def $rsp, implicit-def $eflags, implicit-def $ssp, implicit $rsp, implicit $ssp, debug-location !13 %15:gr32 = MOVSX32rr16 %5, debug-location !13 $edi = COPY %15, debug-location !13 @@ -116,7 +114,6 @@ body: | %14:gr32 = MOVSX32rr16 %5, debug-location !13 %13:gr32 = ADD32ri8 killed %14, 12, implicit-def $eflags, debug-location !13 %11:gr16 = COPY killed %13.sub_16bit, debug-location !13 - DBG_VALUE %11, $noreg, !12, !DIExpression(), debug-location !13 ADJCALLSTACKDOWN64 0, 0, 0, implicit-def $rsp, implicit-def $eflags, implicit-def $ssp, implicit $rsp, implicit $ssp, debug-location !13 %9:gr32 = MOVSX32rr16 %11, debug-location !13 $edi = COPY %9, debug-location !13 @@ -135,14 +132,12 @@ body: | %20:gr32 = MOVSX32rr16 %11, debug-location !13 %19:gr32 = ADD32ri8 killed %20, 1, implicit-def $eflags, debug-location !13 %17:gr16 = COPY killed %19.sub_16bit, debug-location !13 - DBG_VALUE %17, $noreg, !12, !DIExpression(), debug-location !13 - ; Verify that vreg 17 is coalesced into gr32. + ; Verify that vreg 17 is coalesced into a gr32, and not copied any further. ; DOESCOALESCE: %{{[0-9]+}}:gr32 = ADD32ri8 - ; DOESCOALESCE-NEXT: DBG_VALUE %{{[0-9]+}}.sub_16bit, + ; DOESCOALESCE-NOT: COPY ; Verify those registers land in $bx ; CHECK: renamable $ebp = ADD32ri8 - ; CHECK-NEXT: DBG_VALUE $bp ; DOESCOALESCE-LABEL: bb.2.if.end: ; CHECK-LABEL: bb.2.if.end: @@ -154,7 +149,6 @@ body: | %30:gr32 = MOVSX32rr16 killed %2, debug-location !13 %29:gr32 = ADD32rr killed %30, killed %31, implicit-def $eflags, debug-location !13 %26:gr16 = COPY killed %29.sub_16bit, debug-location !13 - DBG_VALUE %26, $noreg, !12, !DIExpression(), debug-location !13 ADJCALLSTACKDOWN64 0, 0, 0, implicit-def $rsp, implicit-def $eflags, implicit-def $ssp, implicit $rsp, implicit $ssp, debug-location !13 %24:gr32 = MOVSX32rr16 %26, debug-location !13 $edi = COPY %24, debug-location !13 diff --git a/test/DebugInfo/MIR/InstrRef/phi-coalescing.mir b/test/DebugInfo/MIR/InstrRef/phi-coalescing.mir index ae4b68b8c26..c37804f6036 100644 --- a/test/DebugInfo/MIR/InstrRef/phi-coalescing.mir +++ b/test/DebugInfo/MIR/InstrRef/phi-coalescing.mir @@ -14,7 +14,7 @@ # # Original C code, the PHI is of the value of 'bar' after the control flow. # Compiled at -O0, applied -mem2reg, llc -O0, then manually added the PHI -# instruction label. +# instruction label. Additional variable locations removed. # # void ext(long); # long getlong(void); @@ -115,14 +115,11 @@ body: | %3:gr64 = COPY $rdi %4:gr64 = COPY killed %3 %6:gr64 = COPY killed %5 - DBG_VALUE %4, $noreg, !12, !DIExpression(), debug-location !13 - DBG_VALUE %6, $noreg, !14, !DIExpression(), debug-location !13 ADJCALLSTACKDOWN64 0, 0, 0, implicit-def $rsp, implicit-def $eflags, implicit-def $ssp, implicit $rsp, implicit $ssp, debug-location !13 $rdi = COPY %4, debug-location !13 CALL64pcrel32 @ext, csr_64, implicit $rsp, implicit $ssp, implicit $rdi, debug-location !13 ADJCALLSTACKUP64 0, 0, implicit-def $rsp, implicit-def $eflags, implicit-def $ssp, implicit $rsp, implicit $ssp, debug-location !13 %9:gr64 = ADD64ri32 %4, 12, implicit-def $eflags, debug-location !13 - DBG_VALUE %9, $noreg, !12, !DIExpression(), debug-location !13 ADJCALLSTACKDOWN64 0, 0, 0, implicit-def $rsp, implicit-def $eflags, implicit-def $ssp, implicit $rsp, implicit $ssp, debug-location !13 $rdi = COPY %9, debug-location !13 CALL64pcrel32 @ext, csr_64, implicit $rsp, implicit $ssp, implicit $rdi, debug-location !13 @@ -138,7 +135,6 @@ body: | ; CHECK-LABEL: bb.1.if.then: bb.1.if.then: %10:gr64 = ADD64ri32 %9, 1, implicit-def $eflags, debug-location !13 - DBG_VALUE %10, $noreg, !12, !DIExpression(), debug-location !13 ; Verify that the vreg is different immediately after register coalescing. ; DOESCOALESCE-NOT: %10:gr64 ADD64ri32 @@ -153,7 +149,6 @@ body: | ; CHECK: DBG_PHI $rbx, 1 DBG_INSTR_REF 1, 0, !12, !DIExpression(), debug-location !13 %14:gr64 = ADD64rr killed %2, %6, implicit-def $eflags, debug-location !13 - DBG_VALUE %14, $noreg, !12, !DIExpression(), debug-location !13 ADJCALLSTACKDOWN64 0, 0, 0, implicit-def $rsp, implicit-def $eflags, implicit-def $ssp, implicit $rsp, implicit $ssp, debug-location !13 $rdi = COPY %14, debug-location !13 CALL64pcrel32 @ext, csr_64, implicit $rsp, implicit $ssp, implicit $rdi, debug-location !13 diff --git a/test/DebugInfo/MIR/InstrRef/phi-regallocd-to-stack.mir b/test/DebugInfo/MIR/InstrRef/phi-regallocd-to-stack.mir index 09593b1c9a9..b40f2ae9cb6 100644 --- a/test/DebugInfo/MIR/InstrRef/phi-regallocd-to-stack.mir +++ b/test/DebugInfo/MIR/InstrRef/phi-regallocd-to-stack.mir @@ -77,10 +77,7 @@ body: | DBG_VALUE $edi, $noreg, !11, !DIExpression(), debug-location !12 DBG_VALUE $esi, $noreg, !13, !DIExpression(), debug-location !12 %2:gr32 = COPY killed $esi - DBG_VALUE %2, $noreg, !13, !DIExpression(), debug-location !12 %1:gr32 = COPY killed $edi - DBG_VALUE %1, $noreg, !11, !DIExpression(), debug-location !12 - DBG_VALUE 0, $noreg, !14, !DIExpression(), debug-location !12 ADJCALLSTACKDOWN64 0, 0, 0, implicit-def dead $rsp, implicit-def dead $eflags, implicit-def dead $ssp, implicit $rsp, implicit $ssp, debug-location !15 %3:gr32 = MOV32r0 implicit-def dead $eflags $edi = COPY killed %3, debug-location !15 @@ -107,7 +104,6 @@ body: | JMP_1 %bb.1, debug-location !17 bb.1: - DBG_VALUE %2, $noreg, !14, !DIExpression(), debug-location !12 %30:gr32 = MOV32ri 0 %31:gr32 = MOV32ri 1 %32:gr32 = MOV32ri 2 diff --git a/test/DebugInfo/MIR/InstrRef/phi-through-regalloc.mir b/test/DebugInfo/MIR/InstrRef/phi-through-regalloc.mir index fbb5db605a7..0e56a79c093 100644 --- a/test/DebugInfo/MIR/InstrRef/phi-through-regalloc.mir +++ b/test/DebugInfo/MIR/InstrRef/phi-through-regalloc.mir @@ -103,9 +103,7 @@ body: | DBG_VALUE $edi, $noreg, !11, !DIExpression(), debug-location !12 DBG_VALUE $esi, $noreg, !13, !DIExpression(), debug-location !12 %2:gr32 = COPY killed $esi - DBG_VALUE %2, $noreg, !13, !DIExpression(), debug-location !12 %1:gr32 = COPY killed $edi - DBG_VALUE %1, $noreg, !11, !DIExpression(), debug-location !12 DBG_VALUE 0, $noreg, !14, !DIExpression(), debug-location !12 ADJCALLSTACKDOWN64 0, 0, 0, implicit-def dead $rsp, implicit-def dead $eflags, implicit-def dead $ssp, implicit $rsp, implicit $ssp, debug-location !15 %3:gr32 = MOV32r0 implicit-def dead $eflags @@ -118,7 +116,6 @@ body: | JMP_1 %bb.1, debug-location !17 bb.1.if.else: - DBG_VALUE %2, $noreg, !14, !DIExpression(), debug-location !12 ; CHECK-LABEL: bb.2.if.end: bb.2.if.end: diff --git a/test/DebugInfo/MIR/InstrRef/x86-drop-compare-inst.mir b/test/DebugInfo/MIR/InstrRef/x86-drop-compare-inst.mir new file mode 100644 index 00000000000..0a56d8a615b --- /dev/null +++ b/test/DebugInfo/MIR/InstrRef/x86-drop-compare-inst.mir @@ -0,0 +1,95 @@ +# RUN: llc %s -o - -experimental-debug-variable-locations -run-pass=peephole-opt | FileCheck %s +# +# X86 initially produces subtract operations to perform comparisons, and then +# downgrades them into cmp instructions if nothing uses the result. It does this +# by calling setDesc on the instruction, mutating it from one sort to another, +# which makes any debug instruction numbers attached to the number invalid. +# This test tests that the relevant instruction number is dropped. +# +# CHECK-NOT: debug-instr-number +# CHECK: CMP32rm +# CHECK-NOT: debug-instr-number +# +--- | + ; ModuleID = '/fast/fs/build34llvm4/reduced.ll' + target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128" + target triple = "x86_64-unknown-linux-gnu" + + %"class.std::vector.534" = type { %"struct.std::_Vector_base.535" } + %"struct.std::_Vector_base.535" = type { %"struct.std::_Vector_base>::_Vector_impl" } + %"struct.std::_Vector_base>::_Vector_impl" = type { i8*, i8*, i8* } + + ; Function Attrs: nofree nosync nounwind readnone speculatable willreturn + declare void @llvm.dbg.declare(metadata, metadata, metadata) #0 + + define hidden fastcc void @soup() unnamed_addr !dbg !3 { + _ZN4llvm11raw_ostreamlsEPKc.exit2752: + %0 = load %"class.std::vector.534"*, %"class.std::vector.534"** undef, align 8, !dbg !7 + %1 = load i8*, i8** undef, align 8, !dbg !7 + %_M_start.i2756 = getelementptr inbounds %"class.std::vector.534", %"class.std::vector.534"* %0, i64 undef, i32 0, i32 0, i32 0, !dbg !7 + %2 = load i8*, i8** %_M_start.i2756, align 8, !dbg !7 + %sub.ptr.lhs.cast.i2757 = ptrtoint i8* %1 to i64, !dbg !7 + %sub.ptr.rhs.cast.i2758 = ptrtoint i8* %2 to i64, !dbg !7 + %sub.ptr.sub.i2759 = sub i64 %sub.ptr.lhs.cast.i2757, %sub.ptr.rhs.cast.i2758, !dbg !7 + %conv373 = trunc i64 %sub.ptr.sub.i2759 to i32, !dbg !7 + call void @llvm.dbg.value(metadata i32 %conv373, metadata !8, metadata !DIExpression()), !dbg !7 + %cmp375.not2842 = icmp eq i32 %conv373, 0, !dbg !7 + br i1 %cmp375.not2842, label %for.cond.cleanup376, label %for.body377, !dbg !7 + + for.cond.cleanup376: ; preds = %_ZN4llvm11raw_ostreamlsEPKc.exit2752 + ret void + + for.body377: ; preds = %_ZN4llvm11raw_ostreamlsEPKc.exit2752 + ret void + } + + ; Function Attrs: nofree nosync nounwind readnone speculatable willreturn + declare void @llvm.dbg.value(metadata, metadata, metadata) #0 + + attributes #0 = { nofree nosync nounwind readnone speculatable willreturn } + + !llvm.module.flags = !{!0} + !llvm.dbg.cu = !{!1} + + !0 = !{i32 2, !"Debug Info Version", i32 3} + !1 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus, file: !2, producer: "beards", isOptimized: true, runtimeVersion: 4, emissionKind: FullDebug) + !2 = !DIFile(filename: "bees.cpp", directory: "") + !3 = distinct !DISubprogram(name: "nope", scope: !2, file: !2, line: 1, type: !4, spFlags: DISPFlagDefinition, unit: !1) + !4 = !DISubroutineType(types: !5) + !5 = !{!6} + !6 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed) + !7 = !DILocation(line: 1, scope: !3) + !8 = !DILocalVariable(name: "flannel", scope: !3) + +... +--- +name: soup +alignment: 16 +tracksRegLiveness: true +registers: + - { id: 0, class: gr64 } + - { id: 1, class: gr64 } + - { id: 2, class: gr32 } + - { id: 3, class: gr32 } +frameInfo: + maxAlignment: 1 +machineFunctionInfo: {} +body: | + bb.0._ZN4llvm11raw_ostreamlsEPKc.exit2752: + successors: %bb.1(0x30000000), %bb.2(0x50000000) + + %1:gr64 = IMPLICIT_DEF + %0:gr64 = MOV64rm killed %1, 1, $noreg, 0, $noreg, debug-location !7 :: (load (s64) from `i8** undef`) + %2:gr32 = COPY %0.sub_32bit, debug-location !7 + %3:gr32 = SUB32rm %2, %0, 1, $noreg, 0, $noreg, implicit-def $eflags, debug-instr-number 1, debug-location !7 :: (load (s32) from %ir._M_start.i2756, align 8) + DBG_INSTR_REF 1, 0, !8, !DIExpression(), debug-location !7 + JCC_1 %bb.2, 5, implicit $eflags, debug-location !7 + JMP_1 %bb.1, debug-location !7 + + bb.1.for.cond.cleanup376: + RET 0 + + bb.2.for.body377: + RET 0 + +...