From 10cedc40e30bb3bfa288869efcd9a9ca900fb40e Mon Sep 17 00:00:00 2001 From: Jeremy Morse Date: Tue, 1 Feb 2022 19:19:20 +0000 Subject: [PATCH 1/7] Re-apply 3fab2d138e30, now with a triple added Was reverted in 1c1b670a73a9 as it broke all non-x86 bots. Original commit message: [DebugInfo][InstrRef] Add a max-stack-slots-to-track cut-out In certain circumstances with things like autogenerated code and asan, you can end up with thousands of Values live at the same time, causing a large working set and a lot of information spilled to the stack. Unfortunately InstrRefBasedLDV doesn't cope well with this and consumes a lot of memory when there are many many stack slots. See the reproducer in D116821. It seems very unlikely that a developer would be able to reason about hundreds of live named local variables at the same time, so a huge working set and many stack slots is an indicator that we're likely analysing autogenerated or instrumented code. In those cases: gracefully degrade by setting an upper bound on the amount of stack slots to track. This limits peak memory consumption, at the cost of dropping some variable locations, but in a rare scenario where it's unlikely someone is actually going to use them. In terms of the patch, this adds a cl::opt for max number of stack slots to track, and has the stack-slot-numbering code optionally return None. That then filters through a number of code paths, which can then chose to not track a spill / restore if it touches an untracked spill slot. The added test checks that we drop variable locations that are on the stack, if we set the limit to zero. Differential Revision: https://reviews.llvm.org/D118601 (cherry picked from commit 14aaaa12366f7d1328b5ec81c71c63de9d878e75) --- .../LiveDebugValues/InstrRefBasedImpl.cpp | 100 ++++++++++++------ .../LiveDebugValues/InstrRefBasedImpl.h | 10 +- .../MIR/InstrRef/spill-slot-limits.mir | 90 ++++++++++++++++ llvm/unittests/CodeGen/InstrRefLDVTest.cpp | 14 +-- 4 files changed, 169 insertions(+), 45 deletions(-) create mode 100644 llvm/test/DebugInfo/MIR/InstrRef/spill-slot-limits.mir diff --git a/llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.cpp b/llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.cpp index 0eb6100230bd1..2e69a5e437e76 100644 --- a/llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.cpp +++ b/llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.cpp @@ -148,6 +148,20 @@ static cl::opt EmulateOldLDV("emulate-old-livedebugvalues", cl::Hidden, cl::desc("Act like old LiveDebugValues did"), cl::init(false)); +// Limit for the maximum number of stack slots we should track, past which we +// will ignore any spills. InstrRefBasedLDV gathers detailed information on all +// stack slots which leads to high memory consumption, and in some scenarios +// (such as asan with very many locals) the working set of the function can be +// very large, causing many spills. In these scenarios, it is very unlikely that +// the developer has hundreds of variables live at the same time that they're +// carefully thinking about -- instead, they probably autogenerated the code. +// When this happens, gracefully stop tracking excess spill slots, rather than +// consuming all the developer's memory. +static cl::opt + StackWorkingSetLimit("livedebugvalues-max-stack-slots", cl::Hidden, + cl::desc("livedebugvalues-stack-ws-limit"), + cl::init(250)); + /// Tracker for converting machine value locations and variable values into /// variable locations (the output of LiveDebugValues), recorded as DBG_VALUEs /// specifying block live-in locations and transfers within blocks. @@ -757,9 +771,15 @@ void MLocTracker::writeRegMask(const MachineOperand *MO, unsigned CurBB, Masks.push_back(std::make_pair(MO, InstID)); } -SpillLocationNo MLocTracker::getOrTrackSpillLoc(SpillLoc L) { +Optional MLocTracker::getOrTrackSpillLoc(SpillLoc L) { SpillLocationNo SpillID(SpillLocs.idFor(L)); + if (SpillID.id() == 0) { + // If there is no location, and we have reached the limit of how many stack + // slots to track, then don't track this one. + if (SpillLocs.size() >= StackWorkingSetLimit) + return None; + // Spill location is untracked: create record for this one, and all // subregister slots too. SpillID = SpillLocationNo(SpillLocs.insert(L)); @@ -898,7 +918,7 @@ bool InstrRefBasedLDV::isCalleeSaved(LocIdx L) const { // void InstrRefBasedLDV::printVarLocInMBB(..) #endif -SpillLocationNo +Optional InstrRefBasedLDV::extractSpillBaseRegAndOffset(const MachineInstr &MI) { assert(MI.hasOneMemOperand() && "Spill instruction does not have exactly one memory operand?"); @@ -913,8 +933,11 @@ InstrRefBasedLDV::extractSpillBaseRegAndOffset(const MachineInstr &MI) { return MTracker->getOrTrackSpillLoc({Reg, Offset}); } -Optional InstrRefBasedLDV::findLocationForMemOperand(const MachineInstr &MI) { - SpillLocationNo SpillLoc = extractSpillBaseRegAndOffset(MI); +Optional +InstrRefBasedLDV::findLocationForMemOperand(const MachineInstr &MI) { + Optional SpillLoc = extractSpillBaseRegAndOffset(MI); + if (!SpillLoc) + return None; // Where in the stack slot is this value defined -- i.e., what size of value // is this? An important question, because it could be loaded into a register @@ -930,7 +953,7 @@ Optional InstrRefBasedLDV::findLocationForMemOperand(const MachineInstr // occur, but the safe action is to indicate the variable is optimised out. return None; - unsigned SpillID = MTracker->getSpillIDWithIdx(SpillLoc, IdxIt->second); + unsigned SpillID = MTracker->getSpillIDWithIdx(*SpillLoc, IdxIt->second); return MTracker->getSpillMLoc(SpillID); } @@ -1251,7 +1274,12 @@ bool InstrRefBasedLDV::transferDebugPHI(MachineInstr &MI) { Register Base; StackOffset Offs = TFI->getFrameIndexReference(*MI.getMF(), FI, Base); SpillLoc SL = {Base, Offs}; - SpillLocationNo SpillNo = MTracker->getOrTrackSpillLoc(SL); + Optional SpillNo = MTracker->getOrTrackSpillLoc(SL); + + // We might be able to find a value, but have chosen not to, to avoid + // tracking too much stack information. + if (!SpillNo) + return true; // Problem: what value should we extract from the stack? LLVM does not // record what size the last store to the slot was, and it would become @@ -1263,7 +1291,7 @@ bool InstrRefBasedLDV::transferDebugPHI(MachineInstr &MI) { Optional Result = None; Optional SpillLoc = None; for (unsigned CS : CandidateSizes) { - unsigned SpillID = MTracker->getLocID(SpillNo, {CS, 0}); + unsigned SpillID = MTracker->getLocID(*SpillNo, {CS, 0}); SpillLoc = MTracker->getSpillMLoc(SpillID); ValueIDNum Val = MTracker->readMLoc(*SpillLoc); // If this value was defined in it's own position, then it was probably @@ -1280,7 +1308,7 @@ bool InstrRefBasedLDV::transferDebugPHI(MachineInstr &MI) { // "supposed" to be is more complex, and benefits a small number of // locations. if (!Result) { - unsigned SpillID = MTracker->getLocID(SpillNo, {64, 0}); + unsigned SpillID = MTracker->getLocID(*SpillNo, {64, 0}); SpillLoc = MTracker->getSpillMLoc(SpillID); Result = MTracker->readMLoc(*SpillLoc); } @@ -1357,11 +1385,12 @@ void InstrRefBasedLDV::transferRegisterDef(MachineInstr &MI) { // If this instruction writes to a spill slot, def that slot. if (hasFoldedStackStore(MI)) { - SpillLocationNo SpillNo = extractSpillBaseRegAndOffset(MI); - for (unsigned int I = 0; I < MTracker->NumSlotIdxes; ++I) { - unsigned SpillID = MTracker->getSpillIDWithIdx(SpillNo, I); - LocIdx L = MTracker->getSpillMLoc(SpillID); - MTracker->setMLoc(L, ValueIDNum(CurBB, CurInst, L)); + if (Optional SpillNo = extractSpillBaseRegAndOffset(MI)) { + for (unsigned int I = 0; I < MTracker->NumSlotIdxes; ++I) { + unsigned SpillID = MTracker->getSpillIDWithIdx(*SpillNo, I); + LocIdx L = MTracker->getSpillMLoc(SpillID); + MTracker->setMLoc(L, ValueIDNum(CurBB, CurInst, L)); + } } } @@ -1398,11 +1427,12 @@ void InstrRefBasedLDV::transferRegisterDef(MachineInstr &MI) { // Tell TTracker about any folded stack store. if (hasFoldedStackStore(MI)) { - SpillLocationNo SpillNo = extractSpillBaseRegAndOffset(MI); - for (unsigned int I = 0; I < MTracker->NumSlotIdxes; ++I) { - unsigned SpillID = MTracker->getSpillIDWithIdx(SpillNo, I); - LocIdx L = MTracker->getSpillMLoc(SpillID); - TTracker->clobberMloc(L, MI.getIterator(), true); + if (Optional SpillNo = extractSpillBaseRegAndOffset(MI)) { + for (unsigned int I = 0; I < MTracker->NumSlotIdxes; ++I) { + unsigned SpillID = MTracker->getSpillIDWithIdx(*SpillNo, I); + LocIdx L = MTracker->getSpillMLoc(SpillID); + TTracker->clobberMloc(L, MI.getIterator(), true); + } } } } @@ -1438,23 +1468,24 @@ void InstrRefBasedLDV::performCopy(Register SrcRegNum, Register DstRegNum) { } } -bool InstrRefBasedLDV::isSpillInstruction(const MachineInstr &MI, - MachineFunction *MF) { +Optional +InstrRefBasedLDV::isSpillInstruction(const MachineInstr &MI, + MachineFunction *MF) { // TODO: Handle multiple stores folded into one. if (!MI.hasOneMemOperand()) - return false; + return None; // Reject any memory operand that's aliased -- we can't guarantee its value. auto MMOI = MI.memoperands_begin(); const PseudoSourceValue *PVal = (*MMOI)->getPseudoValue(); if (PVal->isAliased(MFI)) - return false; + return None; if (!MI.getSpillSize(TII) && !MI.getFoldedSpillSize(TII)) - return false; // This is not a spill instruction, since no valid size was - // returned from either function. + return None; // This is not a spill instruction, since no valid size was + // returned from either function. - return true; + return extractSpillBaseRegAndOffset(MI); } bool InstrRefBasedLDV::isLocationSpill(const MachineInstr &MI, @@ -1511,13 +1542,11 @@ bool InstrRefBasedLDV::transferSpillOrRestoreInst(MachineInstr &MI) { // First, if there are any DBG_VALUEs pointing at a spill slot that is // written to, terminate that variable location. The value in memory // will have changed. DbgEntityHistoryCalculator doesn't try to detect this. - if (isSpillInstruction(MI, MF)) { - SpillLocationNo Loc = extractSpillBaseRegAndOffset(MI); - + if (Optional Loc = isSpillInstruction(MI, MF)) { // Un-set this location and clobber, so that earlier locations don't // continue past this store. for (unsigned SlotIdx = 0; SlotIdx < MTracker->NumSlotIdxes; ++SlotIdx) { - unsigned SpillID = MTracker->getSpillIDWithIdx(Loc, SlotIdx); + unsigned SpillID = MTracker->getSpillIDWithIdx(*Loc, SlotIdx); Optional MLoc = MTracker->getSpillMLoc(SpillID); if (!MLoc) continue; @@ -1535,7 +1564,9 @@ bool InstrRefBasedLDV::transferSpillOrRestoreInst(MachineInstr &MI) { // Try to recognise spill and restore instructions that may transfer a value. if (isLocationSpill(MI, MF, Reg)) { - SpillLocationNo Loc = extractSpillBaseRegAndOffset(MI); + // isLocationSpill returning true should guarantee we can extract a + // location. + SpillLocationNo Loc = *extractSpillBaseRegAndOffset(MI); auto DoTransfer = [&](Register SrcReg, unsigned SpillID) { auto ReadValue = MTracker->readReg(SrcReg); @@ -1562,10 +1593,9 @@ bool InstrRefBasedLDV::transferSpillOrRestoreInst(MachineInstr &MI) { unsigned SpillID = MTracker->getLocID(Loc, {Size, 0}); DoTransfer(Reg, SpillID); } else { - Optional OptLoc = isRestoreInstruction(MI, MF, Reg); - if (!OptLoc) + Optional Loc = isRestoreInstruction(MI, MF, Reg); + if (!Loc) return false; - SpillLocationNo Loc = *OptLoc; // Assumption: we're reading from the base of the stack slot, not some // offset into it. It seems very unlikely LLVM would ever generate @@ -1592,13 +1622,13 @@ bool InstrRefBasedLDV::transferSpillOrRestoreInst(MachineInstr &MI) { for (MCSubRegIterator SRI(Reg, TRI, false); SRI.isValid(); ++SRI) { unsigned Subreg = TRI->getSubRegIndex(Reg, *SRI); - unsigned SpillID = MTracker->getLocID(Loc, Subreg); + unsigned SpillID = MTracker->getLocID(*Loc, Subreg); DoTransfer(*SRI, SpillID); } // Directly look up this registers slot idx by size, and transfer. unsigned Size = TRI->getRegSizeInBits(Reg, *MRI); - unsigned SpillID = MTracker->getLocID(Loc, {Size, 0}); + unsigned SpillID = MTracker->getLocID(*Loc, {Size, 0}); DoTransfer(Reg, SpillID); } return true; diff --git a/llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.h b/llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.h index e7383209c0274..fe1d29260e4da 100644 --- a/llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.h +++ b/llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.h @@ -616,7 +616,9 @@ class MLocTracker { void writeRegMask(const MachineOperand *MO, unsigned CurBB, unsigned InstID); /// Find LocIdx for SpillLoc \p L, creating a new one if it's not tracked. - SpillLocationNo getOrTrackSpillLoc(SpillLoc L); + /// Returns None when in scenarios where a spill slot could be tracked, but + /// we would likely run into resource limitations. + Optional getOrTrackSpillLoc(SpillLoc L); // Get LocIdx of a spill ID. LocIdx getSpillMLoc(unsigned SpillID) { @@ -873,7 +875,8 @@ class InstrRefBasedLDV : public LDVImpl { StringRef StackProbeSymbolName; /// Tests whether this instruction is a spill to a stack slot. - bool isSpillInstruction(const MachineInstr &MI, MachineFunction *MF); + Optional isSpillInstruction(const MachineInstr &MI, + MachineFunction *MF); /// Decide if @MI is a spill instruction and return true if it is. We use 2 /// criteria to make this decision: @@ -891,7 +894,8 @@ class InstrRefBasedLDV : public LDVImpl { /// Given a spill instruction, extract the spill slot information, ensure it's /// tracked, and return the spill number. - SpillLocationNo extractSpillBaseRegAndOffset(const MachineInstr &MI); + Optional + extractSpillBaseRegAndOffset(const MachineInstr &MI); /// Observe a single instruction while stepping through a block. void process(MachineInstr &MI, ValueIDNum **MLiveOuts = nullptr, diff --git a/llvm/test/DebugInfo/MIR/InstrRef/spill-slot-limits.mir b/llvm/test/DebugInfo/MIR/InstrRef/spill-slot-limits.mir new file mode 100644 index 0000000000000..7969fafabc788 --- /dev/null +++ b/llvm/test/DebugInfo/MIR/InstrRef/spill-slot-limits.mir @@ -0,0 +1,90 @@ +# RUN: llc %s -o - -experimental-debug-variable-locations \ +# RUN: -mtriple=x86_64-unknown-unknown \ +# RUN: -run-pass=livedebugvalues -livedebugvalues-max-stack-slots=0 | \ +# RUN: FileCheck %s --implicit-check-not=DBG_VALUE +# RUN: llc %s -o - -experimental-debug-variable-locations \ +# RUN: -mtriple=x86_64-unknown-unknown \ +# RUN: -run-pass=livedebugvalues -livedebugvalues-max-stack-slots=100 | \ +# RUN: FileCheck %s --check-prefixes=NOLIMIT --implicit-check-not=DBG_VALUE +# +# Test that spills of live values to the stack are NOT tracked by +# LiveDebugValues if an internal accounting limit is exceeded -- in this test, +# set to zero. This is to avoid scenarios where we track thousands of stack +# slots, which can show up with autogenerated code and/or asan. +# +# This is a copy of livedebugvalues_stackslot_subregs.mir, here the stack slot +# limit is set to zero, meaning the spill shouldn't be tracked. +# +## Capture variable num, +# CHECK: ![[VARNUM:[0-9]+]] = !DILocalVariable +# +## There should be no variable location, just a single DBG_VALUE $noreg. +# CHECK: DBG_VALUE $noreg +# +## And then another. +# CHECK: DBG_VALUE $noreg +# +## Test that if there's no limit, we _do_ get some locations. +# NOLIMIT: DBG_INSTR_REF 1, 0 +# NOLIMIT-NEXT: DBG_VALUE $esi +# +# NOLIMIT: DBG_INSTR_REF 5, +# NOLIMIT-NEXT: DBG_VALUE $rsp +--- | + define i8 @test(i32 %bar) local_unnamed_addr !dbg !7 { + entry: + ret i8 0, !dbg !12 + } + + declare dso_local void @ext(i64) + + !llvm.dbg.cu = !{!0} + !llvm.module.flags = !{!3, !4, !5, !6} + !0 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus, file: !1, producer: "", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug) + !1 = !DIFile(filename: "foo.cpp", directory: ".") + !2 = !DIBasicType(name: "int", size: 8, encoding: DW_ATE_signed) + !3 = !{i32 2, !"Dwarf Version", i32 4} + !4 = !{i32 2, !"Debug Info Version", i32 3} + !5 = !{i32 1, !"wchar_size", i32 2} + !6 = !{i32 7, !"PIC Level", i32 2} + !7 = distinct !DISubprogram(name: "foo", linkageName: "foo", scope: !1, file: !1, line: 6, type: !8, scopeLine: 6, flags: DIFlagPrototyped, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !0, retainedNodes: !10) + !8 = !DISubroutineType(types: !9) + !9 = !{!2, !2} + !10 = !{!11} + !11 = !DILocalVariable(name: "baz", scope: !7, file: !1, line: 7, type: !2) + !12 = !DILocation(line: 10, scope: !7) +... +--- +name: test +tracksRegLiveness: true +liveins: + - { reg: '$rdi', virtual-reg: '' } +stack: + - { id: 0, name: '', type: spill-slot, offset: -16, size: 8, alignment: 8, + stack-id: default, callee-saved-register: '', callee-saved-restored: true, + debug-info-variable: '', debug-info-expression: '', debug-info-location: '' } +body: | + bb.0: + liveins: $rdi, $rax, $rbx + $eax = MOV32ri 0, debug-instr-number 1 + $edi = COPY $eax + MOV64mr $rsp, 1, $noreg, 16, $noreg, $rdi :: (store 8 into %stack.0) + $rsi = MOV64rm $rsp, 1, $noreg, 8, $noreg :: (load 8 from %stack.0) + + MOV64mr $rsp, 1, $noreg, 16, $noreg, $rbx :: (store 8 into %stack.0) + $rax = MOV64ri 0 + $rdi = MOV64ri 0 + + DBG_INSTR_REF 1, 0, !11, !DIExpression(), debug-location !12 + ; This shouldn't find anything -- we have disabled tracking of spills. + + ; In addition to plain spills, spills that are folded into instructions + ; shouldn't be tracked either. + INC32m $rsp, 1, $noreg, 4, $noreg, implicit-def dead $eflags, debug-instr-number 5, debug-location !12 :: (store (s32) into %stack.0) + + + DBG_INSTR_REF 5, 1000000, !11, !DIExpression(), debug-location !12 + ; Shouldn't be able to find the reference to instr 5's memory operand. + + RET64 $rsi, debug-location !12 +... diff --git a/llvm/unittests/CodeGen/InstrRefLDVTest.cpp b/llvm/unittests/CodeGen/InstrRefLDVTest.cpp index cf7b23662365a..9b7683108d777 100644 --- a/llvm/unittests/CodeGen/InstrRefLDVTest.cpp +++ b/llvm/unittests/CodeGen/InstrRefLDVTest.cpp @@ -642,7 +642,7 @@ TEST_F(InstrRefLDVTest, MTransferCopies) { // it's not completely clear why, but here we only care about correctly // identifying the slot, not that all the surrounding data is correct. SpillLoc L = {getRegByName("RSP"), StackOffset::getFixed(-8)}; - SpillLocationNo SpillNo = MTracker->getOrTrackSpillLoc(L); + SpillLocationNo SpillNo = *MTracker->getOrTrackSpillLoc(L); unsigned SpillLocID = MTracker->getLocID(SpillNo, {64, 0}); LocIdx SpillLoc = MTracker->getSpillMLoc(SpillLocID); ValueIDNum V = MTracker->readMLoc(SpillLoc); @@ -766,7 +766,7 @@ TEST_F(InstrRefLDVTest, MTransferSubregSpills) { ValueIDNum DefNum(0, 1, RegLoc); // Read the corresponding subreg field from the stack. SpillLoc L = {getRegByName("RSP"), StackOffset::getFixed(-8)}; - SpillLocationNo SpillNo = MTracker->getOrTrackSpillLoc(L); + SpillLocationNo SpillNo = *MTracker->getOrTrackSpillLoc(L); unsigned SpillID = MTracker->getLocID(SpillNo, SubRegIdxes[I]); LocIdx SpillLoc = MTracker->getSpillMLoc(SpillID); ValueIDNum SpillValue = MTracker->readMLoc(SpillLoc); @@ -803,7 +803,7 @@ TEST_F(InstrRefLDVTest, MTransferSubregSpills) { // $rbx should contain something else; today it's a def at the spill point // of the 4 byte value. SpillLoc L = {getRegByName("RSP"), StackOffset::getFixed(-8)}; - SpillLocationNo SpillNo = MTracker->getOrTrackSpillLoc(L); + SpillLocationNo SpillNo = *MTracker->getOrTrackSpillLoc(L); unsigned SpillID = MTracker->getLocID(SpillNo, {64, 0}); LocIdx Spill64Loc = MTracker->getSpillMLoc(SpillID); ValueIDNum DefAtSpill64(0, 3, Spill64Loc); @@ -817,7 +817,7 @@ TEST_F(InstrRefLDVTest, MTransferSubregSpills) { LocIdx RegLoc = MTracker->getRegMLoc(getRegByName(SubRegNames[I])); ValueIDNum DefNum(0, 1, RegLoc); // Read the corresponding subreg field from the stack. - SpillNo = MTracker->getOrTrackSpillLoc(L); + SpillNo = *MTracker->getOrTrackSpillLoc(L); SpillID = MTracker->getLocID(SpillNo, SubRegIdxes[I]); LocIdx SpillLoc = MTracker->getSpillMLoc(SpillID); ValueIDNum SpillValue = MTracker->readMLoc(SpillLoc); @@ -846,7 +846,7 @@ TEST_F(InstrRefLDVTest, MTransferSubregSpills) { for (unsigned int I = 0; I < 5; ++I) { // Read subreg fields from the stack. - SpillLocationNo SpillNo = MTracker->getOrTrackSpillLoc(L); + SpillLocationNo SpillNo = *MTracker->getOrTrackSpillLoc(L); unsigned SpillID = MTracker->getLocID(SpillNo, SubRegIdxes[I]); LocIdx SpillLoc = MTracker->getSpillMLoc(SpillID); ValueIDNum SpillValue = MTracker->readMLoc(SpillLoc); @@ -859,7 +859,7 @@ TEST_F(InstrRefLDVTest, MTransferSubregSpills) { // Read xmm0's position and ensure it has a value. Should be the live-in // value to the block, as IMPLICIT_DEF isn't a real def. - SpillNo = MTracker->getOrTrackSpillLoc(L); + SpillNo = *MTracker->getOrTrackSpillLoc(L); SpillID = MTracker->getLocID(SpillNo, {128, 0}); LocIdx Spill128Loc = MTracker->getSpillMLoc(SpillID); SpillValue = MTracker->readMLoc(Spill128Loc); @@ -1097,7 +1097,7 @@ TEST_F(InstrRefLDVTest, MLocDiamondSpills) { // Create a stack location and ensure it's tracked. SpillLoc SL = {getRegByName("RSP"), StackOffset::getFixed(-8)}; - SpillLocationNo SpillNo = MTracker->getOrTrackSpillLoc(SL); + SpillLocationNo SpillNo = *MTracker->getOrTrackSpillLoc(SL); ASSERT_EQ(MTracker->getNumLocs(), 10u); // Tracks all possible stack locs. // Locations are: RSP, stack slots from 2^3 bits wide up to 2^9 for zmm regs, // then slots for sub_8bit_hi and sub_16bit_hi ({8, 8} and {16, 16}). From bf78f84dfefaebbe7ce310602562e64cc6f492a2 Mon Sep 17 00:00:00 2001 From: Jeremy Morse Date: Wed, 2 Feb 2022 12:12:32 +0000 Subject: [PATCH 2/7] [DebugInfo][InstrRef][NFC] Cache some PHI resolutions Install a cache of DBG_INSTR_REF -> ValueIDNum resolutions, for scenarios where the value has to be reconstructed from several DBG_PHIs. Whenever this happens, it's because branch folding + tail duplication has messed with the SSA form of the program, and we have to solve a mini SSA problem to find the variable value. This is always called twice, so it makes sense to cache the value. This gives a ~0.5% geomean compile-time-performance improvement on CTMark. Differential Revision: https://reviews.llvm.org/D118455 (cherry picked from commit d556eb7e27c25ae20befb0811bc8a3423241431d) --- .../LiveDebugValues/InstrRefBasedImpl.cpp | 16 ++++++++++++++++ .../CodeGen/LiveDebugValues/InstrRefBasedImpl.h | 12 ++++++++++++ 2 files changed, 28 insertions(+) diff --git a/llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.cpp b/llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.cpp index 2e69a5e437e76..25d764dee2ed8 100644 --- a/llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.cpp +++ b/llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.cpp @@ -3122,6 +3122,7 @@ bool InstrRefBasedLDV::ExtendRanges(MachineFunction &MF, DebugPHINumToValue.clear(); OverlapFragments.clear(); SeenFragments.clear(); + SeenDbgPHIs.clear(); return Changed; } @@ -3387,6 +3388,21 @@ Optional InstrRefBasedLDV::resolveDbgPHIs(MachineFunction &MF, ValueIDNum **MLiveIns, MachineInstr &Here, uint64_t InstrNum) { + // This function will be called twice per DBG_INSTR_REF, and might end up + // computing lots of SSA information: memoize it. + auto SeenDbgPHIIt = SeenDbgPHIs.find(&Here); + if (SeenDbgPHIIt != SeenDbgPHIs.end()) + return SeenDbgPHIIt->second; + + Optional Result = + resolveDbgPHIsImpl(MF, MLiveOuts, MLiveIns, Here, InstrNum); + SeenDbgPHIs.insert({&Here, Result}); + return Result; +} + +Optional InstrRefBasedLDV::resolveDbgPHIsImpl( + MachineFunction &MF, ValueIDNum **MLiveOuts, ValueIDNum **MLiveIns, + MachineInstr &Here, uint64_t InstrNum) { // Pick out records of DBG_PHI instructions that have been observed. If there // are none, then we cannot compute a value number. auto RangePair = std::equal_range(DebugPHINumToValue.begin(), diff --git a/llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.h b/llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.h index fe1d29260e4da..1e1bb758d3d44 100644 --- a/llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.h +++ b/llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.h @@ -864,6 +864,12 @@ class InstrRefBasedLDV : public LDVImpl { OverlapMap OverlapFragments; VarToFragments SeenFragments; + /// Mapping of DBG_INSTR_REF instructions to their values, for those + /// DBG_INSTR_REFs that call resolveDbgPHIs. These variable references solve + /// a mini SSA problem caused by DBG_PHIs being cloned, this collection caches + /// the result. + DenseMap> SeenDbgPHIs; + /// True if we need to examine call instructions for stack clobbers. We /// normally assume that they don't clobber SP, but stack probes on Windows /// do. @@ -944,6 +950,12 @@ class InstrRefBasedLDV : public LDVImpl { ValueIDNum **MLiveIns, MachineInstr &Here, uint64_t InstrNum); + Optional resolveDbgPHIsImpl(MachineFunction &MF, + ValueIDNum **MLiveOuts, + ValueIDNum **MLiveIns, + MachineInstr &Here, + uint64_t InstrNum); + /// Step through the function, recording register definitions and movements /// in an MLocTracker. Convert the observations into a per-block transfer /// function in \p MLocTransfer, suitable for using with the machine value From 1794a5a7922d2f03dbdd6caa85e667c0c1881767 Mon Sep 17 00:00:00 2001 From: Jeremy Morse Date: Wed, 2 Feb 2022 12:23:52 +0000 Subject: [PATCH 3/7] [DebugInfo][InstrRef][NFC] Free resources at an earlier stage This patch releases some memory from InstrRefBasedLDV earlier that it would otherwise. The underlying problem is: * We store a big table of "live in values for each block", * We translate that into DBG_VALUE instructions in each block, And both exist in memory at the same time, which needlessly doubles that information. The most of what this patch does is: as we progressively translate live-in information into DBG_VALUEs, we free the variable-value / machine-value tracking information as we go, which significantly reduces peak memory. While I'm here, also add a clear method to wipe variable assignments that have been accumulated into VLocTracker objects, and turn a DenseMap into a SmallDenseMap to avoid an initial allocation. Differential Revision: https://reviews.llvm.org/D118453 (cherry picked from commit a80181a81ea44215e49e5da1457614ec0bd44111) --- .../LiveDebugValues/InstrRefBasedImpl.cpp | 30 ++++++++++++++----- .../LiveDebugValues/InstrRefBasedImpl.h | 7 ++++- 2 files changed, 29 insertions(+), 8 deletions(-) diff --git a/llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.cpp b/llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.cpp index 25d764dee2ed8..7b06bc9b0c39c 100644 --- a/llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.cpp +++ b/llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.cpp @@ -1029,7 +1029,7 @@ bool InstrRefBasedLDV::transferDebugInstrRef(MachineInstr &MI, // Only handle this instruction when we are building the variable value // transfer function. - if (!VTracker) + if (!VTracker && !TTracker) return false; unsigned InstNo = MI.getOperand(0).getImm(); @@ -1185,7 +1185,8 @@ bool InstrRefBasedLDV::transferDebugInstrRef(MachineInstr &MI, // for DBG_INSTR_REFs as DBG_VALUEs (just, the former can refer to values that // aren't immediately available). DbgValueProperties Properties(Expr, false); - VTracker->defVar(MI, Properties, NewID); + if (VTracker) + VTracker->defVar(MI, Properties, NewID); // If we're on the final pass through the function, decompose this INSTR_REF // into a plain DBG_VALUE. @@ -2826,6 +2827,7 @@ void InstrRefBasedLDV::emitLocations( const TargetPassConfig &TPC) { TTracker = new TransferTracker(TII, MTracker, MF, *TRI, CalleeSavedRegs, TPC); unsigned NumLocs = MTracker->getNumLocs(); + VTracker = nullptr; // For each block, load in the machine value locations and variable value // live-ins, then step through each instruction in the block. New DBG_VALUEs @@ -2844,6 +2846,15 @@ void InstrRefBasedLDV::emitLocations( TTracker->checkInstForNewValues(CurInst, MI.getIterator()); ++CurInst; } + + // Our block information has now been converted into DBG_VALUEs, to be + // inserted below. Free the memory we allocated to track variable / register + // values. If we don't, we needlessy record the same info in memory twice. + delete[] MInLocs[bbnum]; + delete[] MOutLocs[bbnum]; + MInLocs[bbnum] = nullptr; + MOutLocs[bbnum] = nullptr; + SavedLiveIns[bbnum].clear(); } emitTransfers(AllVarsNumbering); @@ -3080,6 +3091,12 @@ bool InstrRefBasedLDV::ExtendRanges(MachineFunction &MF, << " has " << MaxNumBlocks << " basic blocks and " << VarAssignCount << " variable assignments, exceeding limits.\n"); + + // Perform memory cleanup that emitLocations would do otherwise. + for (int Idx = 0; Idx < MaxNumBlocks; ++Idx) { + delete[] MOutLocs[Idx]; + delete[] MInLocs[Idx]; + } } else { // Compute the extended ranges, iterating over scopes. There might be // something to be said for ordering them by size/locality, but that's for @@ -3091,6 +3108,9 @@ bool InstrRefBasedLDV::ExtendRanges(MachineFunction &MF, vlocs); } + // Now that we've analysed variable assignments, free any tracking data. + vlocs.clear(); + // Using the computed value locations and variable values for each block, // create the DBG_VALUE instructions representing the extended variable // locations. @@ -3100,11 +3120,7 @@ bool InstrRefBasedLDV::ExtendRanges(MachineFunction &MF, Changed = TTracker->Transfers.size() != 0; } - // Common clean-up of memory. - for (int Idx = 0; Idx < MaxNumBlocks; ++Idx) { - delete[] MOutLocs[Idx]; - delete[] MInLocs[Idx]; - } + // Elements of these arrays will be deleted by emitLocations. delete[] MOutLocs; delete[] MInLocs; diff --git a/llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.h b/llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.h index 1e1bb758d3d44..9ae0d4dd087c3 100644 --- a/llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.h +++ b/llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.h @@ -680,7 +680,7 @@ class VLocTracker { /// movement of values between locations inside of a block is handled at a /// much later stage, in the TransferTracker class. MapVector Vars; - DenseMap Scopes; + SmallDenseMap Scopes; MachineBasicBlock *MBB = nullptr; const OverlapMap &OverlappingFragments; DbgValueProperties EmptyProperties; @@ -749,6 +749,11 @@ class VLocTracker { Scopes[Overlapped] = Loc; } } + + void clear() { + Vars.clear(); + Scopes.clear(); + } }; // XXX XXX docs From 62fbb9d0d25626da31e4c5550e227cb2a3fcf27b Mon Sep 17 00:00:00 2001 From: Jeremy Morse Date: Wed, 2 Feb 2022 13:48:36 +0000 Subject: [PATCH 4/7] [DebugInfo][InstrRef][NFC] Use depth-first scope search for variable locs This patch aims to reduce max-rss from instruction referencing, by avoiding keeping variable value information in memory for too long. Instead of computing all the variable values then emitting them to DBG_VALUE instructions, this patch tries to stream the information out through a depth first search: * Make use of the fact LexicalScopes gives a depth-number to each lexical scope, * Produce a map that identifies the last lexical scope to make use of a block, * Enumerate each scope in LexicalScopes' DFS order, solving the variable value problem, * After each scope is processed, look for any blocks that won't be used by any other scope, and emit all the variable information to DBG_VALUE instructions. Differential Revision: https://reviews.llvm.org/D118460 (cherry picked from commit 9fd9d56dc6bdeeddf8cf2af834c6c39d00cd7244) --- .../LiveDebugValues/InstrRefBasedImpl.cpp | 234 +++++++++++++----- .../LiveDebugValues/InstrRefBasedImpl.h | 34 ++- 2 files changed, 196 insertions(+), 72 deletions(-) diff --git a/llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.cpp b/llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.cpp index 7b06bc9b0c39c..268095e0943a0 100644 --- a/llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.cpp +++ b/llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.cpp @@ -2821,45 +2821,6 @@ void InstrRefBasedLDV::dump_mloc_transfer( } #endif -void InstrRefBasedLDV::emitLocations( - MachineFunction &MF, LiveInsT SavedLiveIns, ValueIDNum **MOutLocs, - ValueIDNum **MInLocs, DenseMap &AllVarsNumbering, - const TargetPassConfig &TPC) { - TTracker = new TransferTracker(TII, MTracker, MF, *TRI, CalleeSavedRegs, TPC); - unsigned NumLocs = MTracker->getNumLocs(); - VTracker = nullptr; - - // For each block, load in the machine value locations and variable value - // live-ins, then step through each instruction in the block. New DBG_VALUEs - // to be inserted will be created along the way. - for (MachineBasicBlock &MBB : MF) { - unsigned bbnum = MBB.getNumber(); - MTracker->reset(); - MTracker->loadFromArray(MInLocs[bbnum], bbnum); - TTracker->loadInlocs(MBB, MInLocs[bbnum], SavedLiveIns[MBB.getNumber()], - NumLocs); - - CurBB = bbnum; - CurInst = 1; - for (auto &MI : MBB) { - process(MI, MOutLocs, MInLocs); - TTracker->checkInstForNewValues(CurInst, MI.getIterator()); - ++CurInst; - } - - // Our block information has now been converted into DBG_VALUEs, to be - // inserted below. Free the memory we allocated to track variable / register - // values. If we don't, we needlessy record the same info in memory twice. - delete[] MInLocs[bbnum]; - delete[] MOutLocs[bbnum]; - MInLocs[bbnum] = nullptr; - MOutLocs[bbnum] = nullptr; - SavedLiveIns[bbnum].clear(); - } - - emitTransfers(AllVarsNumbering); -} - void InstrRefBasedLDV::initialSetup(MachineFunction &MF) { // Build some useful data structures. @@ -2902,8 +2863,175 @@ void InstrRefBasedLDV::initialSetup(MachineFunction &MF) { #endif } +// Produce an "ejection map" for blocks, i.e., what's the highest-numbered +// lexical scope it's used in. When exploring in DFS order and we pass that +// scope, the block can be processed and any tracking information freed. +void InstrRefBasedLDV::makeDepthFirstEjectionMap( + SmallVectorImpl &EjectionMap, + const ScopeToDILocT &ScopeToDILocation, + ScopeToAssignBlocksT &ScopeToAssignBlocks) { + SmallPtrSet BlocksToExplore; + SmallVector, 4> WorkStack; + auto *TopScope = LS.getCurrentFunctionScope(); + + // Unlike lexical scope explorers, we explore in reverse order, to find the + // "last" lexical scope used for each block early. + WorkStack.push_back({TopScope, TopScope->getChildren().size() - 1}); + + while (!WorkStack.empty()) { + auto &ScopePosition = WorkStack.back(); + LexicalScope *WS = ScopePosition.first; + ssize_t ChildNum = ScopePosition.second--; + + const SmallVectorImpl &Children = WS->getChildren(); + if (ChildNum >= 0) { + // If ChildNum is positive, there are remaining children to explore. + // Push the child and its children-count onto the stack. + auto &ChildScope = Children[ChildNum]; + WorkStack.push_back( + std::make_pair(ChildScope, ChildScope->getChildren().size() - 1)); + } else { + WorkStack.pop_back(); + + // We've explored all children and any later blocks: examine all blocks + // in our scope. If they haven't yet had an ejection number set, then + // this scope will be the last to use that block. + auto DILocationIt = ScopeToDILocation.find(WS); + if (DILocationIt != ScopeToDILocation.end()) { + getBlocksForScope(DILocationIt->second, BlocksToExplore, + ScopeToAssignBlocks.find(WS)->second); + for (auto *MBB : BlocksToExplore) { + unsigned BBNum = MBB->getNumber(); + if (EjectionMap[BBNum] == 0) + EjectionMap[BBNum] = WS->getDFSOut(); + } + + BlocksToExplore.clear(); + } + } + } +} + +bool InstrRefBasedLDV::depthFirstVLocAndEmit( + unsigned MaxNumBlocks, const ScopeToDILocT &ScopeToDILocation, + const ScopeToVarsT &ScopeToVars, ScopeToAssignBlocksT &ScopeToAssignBlocks, + LiveInsT &Output, ValueIDNum **MOutLocs, ValueIDNum **MInLocs, + SmallVectorImpl &AllTheVLocs, MachineFunction &MF, + DenseMap &AllVarsNumbering, + const TargetPassConfig &TPC) { + TTracker = new TransferTracker(TII, MTracker, MF, *TRI, CalleeSavedRegs, TPC); + unsigned NumLocs = MTracker->getNumLocs(); + VTracker = nullptr; + + // No scopes? No variable locations. + if (!LS.getCurrentFunctionScope()) + return false; + + // Build map from block number to the last scope that uses the block. + SmallVector EjectionMap; + EjectionMap.resize(MaxNumBlocks, 0); + makeDepthFirstEjectionMap(EjectionMap, ScopeToDILocation, + ScopeToAssignBlocks); + + // Helper lambda for ejecting a block -- if nothing is going to use the block, + // we can translate the variable location information into DBG_VALUEs and then + // free all of InstrRefBasedLDV's data structures. + auto EjectBlock = [&](MachineBasicBlock &MBB) -> void { + unsigned BBNum = MBB.getNumber(); + AllTheVLocs[BBNum].clear(); + + // Prime the transfer-tracker, and then step through all the block + // instructions, installing transfers. + MTracker->reset(); + MTracker->loadFromArray(MInLocs[BBNum], BBNum); + TTracker->loadInlocs(MBB, MInLocs[BBNum], Output[BBNum], NumLocs); + + CurBB = BBNum; + CurInst = 1; + for (auto &MI : MBB) { + process(MI, MOutLocs, MInLocs); + TTracker->checkInstForNewValues(CurInst, MI.getIterator()); + ++CurInst; + } + + // Free machine-location tables for this block. + delete[] MInLocs[BBNum]; + delete[] MOutLocs[BBNum]; + // Make ourselves brittle to use-after-free errors. + MInLocs[BBNum] = nullptr; + MOutLocs[BBNum] = nullptr; + // We don't need live-in variable values for this block either. + Output[BBNum].clear(); + AllTheVLocs[BBNum].clear(); + }; + + SmallPtrSet BlocksToExplore; + SmallVector, 4> WorkStack; + WorkStack.push_back({LS.getCurrentFunctionScope(), 0}); + unsigned HighestDFSIn = 0; + + // Proceed to explore in depth first order. + while (!WorkStack.empty()) { + auto &ScopePosition = WorkStack.back(); + LexicalScope *WS = ScopePosition.first; + ssize_t ChildNum = ScopePosition.second++; + + // We obesrve scopes with children twice here, once descending in, once + // ascending out of the scope nest. Use HighestDFSIn as a ratchet to ensure + // we don't process a scope twice. Additionally, ignore scopes that don't + // have a DILocation -- by proxy, this means we never tracked any variable + // assignments in that scope. + auto DILocIt = ScopeToDILocation.find(WS); + if (HighestDFSIn <= WS->getDFSIn() && DILocIt != ScopeToDILocation.end()) { + const DILocation *DILoc = DILocIt->second; + auto &VarsWeCareAbout = ScopeToVars.find(WS)->second; + auto &BlocksInScope = ScopeToAssignBlocks.find(WS)->second; + + buildVLocValueMap(DILoc, VarsWeCareAbout, BlocksInScope, Output, MOutLocs, + MInLocs, AllTheVLocs); + } + + HighestDFSIn = std::max(HighestDFSIn, WS->getDFSIn()); + + // Descend into any scope nests. + const SmallVectorImpl &Children = WS->getChildren(); + if (ChildNum < (ssize_t)Children.size()) { + // There are children to explore -- push onto stack and continue. + auto &ChildScope = Children[ChildNum]; + WorkStack.push_back(std::make_pair(ChildScope, 0)); + } else { + WorkStack.pop_back(); + + // We've explored a leaf, or have explored all the children of a scope. + // Try to eject any blocks where this is the last scope it's relevant to. + auto DILocationIt = ScopeToDILocation.find(WS); + if (DILocationIt == ScopeToDILocation.end()) + continue; + + getBlocksForScope(DILocationIt->second, BlocksToExplore, + ScopeToAssignBlocks.find(WS)->second); + for (auto *MBB : BlocksToExplore) + if (WS->getDFSOut() == EjectionMap[MBB->getNumber()]) + EjectBlock(const_cast(*MBB)); + + BlocksToExplore.clear(); + } + } + + // Some artificial blocks may not have been ejected, meaning they're not + // connected to an actual legitimate scope. This can technically happen + // with things like the entry block. In theory, we shouldn't need to do + // anything for such out-of-scope blocks, but for the sake of being similar + // to VarLocBasedLDV, eject these too. + for (auto *MBB : ArtificialBlocks) + if (MOutLocs[MBB->getNumber()]) + EjectBlock(*MBB); + + return emitTransfers(AllVarsNumbering); +} + bool InstrRefBasedLDV::emitTransfers( - DenseMap &AllVarsNumbering) { + DenseMap &AllVarsNumbering) { // Go through all the transfers recorded in the TransferTracker -- this is // both the live-ins to a block, and any movements of values that happen // in the middle. @@ -3098,26 +3226,12 @@ bool InstrRefBasedLDV::ExtendRanges(MachineFunction &MF, delete[] MInLocs[Idx]; } } else { - // Compute the extended ranges, iterating over scopes. There might be - // something to be said for ordering them by size/locality, but that's for - // the future. For each scope, solve the variable value problem, producing - // a map of variables to values in SavedLiveIns. - for (auto &P : ScopeToVars) { - buildVLocValueMap(ScopeToDILocation[P.first], P.second, - ScopeToAssignBlocks[P.first], SavedLiveIns, MOutLocs, MInLocs, - vlocs); - } - - // Now that we've analysed variable assignments, free any tracking data. - vlocs.clear(); - - // Using the computed value locations and variable values for each block, - // create the DBG_VALUE instructions representing the extended variable - // locations. - emitLocations(MF, SavedLiveIns, MOutLocs, MInLocs, AllVarsNumbering, *TPC); - - // Did we actually make any changes? If we created any DBG_VALUEs, then yes. - Changed = TTracker->Transfers.size() != 0; + // Optionally, solve the variable value problem and emit to blocks by using + // a lexical-scope-depth search. It should be functionally identical to + // the "else" block of this condition. + Changed = depthFirstVLocAndEmit( + MaxNumBlocks, ScopeToDILocation, ScopeToVars, ScopeToAssignBlocks, + SavedLiveIns, MOutLocs, MInLocs, vlocs, MF, AllVarsNumbering, *TPC); } // Elements of these arrays will be deleted by emitLocations. diff --git a/llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.h b/llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.h index 9ae0d4dd087c3..d778561db471c 100644 --- a/llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.h +++ b/llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.h @@ -1071,18 +1071,6 @@ class InstrRefBasedLDV : public LDVImpl { const LiveIdxT &LiveOuts, ValueIDNum **MOutLocs, const SmallVectorImpl &BlockOrders); - /// Given the solutions to the two dataflow problems, machine value locations - /// in \p MInLocs and live-in variable values in \p SavedLiveIns, runs the - /// TransferTracker class over the function to produce live-in and transfer - /// DBG_VALUEs, then inserts them. Groups of DBG_VALUEs are inserted in the - /// order given by AllVarsNumbering -- this could be any stable order, but - /// right now "order of appearence in function, when explored in RPO", so - /// that we can compare explictly against VarLocBasedImpl. - void emitLocations(MachineFunction &MF, LiveInsT SavedLiveIns, - ValueIDNum **MOutLocs, ValueIDNum **MInLocs, - DenseMap &AllVarsNumbering, - const TargetPassConfig &TPC); - /// Take collections of DBG_VALUE instructions stored in TTracker, and /// install them into their output blocks. Preserves a stable order of /// DBG_VALUEs produced (which would otherwise cause nondeterminism) through @@ -1093,6 +1081,28 @@ class InstrRefBasedLDV : public LDVImpl { /// RPOT block ordering. void initialSetup(MachineFunction &MF); + /// Produce a map of the last lexical scope that uses a block, using the + /// scopes DFSOut number. Mapping is block-number to DFSOut. + /// \p EjectionMap Pre-allocated vector in which to install the built ma. + /// \p ScopeToDILocation Mapping of LexicalScopes to their DILocations. + /// \p AssignBlocks Map of blocks where assignments happen for a scope. + void makeDepthFirstEjectionMap(SmallVectorImpl &EjectionMap, + const ScopeToDILocT &ScopeToDILocation, + ScopeToAssignBlocksT &AssignBlocks); + + /// When determining per-block variable values and emitting to DBG_VALUEs, + /// this function explores by lexical scope depth. Doing so means that per + /// block information can be fully computed before exploration finishes, + /// allowing us to emit it and free data structures earlier than otherwise. + /// It's also good for locality. + bool depthFirstVLocAndEmit( + unsigned MaxNumBlocks, const ScopeToDILocT &ScopeToDILocation, + const ScopeToVarsT &ScopeToVars, ScopeToAssignBlocksT &ScopeToBlocks, + LiveInsT &Output, ValueIDNum **MOutLocs, ValueIDNum **MInLocs, + SmallVectorImpl &AllTheVLocs, MachineFunction &MF, + DenseMap &AllVarsNumbering, + const TargetPassConfig &TPC); + bool ExtendRanges(MachineFunction &MF, MachineDominatorTree *DomTree, TargetPassConfig *TPC, unsigned InputBBLimit, unsigned InputDbgValLimit) override; From 22a0163e7e345bb854a3d1ac74eec8b62c87bcb5 Mon Sep 17 00:00:00 2001 From: Jeremy Morse Date: Wed, 2 Feb 2022 15:54:24 +0000 Subject: [PATCH 5/7] Follow up to 9fd9d56dc6b, avoid a memory leak Gaps in the basic block number range (from blocks being deleted or folded) get block-value-tables allocated but never ejected, leading to a memory leak, currently tripping up the asan buildbots. Fix this up by manually freeing that memory. As suggested elsewhere, if these things were owned by a unique_ptr then cleanup would happen automagically. D118774 should eliminate the need for this dance. (cherry picked from commit 206cafb680cea0741f8c7b276351db516ff27f81) --- llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.cpp | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.cpp b/llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.cpp index 268095e0943a0..cc800736a2dc1 100644 --- a/llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.cpp +++ b/llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.cpp @@ -3027,6 +3027,16 @@ bool InstrRefBasedLDV::depthFirstVLocAndEmit( if (MOutLocs[MBB->getNumber()]) EjectBlock(*MBB); + // Finally, there might have been gaps in the block numbering, from dead + // blocks being deleted or folded. In those scenarios, we might allocate a + // block-table that's never ejected, meaning we have to free it at the end. + for (unsigned int I = 0; I < MaxNumBlocks; ++I) { + if (MInLocs[I]) { + delete[] MInLocs[I]; + delete[] MOutLocs[I]; + } + } + return emitTransfers(AllVarsNumbering); } From 022c0e4b4544d6e6e1b5d964f3706959dfd42c07 Mon Sep 17 00:00:00 2001 From: Jeremy Morse Date: Wed, 2 Feb 2022 15:04:14 +0000 Subject: [PATCH 6/7] [DebugInfo][InstrRef] Fix a tombstone-in-DenseMap crash from D117877 This is a follow-up to D117877: variable assignments of DBG_VALUE $noreg, or DBG_INSTR_REFs where no value can be found, are represented by a DbgValue object with Kind "Undef", explicitly meaning "there is no value". In D117877 I added a special-case to some assignment accounting faster, without considering this scenario. It causes variables to be given the value ValueIDNum::EmptyValue, which then ends up being a DenseMap key. The DenseMap asserts, because EmptyValue is the tombstone key. Fix this by handling the assign-undef scenario in the special case, to match what happens in the general case: the variable has no value if it's only ever assigned $noreg / undef. Differential Revision: https://reviews.llvm.org/D118715 (cherry picked from commit 43de305704a50983bf134d8fb916f752a02eb076) --- .../LiveDebugValues/InstrRefBasedImpl.cpp | 5 ++ .../InstrRef/single-assign-propagation.mir | 65 ++++++++++++++++++- 2 files changed, 68 insertions(+), 2 deletions(-) diff --git a/llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.cpp b/llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.cpp index cc800736a2dc1..3b4d717c9ab4a 100644 --- a/llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.cpp +++ b/llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.cpp @@ -2796,6 +2796,11 @@ void InstrRefBasedLDV::placePHIsForSingleVarDefinition( auto ValueIt = VLocs.Vars.find(Var); const DbgValue &Value = ValueIt->second; + // If it's an explicit assignment of "undef", that means there is no location + // anyway, anywhere. + if (Value.Kind == DbgValue::Undef) + return; + // Assign the variable value to entry to each dominated block that's in scope. // Skip the definition block -- it's assigned the variable value in the middle // of the block somewhere. diff --git a/llvm/test/DebugInfo/MIR/InstrRef/single-assign-propagation.mir b/llvm/test/DebugInfo/MIR/InstrRef/single-assign-propagation.mir index 19408b2ec4291..c247004368837 100644 --- a/llvm/test/DebugInfo/MIR/InstrRef/single-assign-propagation.mir +++ b/llvm/test/DebugInfo/MIR/InstrRef/single-assign-propagation.mir @@ -1,9 +1,11 @@ # RUN: llc %s -march=x86-64 -run-pass=livedebugvalues -o - \ # RUN: -experimental-debug-variable-locations=true \ -# RUN: | FileCheck %s -implicit-check-not=DBG_VALUE +# RUN: | FileCheck %s -implicit-check-not=DBG_VALUE \ +# RUN: --check-prefixes=CHECK,COMMON # RUN: llc %s -march=x86-64 -run-pass=livedebugvalues -o - \ # RUN: -experimental-debug-variable-locations=false \ -# RUN: | FileCheck %s --check-prefixes=VARLOC -implicit-check-not=DBG_VALUE +# RUN: | FileCheck %s -implicit-check-not=DBG_VALUE \ +# RUN: --check-prefixes=VARLOC,COMMON # # This test is designed to stimulate a simplification of variable-value # propagation in InstrRefBasedLDV. When we only have a single assignment of @@ -63,12 +65,23 @@ # VARLOC-LABEL: bb.3: # VARLOC: DBG_VALUE # +## Common tail for 'test2' -- this is checking that the assignment of undef or +## $noreg in single-assignment mode doesn't lead to trouble further down the +## line, specifically assertion failures. +# +# COMMON-LABEL: name: test2 +# COMMON: DBG_VALUE $noreg --- | define i32 @_Z8bb_to_bb() local_unnamed_addr !dbg !12 { entry: ret i32 0, !dbg !17 } + define i32 @test2() local_unnamed_addr !dbg !112 { + entry: + ret i32 0, !dbg !117 + } + !llvm.dbg.cu = !{!0} !llvm.module.flags = !{!7, !8, !9, !10} !llvm.ident = !{!11} @@ -94,6 +107,13 @@ !18 = distinct !DILexicalBlock(scope: !12, file: !1, line: 1, column: 1) !19 = distinct !DILexicalBlock(scope: !12, file: !1, line: 1, column: 1) !20 = !DILocation(line: 10, scope: !19) + !112 = distinct !DISubprogram(name: "test2", linkageName: "102", scope: !1, file: !1, line: 6, type: !13, scopeLine: 6, flags: DIFlagPrototyped, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !0, retainedNodes: !115) + !115 = !{!116} + !116 = !DILocalVariable(name: "myVar", scope: !118, file: !1, line: 7, type: !6) + !117 = !DILocation(line: 10, scope: !118) + !118 = distinct !DILexicalBlock(scope: !112, file: !1, line: 1, column: 1) + !119 = distinct !DILexicalBlock(scope: !112, file: !1, line: 1, column: 1) + !120 = !DILocation(line: 10, scope: !119) ... --- @@ -136,3 +156,44 @@ body: | bb.6: RET 0, debug-location !17 +... +--- +name: test2 +debugValueSubstitutions: + - { srcinst: 4, srcop: 0, dstinst: 3, dstop: 0, subreg: 0 } +body: | + bb.0.entry: + successors: %bb.1, %bb.5, %bb.6 + + $rax = MOV64ri 1, debug-instr-number 1, debug-location !117 + JCC_1 %bb.5, 1, implicit $eflags + JCC_1 %bb.6, 2, implicit $eflags + JMP_1 %bb.1 + + bb.1: + successors: %bb.2, %bb.3 + + DBG_VALUE $noreg, $noreg, !116, !DIExpression(), debug-location !117 + JCC_1 %bb.3, 1, implicit $eflags, debug-location !117 + + bb.2: + successors: %bb.4 + + JMP_1 %bb.4, debug-location !120 + + bb.3: + successors: %bb.4 + + JMP_1 %bb.4, debug-location !117 + + bb.4: + successors: %bb.5, %bb.6 + + JCC_1 %bb.5, 1, implicit $eflags, debug-location !117 + JMP_1 %bb.6, debug-location !117 + + bb.5: + RET 0, debug-location !117 + + bb.6: + RET 0, debug-location !117 From 543134b140d9f4f3b36dfa8bbe41025d8f66d9c5 Mon Sep 17 00:00:00 2001 From: Jeremy Morse Date: Wed, 2 Feb 2022 19:17:42 +0000 Subject: [PATCH 7/7] [DebugInfo] Re-enable instruction referencing for x86_64 After discussion in D116821 this was turned off in 74db5c8c95e, 14aaaa12366f7 applied to limit the maximum memory consumption in rare conditions, plus some performance patches. (cherry picked from commit 6e03a68b776dc06826dacbdab26d24a90bb2173b) --- llvm/lib/CodeGen/LiveDebugValues/LiveDebugValues.cpp | 5 +++++ llvm/test/DebugInfo/X86/instr-ref-flag.ll | 4 ---- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/llvm/lib/CodeGen/LiveDebugValues/LiveDebugValues.cpp b/llvm/lib/CodeGen/LiveDebugValues/LiveDebugValues.cpp index 8f697611a82c0..40770b15aa35b 100644 --- a/llvm/lib/CodeGen/LiveDebugValues/LiveDebugValues.cpp +++ b/llvm/lib/CodeGen/LiveDebugValues/LiveDebugValues.cpp @@ -123,6 +123,11 @@ bool LiveDebugValues::runOnMachineFunction(MachineFunction &MF) { } bool llvm::debuginfoShouldUseDebugInstrRef(const Triple &T) { + // Enable by default on x86_64, disable if explicitly turned off on cmdline. + if (T.getArch() == llvm::Triple::x86_64 && + ValueTrackingVariableLocations != cl::boolOrDefault::BOU_FALSE) + return true; + // Enable if explicitly requested on command line. return ValueTrackingVariableLocations == cl::boolOrDefault::BOU_TRUE; } diff --git a/llvm/test/DebugInfo/X86/instr-ref-flag.ll b/llvm/test/DebugInfo/X86/instr-ref-flag.ll index 56d34aedabd02..f9d5f99edf77f 100644 --- a/llvm/test/DebugInfo/X86/instr-ref-flag.ll +++ b/llvm/test/DebugInfo/X86/instr-ref-flag.ll @@ -13,10 +13,6 @@ ;; by llc by default, and that it can be turned explicitly on or off as ;; desired. -;; Xfail due to faults found in the discussion on -;; https://reviews.llvm.org/D116821 -; XFAIL: * - ; INSTRREFON: DBG_INSTR_REF ; INSTRREFOFF: DBG_VALUE