Re-apply 3fab2d1, now with a triple added

Was reverted in 1c1b670 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

Author: jmorse

llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.cpp

@@ -148,6 +148,20 @@ static cl::opt<bool> 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<unsigned>
+    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<SpillLocationNo> 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<SpillLocationNo>
 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<LocIdx> InstrRefBasedLDV::findLocationForMemOperand(const MachineInstr &MI) {
-  SpillLocationNo SpillLoc =  extractSpillBaseRegAndOffset(MI);
+Optional<LocIdx>
+InstrRefBasedLDV::findLocationForMemOperand(const MachineInstr &MI) {
+  Optional<SpillLocationNo> 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<LocIdx> 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<SpillLocationNo> 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<ValueIDNum> Result = None;
     Optional<LocIdx> 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<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));
+      }
     }
   }
 
@@ -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<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);
+      }
     }
   }
 }
@@ -1438,23 +1468,24 @@ void InstrRefBasedLDV::performCopy(Register SrcRegNum, Register DstRegNum) {
   }
 }
 
-bool InstrRefBasedLDV::isSpillInstruction(const MachineInstr &MI,
-                                          MachineFunction *MF) {
+Optional<SpillLocationNo>
+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<SpillLocationNo> 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<LocIdx> 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<SpillLocationNo> OptLoc = isRestoreInstruction(MI, MF, Reg);
-    if (!OptLoc)
+    Optional<SpillLocationNo> 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;

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<SpillLocationNo> 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<SpillLocationNo> 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<SpillLocationNo>
+  extractSpillBaseRegAndOffset(const MachineInstr &MI);
 
   /// Observe a single instruction while stepping through a block.
   void process(MachineInstr &MI, ValueIDNum **MLiveOuts = nullptr,

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
+...