PR for llvm/llvm-project#53548

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) {
@@ -678,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<DebugVariable, DbgValue> Vars;
-  DenseMap<DebugVariable, const DILocation *> Scopes;
+  SmallDenseMap<DebugVariable, const DILocation *, 8> Scopes;
   MachineBasicBlock *MBB = nullptr;
   const OverlapMap &OverlappingFragments;
   DbgValueProperties EmptyProperties;
@@ -747,6 +749,11 @@ class VLocTracker {
       Scopes[Overlapped] = Loc;
     }
   }
+
+  void clear() {
+    Vars.clear();
+    Scopes.clear();
+  }
 };
 
 // XXX XXX docs
@@ -862,6 +869,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<MachineInstr *, Optional<ValueIDNum>> 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.
@@ -873,7 +886,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 +905,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,
@@ -940,6 +955,12 @@ class InstrRefBasedLDV : public LDVImpl {
                                       ValueIDNum **MLiveIns, MachineInstr &Here,
                                       uint64_t InstrNum);
 
+  Optional<ValueIDNum> 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
@@ -1050,18 +1071,6 @@ class InstrRefBasedLDV : public LDVImpl {
               const LiveIdxT &LiveOuts, ValueIDNum **MOutLocs,
               const SmallVectorImpl<const MachineBasicBlock *> &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<DebugVariable, unsigned> &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
@@ -1072,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<unsigned> &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<VLocTracker> &AllTheVLocs, MachineFunction &MF,
+      DenseMap<DebugVariable, unsigned> &AllVarsNumbering,
+      const TargetPassConfig &TPC);
+
   bool ExtendRanges(MachineFunction &MF, MachineDominatorTree *DomTree,
                     TargetPassConfig *TPC, unsigned InputBBLimit,
                     unsigned InputDbgValLimit) override;

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;
 }

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

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

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
 

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