Revert "[DebugInfo][InstrRef] Add a max-stack-slots-to-track cut-out"

This reverts commit 3fab2d138e.

Breaking PPC sanitizer build:
https://lab.llvm.org/buildbot/#/builders/105/builds/20857

llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.cpp

@@ -148,20 +148,6 @@ 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.
@@ -771,15 +757,9 @@ void MLocTracker::writeRegMask(const MachineOperand *MO, unsigned CurBB,
   Masks.push_back(std::make_pair(MO, InstID));
 }
 
-Optional<SpillLocationNo> MLocTracker::getOrTrackSpillLoc(SpillLoc L) {
+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));
@@ -918,7 +898,7 @@ bool InstrRefBasedLDV::isCalleeSaved(LocIdx L) const {
 // void InstrRefBasedLDV::printVarLocInMBB(..)
 #endif
 
-Optional<SpillLocationNo>
+SpillLocationNo
 InstrRefBasedLDV::extractSpillBaseRegAndOffset(const MachineInstr &MI) {
   assert(MI.hasOneMemOperand() &&
          "Spill instruction does not have exactly one memory operand?");
@@ -933,11 +913,8 @@ InstrRefBasedLDV::extractSpillBaseRegAndOffset(const MachineInstr &MI) {
   return MTracker->getOrTrackSpillLoc({Reg, Offset});
 }
 
-Optional<LocIdx>
-InstrRefBasedLDV::findLocationForMemOperand(const MachineInstr &MI) {
-  Optional<SpillLocationNo> SpillLoc = extractSpillBaseRegAndOffset(MI);
-  if (!SpillLoc)
-    return None;
+Optional<LocIdx> InstrRefBasedLDV::findLocationForMemOperand(const MachineInstr &MI) {
+  SpillLocationNo SpillLoc =  extractSpillBaseRegAndOffset(MI);
 
   // 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
@@ -953,7 +930,7 @@ InstrRefBasedLDV::findLocationForMemOperand(const MachineInstr &MI) {
     // 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);
 }
 
@@ -1274,12 +1251,7 @@ bool InstrRefBasedLDV::transferDebugPHI(MachineInstr &MI) {
     Register Base;
     StackOffset Offs = TFI->getFrameIndexReference(*MI.getMF(), FI, Base);
     SpillLoc SL = {Base, Offs};
-    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;
+    SpillLocationNo SpillNo = MTracker->getOrTrackSpillLoc(SL);
 
     // 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
@@ -1291,7 +1263,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
@@ -1308,7 +1280,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);
     }
@@ -1385,12 +1357,11 @@ void InstrRefBasedLDV::transferRegisterDef(MachineInstr &MI) {
 
   // If this instruction writes to a spill slot, def that slot.
   if (hasFoldedStackStore(MI)) {
-    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));
-      }
+    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));
     }
   }
 
@@ -1427,12 +1398,11 @@ void InstrRefBasedLDV::transferRegisterDef(MachineInstr &MI) {
 
   // Tell TTracker about any folded stack store.
   if (hasFoldedStackStore(MI)) {
-    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);
-      }
+    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);
     }
   }
 }
@@ -1468,24 +1438,23 @@ void InstrRefBasedLDV::performCopy(Register SrcRegNum, Register DstRegNum) {
   }
 }
 
-Optional<SpillLocationNo>
-InstrRefBasedLDV::isSpillInstruction(const MachineInstr &MI,
-                                     MachineFunction *MF) {
+bool InstrRefBasedLDV::isSpillInstruction(const MachineInstr &MI,
+                                          MachineFunction *MF) {
   // TODO: Handle multiple stores folded into one.
   if (!MI.hasOneMemOperand())
-    return None;
+    return false;
 
   // 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 None;
+    return false;
 
   if (!MI.getSpillSize(TII) && !MI.getFoldedSpillSize(TII))
-    return None; // This is not a spill instruction, since no valid size was
-                 // returned from either function.
+    return false; // This is not a spill instruction, since no valid size was
+                  // returned from either function.
 
-  return extractSpillBaseRegAndOffset(MI);
+  return true;
 }
 
 bool InstrRefBasedLDV::isLocationSpill(const MachineInstr &MI,
@@ -1542,11 +1511,13 @@ 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 (Optional<SpillLocationNo> Loc = isSpillInstruction(MI, MF)) {
+  if (isSpillInstruction(MI, MF)) {
+    SpillLocationNo Loc = extractSpillBaseRegAndOffset(MI);
+
     // 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;
@@ -1564,9 +1535,7 @@ bool InstrRefBasedLDV::transferSpillOrRestoreInst(MachineInstr &MI) {
 
   // Try to recognise spill and restore instructions that may transfer a value.
   if (isLocationSpill(MI, MF, Reg)) {
-    // isLocationSpill returning true should guarantee we can extract a
-    // location.
-    SpillLocationNo Loc = *extractSpillBaseRegAndOffset(MI);
+    SpillLocationNo Loc = extractSpillBaseRegAndOffset(MI);
 
     auto DoTransfer = [&](Register SrcReg, unsigned SpillID) {
       auto ReadValue = MTracker->readReg(SrcReg);
@@ -1593,9 +1562,10 @@ bool InstrRefBasedLDV::transferSpillOrRestoreInst(MachineInstr &MI) {
     unsigned SpillID = MTracker->getLocID(Loc, {Size, 0});
     DoTransfer(Reg, SpillID);
   } else {
-    Optional<SpillLocationNo> Loc = isRestoreInstruction(MI, MF, Reg);
-    if (!Loc)
+    Optional<SpillLocationNo> OptLoc = isRestoreInstruction(MI, MF, Reg);
+    if (!OptLoc)
       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
@@ -1622,13 +1592,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,9 +616,7 @@ public:
   void writeRegMask(const MachineOperand *MO, unsigned CurBB, unsigned InstID);
 
   /// Find LocIdx for SpillLoc \p L, creating a new one if it's not tracked.
-  /// 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);
+  SpillLocationNo getOrTrackSpillLoc(SpillLoc L);
 
   // Get LocIdx of a spill ID.
   LocIdx getSpillMLoc(unsigned SpillID) {
@@ -875,8 +873,7 @@ private:
   StringRef StackProbeSymbolName;
 
   /// Tests whether this instruction is a spill to a stack slot.
-  Optional<SpillLocationNo> isSpillInstruction(const MachineInstr &MI,
-                                               MachineFunction *MF);
+  bool 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:
@@ -894,8 +891,7 @@ private:
 
   /// Given a spill instruction, extract the spill slot information, ensure it's
   /// tracked, and return the spill number.
-  Optional<SpillLocationNo>
-  extractSpillBaseRegAndOffset(const MachineInstr &MI);
+  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

@@ -1,88 +0,0 @@
-# RUN: llc %s -o - -experimental-debug-variable-locations \
-# 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:     -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
-...

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}).