[DebugInfo] Handle stack slot offsets for spilled sub-registers in LDV

[dstenb]

Summary

Extend LDV so that stack slot offsets for spilled sub-registers
are added to the emitted debug locations. This is accomplished
by querying InstrInfo::getStackSlotRange().

With this change, LDV will add a DW_OP_plus_uconst operation to
the expression if a sub-register is spilled. Later on, PEI will
add an offset operation for the stack slot, meaning that we will
get expressions of the forms:

• {DW_OP_constu #fp-offset, DW_OP_minus, DW_OP_plus_uconst #subreg-offset}
• {DW_OP_plus_const #fp-offset, DW_OP_minus, DW_OP_plus_uconst #subreg-offset}

The two offset operations should ideally be merged.

[aprantl]

The best place to do the simplification / constant-folding in the DIExpresssion is in DwarfExpression.cpp, just before it gets emitted.

lib/CodeGen/LiveDebugVariables.cpp

1222 ↗

(On Diff #163761)

The DW_OP_deref is not already part of the expression if Loc.wasIndirect()?

test/CodeGen/PowerPC/live-debug-vars-subreg-offset.ll

43 ↗

(On Diff #163761)

Nice testcase! You can reduce it even further by setting all of the !dbg locations to the same location.

[dstenb]

In D51612#1223266, @aprantl wrote:

The best place to do the simplification / constant-folding in the DIExpresssion is in DwarfExpression.cpp, just before it gets emitted.

Okay! I'll look into putting together a separate patch for that.

lib/CodeGen/LiveDebugVariables.cpp

1222 ↗

(On Diff #163761)

As far as I have understood it, the locations that are covered by "WasIndirect" are the indirect (reg+offset) DBG_VALUEs, which if they are not spilled are as the indirect DW_OP_*breg* operations, so they will not have a DW_OP_deref beforehand.

test/CodeGen/PowerPC/live-debug-vars-subreg-offset.ll

43 ↗

(On Diff #163761)

Good idea!

[dstenb]

lib/CodeGen/LiveDebugVariables.cpp

1222 ↗

(On Diff #163761)

s/spilled are as/will be emitted as/

[dstenb]

Removed unnecessary !dbg locations in the test. I also moved the file to test/DebugInfo/.

[aprantl]

Thanks.

[dstenb]

Thanks for the review! I'll submit this tomorrow when I have time to track the build bots.

llvm/trunk/lib/CodeGen/LiveDebugVariables.cpp

 private:
   unsigned LocNo : 31;
   unsigned WasIndirect : 1;
 };
 
 /// LocMap - Map of where a user value is live, and its location.
 using LocMap = IntervalMap<SlotIndex, DbgValueLocation, 4>;
 
+/// SpillOffsetMap - Map of stack slot offsets for spilled locations.
+/// Non-spilled locations are not added to the map.
+using SpillOffsetMap = DenseMap<unsigned, unsigned>;
+
 namespace {
 
 class LDVImpl;
 
 /// UserValue - A user value is a part of a debug info user variable.
 ///
 /// A DBG_VALUE instruction notes that (a sub-register of) a virtual register
 /// holds part of a user variable. The part is identified by a byte offset.
class UserValue {
   LocMap locInts;
 
   /// Set of interval start indexes that have been trimmed to the
   /// lexical scope.
   SmallSet<SlotIndex, 2> trimmedDefs;
 
   /// insertDebugValue - Insert a DBG_VALUE into MBB at Idx for LocNo.
   void insertDebugValue(MachineBasicBlock *MBB, SlotIndex StartIdx,
-                        SlotIndex StopIdx,
-                        DbgValueLocation Loc, bool Spilled, LiveIntervals &LIS,
+                        SlotIndex StopIdx, DbgValueLocation Loc, bool Spilled,
+                        unsigned SpillOffset, LiveIntervals &LIS,
                         const TargetInstrInfo &TII,
                         const TargetRegisterInfo &TRI);
 
   /// splitLocation - Replace OldLocNo ranges with NewRegs ranges where NewRegs
   /// is live. Returns true if any changes were made.
   bool splitLocation(unsigned OldLocNo, ArrayRef<unsigned> NewRegs,
                      LiveIntervals &LIS);
 
void computeIntervals(MachineRegisterInfo &MRI, const TargetRegisterInfo &TRI,
                         LiveIntervals &LIS, LexicalScopes &LS);
 
   /// splitRegister - Replace OldReg ranges with NewRegs ranges where NewRegs is
   /// live. Returns true if any changes were made.
   bool splitRegister(unsigned OldReg, ArrayRef<unsigned> NewRegs,
                      LiveIntervals &LIS);
 
   /// rewriteLocations - Rewrite virtual register locations according to the
-  /// provided virtual register map. Record which locations were spilled.
-  void rewriteLocations(VirtRegMap &VRM, const TargetRegisterInfo &TRI,
-                        BitVector &SpilledLocations);
+  /// provided virtual register map. Record the stack slot offsets for the
+  /// locations that were spilled.
+  void rewriteLocations(VirtRegMap &VRM, const MachineFunction &MF,
+                        const TargetInstrInfo &TII,
+                        const TargetRegisterInfo &TRI,
+                        SpillOffsetMap &SpillOffsets);
 
   /// emitDebugValues - Recreate DBG_VALUE instruction from data structures.
   void emitDebugValues(VirtRegMap *VRM, LiveIntervals &LIS,
                        const TargetInstrInfo &TII,
                        const TargetRegisterInfo &TRI,
-                       const BitVector &SpilledLocations);
+                       const SpillOffsetMap &SpillOffsets);
 
   /// getDebugLoc - Return DebugLoc of this UserValue.
   DebugLoc getDebugLoc() { return dl;}
 
   void print(raw_ostream &, const TargetRegisterInfo *);
 };
 
 /// LDVImpl - Implementation of the LiveDebugVariables pass.

 }
 
 void LiveDebugVariables::
 splitRegister(unsigned OldReg, ArrayRef<unsigned> NewRegs, LiveIntervals &LIS) {
   if (pImpl)
     static_cast<LDVImpl*>(pImpl)->splitRegister(OldReg, NewRegs);
 }
 
-void UserValue::rewriteLocations(VirtRegMap &VRM, const TargetRegisterInfo &TRI,
-                                 BitVector &SpilledLocations) {
+void UserValue::rewriteLocations(VirtRegMap &VRM, const MachineFunction &MF,
+                                 const TargetInstrInfo &TII,
+                                 const TargetRegisterInfo &TRI,
+                                 SpillOffsetMap &SpillOffsets) {
   // Build a set of new locations with new numbers so we can coalesce our
   // IntervalMap if two vreg intervals collapse to the same physical location.
   // Use MapVector instead of SetVector because MapVector::insert returns the
   // position of the previously or newly inserted element. The boolean value
   // tracks if the location was produced by a spill.
   // FIXME: This will be problematic if we ever support direct and indirect
   // frame index locations, i.e. expressing both variables in memory and
   // 'int x, *px = &x'. The "spilled" bit must become part of the location.
-  MapVector<MachineOperand, bool> NewLocations;
+  MapVector<MachineOperand, std::pair<bool, unsigned>> NewLocations;
   SmallVector<unsigned, 4> LocNoMap(locations.size());
   for (unsigned I = 0, E = locations.size(); I != E; ++I) {
     bool Spilled = false;
+    unsigned SpillOffset = 0;
     MachineOperand Loc = locations[I];
     // Only virtual registers are rewritten.
     if (Loc.isReg() && Loc.getReg() &&
         TargetRegisterInfo::isVirtualRegister(Loc.getReg())) {
       unsigned VirtReg = Loc.getReg();
       if (VRM.isAssignedReg(VirtReg) &&
           TargetRegisterInfo::isPhysicalRegister(VRM.getPhys(VirtReg))) {
         // This can create a %noreg operand in rare cases when the sub-register
         // index is no longer available. That means the user value is in a
         // non-existent sub-register, and %noreg is exactly what we want.
         Loc.substPhysReg(VRM.getPhys(VirtReg), TRI);
       } else if (VRM.getStackSlot(VirtReg) != VirtRegMap::NO_STACK_SLOT) {
-        // FIXME: Translate SubIdx to a stackslot offset.
+        // Retrieve the stack slot offset.
+        unsigned SpillSize;
+        const MachineRegisterInfo &MRI = MF.getRegInfo();
+        const TargetRegisterClass *TRC = MRI.getRegClass(VirtReg);
+        bool Success = TII.getStackSlotRange(TRC, Loc.getSubReg(), SpillSize,
+                                             SpillOffset, MF);
+
+        // FIXME: Invalidate the location if the offset couldn't be calculated.
+        (void)Success;
+
         Loc = MachineOperand::CreateFI(VRM.getStackSlot(VirtReg));
         Spilled = true;
       } else {
         Loc.setReg(0);
         Loc.setSubReg(0);
       }
     }
 
     // Insert this location if it doesn't already exist and record a mapping
     // from the old number to the new number.
-    auto InsertResult = NewLocations.insert({Loc, Spilled});
+    auto InsertResult = NewLocations.insert({Loc, {Spilled, SpillOffset}});
     unsigned NewLocNo = std::distance(NewLocations.begin(), InsertResult.first);
     LocNoMap[I] = NewLocNo;
   }
 
-  // Rewrite the locations and record which ones were spill slots.
+  // Rewrite the locations and record the stack slot offsets for spills.
   locations.clear();
-  SpilledLocations.clear();
-  SpilledLocations.resize(NewLocations.size());
+  SpillOffsets.clear();
   for (auto &Pair : NewLocations) {
+    bool Spilled;
+    unsigned SpillOffset;
+    std::tie(Spilled, SpillOffset) = Pair.second;
     locations.push_back(Pair.first);
-    if (Pair.second) {
+    if (Spilled) {
       unsigned NewLocNo = std::distance(&*NewLocations.begin(), &Pair);
-      SpilledLocations.set(NewLocNo);
+      SpillOffsets[NewLocNo] = SpillOffset;
     }
   }
 
   // Update the interval map, but only coalesce left, since intervals to the
   // right use the old location numbers. This should merge two contiguous
   // DBG_VALUE intervals with different vregs that were allocated to the same
   // physical register.
   for (LocMap::iterator I = locInts.begin(); I.valid(); ++I) {
if (I->definesRegister(Reg, &TRI))
       // The insert location is directly after the instruction/bundle.
       return std::next(I);
     ++I;
   }
   return MBB->end();
 }
 
 void UserValue::insertDebugValue(MachineBasicBlock *MBB, SlotIndex StartIdx,
-                                 SlotIndex StopIdx,
-                                 DbgValueLocation Loc, bool Spilled,
-                                 LiveIntervals &LIS,
-                                 const TargetInstrInfo &TII,
+                                 SlotIndex StopIdx, DbgValueLocation Loc,
+                                 bool Spilled, unsigned SpillOffset,
+                                 LiveIntervals &LIS, const TargetInstrInfo &TII,
                                  const TargetRegisterInfo &TRI) {
   SlotIndex MBBEndIdx = LIS.getMBBEndIdx(&*MBB);
   // Only search within the current MBB.
   StopIdx = (MBBEndIdx < StopIdx) ? MBBEndIdx : StopIdx;
   MachineBasicBlock::iterator I = findInsertLocation(MBB, StartIdx, LIS);
   // Undef values don't exist in locations so create new "noreg" register MOs
   // for them. See getLocationNo().
   MachineOperand MO = !Loc.isUndef() ?
     locations[Loc.locNo()] :
     MachineOperand::CreateReg(/* Reg */ 0, /* isDef */ false, /* isImp */ false,
                               /* isKill */ false, /* isDead */ false,
                               /* isUndef */ false, /* isEarlyClobber */ false,
                               /* SubReg */ 0, /* isDebug */ true);
 
   ++NumInsertedDebugValues;
 
   assert(cast<DILocalVariable>(Variable)
              ->isValidLocationForIntrinsic(getDebugLoc()) &&
          "Expected inlined-at fields to agree");
 
   // If the location was spilled, the new DBG_VALUE will be indirect. If the
   // original DBG_VALUE was indirect, we need to add DW_OP_deref to indicate
-  // that the original virtual register was a pointer.
+  // that the original virtual register was a pointer. Also, add the stack slot
+  // offset for the spilled register to the expression.
   const DIExpression *Expr = Expression;
   bool IsIndirect = Loc.wasIndirect();
   if (Spilled) {
-    if (IsIndirect)
-      Expr = DIExpression::prepend(Expr, DIExpression::WithDeref);
+    auto Deref = IsIndirect ? DIExpression::WithDeref : DIExpression::NoDeref;
+    Expr =
+        DIExpression::prepend(Expr, DIExpression::NoDeref, SpillOffset, Deref);
     IsIndirect = true;
   }
 
   assert((!Spilled || MO.isFI()) && "a spilled location must be a frame index");
 
   do {
     BuildMI(*MBB, I, getDebugLoc(), TII.get(TargetOpcode::DBG_VALUE),
             IsIndirect, MO, Variable, Expr);
 
     // Continue and insert DBG_VALUES after every redefinition of register
     // associated with the debug value within the range
     I = findNextInsertLocation(MBB, I, StopIdx, MO, LIS, TRI);
   } while (I != MBB->end());
 }
 
 void UserValue::emitDebugValues(VirtRegMap *VRM, LiveIntervals &LIS,
                                 const TargetInstrInfo &TII,
                                 const TargetRegisterInfo &TRI,
-                                const BitVector &SpilledLocations) {
+                                const SpillOffsetMap &SpillOffsets) {
   MachineFunction::iterator MFEnd = VRM->getMachineFunction().end();
 
   for (LocMap::const_iterator I = locInts.begin(); I.valid();) {
     SlotIndex Start = I.start();
     SlotIndex Stop = I.stop();
     DbgValueLocation Loc = I.value();
-    bool Spilled = !Loc.isUndef() ? SpilledLocations.test(Loc.locNo()) : false;
+    auto SpillIt =
+        !Loc.isUndef() ? SpillOffsets.find(Loc.locNo()) : SpillOffsets.end();
+    bool Spilled = SpillIt != SpillOffsets.end();
+    unsigned SpillOffset = Spilled ? SpillIt->second : 0;
 
     // If the interval start was trimmed to the lexical scope insert the
     // DBG_VALUE at the previous index (otherwise it appears after the
     // first instruction in the range).
     if (trimmedDefs.count(Start))
       Start = Start.getPrevIndex();
 
     LLVM_DEBUG(dbgs() << "\t[" << Start << ';' << Stop << "):" << Loc.locNo());
     MachineFunction::iterator MBB = LIS.getMBBFromIndex(Start)->getIterator();
     SlotIndex MBBEnd = LIS.getMBBEndIdx(&*MBB);
 
     LLVM_DEBUG(dbgs() << ' ' << printMBBReference(*MBB) << '-' << MBBEnd);
-    insertDebugValue(&*MBB, Start, Stop, Loc, Spilled, LIS, TII, TRI);
+    insertDebugValue(&*MBB, Start, Stop, Loc, Spilled, SpillOffset, LIS, TII,
+                     TRI);
     // This interval may span multiple basic blocks.
     // Insert a DBG_VALUE into each one.
     while (Stop > MBBEnd) {
       // Move to the next block.
       Start = MBBEnd;
       if (++MBB == MFEnd)
         break;
       MBBEnd = LIS.getMBBEndIdx(&*MBB);
       LLVM_DEBUG(dbgs() << ' ' << printMBBReference(*MBB) << '-' << MBBEnd);
-      insertDebugValue(&*MBB, Start, Stop, Loc, Spilled, LIS, TII, TRI);
+      insertDebugValue(&*MBB, Start, Stop, Loc, Spilled, SpillOffset, LIS, TII,
+                       TRI);
     }
     LLVM_DEBUG(dbgs() << '\n');
     if (MBB == MFEnd)
       break;
 
     ++I;
   }
 }
 
 void LDVImpl::emitDebugValues(VirtRegMap *VRM) {
   LLVM_DEBUG(dbgs() << "********** EMITTING LIVE DEBUG VARIABLES **********\n");
   if (!MF)
     return;
   const TargetInstrInfo *TII = MF->getSubtarget().getInstrInfo();
-  BitVector SpilledLocations;
+  SpillOffsetMap SpillOffsets;
   for (unsigned i = 0, e = userValues.size(); i != e; ++i) {
     LLVM_DEBUG(userValues[i]->print(dbgs(), TRI));
-    userValues[i]->rewriteLocations(*VRM, *TRI, SpilledLocations);
-    userValues[i]->emitDebugValues(VRM, *LIS, *TII, *TRI, SpilledLocations);
+    userValues[i]->rewriteLocations(*VRM, *MF, *TII, *TRI, SpillOffsets);
+    userValues[i]->emitDebugValues(VRM, *LIS, *TII, *TRI, SpillOffsets);
   }
   EmitDone = true;
 }
 
 void LiveDebugVariables::emitDebugValues(VirtRegMap *VRM) {
   if (pImpl)
     static_cast<LDVImpl*>(pImpl)->emitDebugValues(VRM);
 }

llvm/trunk/test/DebugInfo/PowerPC/live-debug-vars-subreg-offset.ll

+; RUN: llc -O3 -stop-after virtregrewriter %s -o - | FileCheck %s
+
+target datalayout = "E-m:e-i64:64-n32:64"
+target triple = "ppc64"
+
+; Verify that live-debug-variables includes the stack slot offset
+; for the sub-register in the debug expression for `spilled'.
+
+; This reproducer was generated from the following C file:
+;
+;     extern int foo(void);
+;     extern void bar(int);
+;
+;     int main() {
+;       int spilled = foo();
+;
+;       // Clobber all GPRs.
+;       __asm volatile ("" : : : "0", "1", "2", "3", "4", "5", "6", "7",
+;                                "8", "9", "10", "11", "12", "13", "14", "15",
+;                                "16", "17", "18", "19", "20", "21", "22", "23",
+;                                "24", "25", "26", "27", "28", "29", "30", "31");
+;
+;       for(;;)
+;         bar(spilled);
+;
+;       return 0;
+;     }
+;
+; compiled using:
+;
+;     clang --target=ppc64 -O3 -g -S -emit-llvm
+
+; CHECK: ![[VAR:.*]] = !DILocalVariable(name: "spilled"
+
+; CHECK: STD $x3, 0, %stack.0
+; CHECK-NEXT: DBG_VALUE %stack.0, 0, ![[VAR]], !DIExpression(DW_OP_plus_uconst, 4)
+
+; Function Attrs: noreturn nounwind
+define signext i32 @main() local_unnamed_addr #0 !dbg !8 {
+entry:
+  %call = tail call signext i32 @foo() #2, !dbg !14
+  call void @llvm.dbg.value(metadata i32 %call, metadata !13, metadata !DIExpression()), !dbg !14
+  tail call void asm sideeffect "", "~{r0},~{r1},~{r2},~{r3},~{r4},~{r5},~{r6},~{r7},~{r8},~{r9},~{r10},~{r11},~{r12},~{r13},~{r14},~{r15},~{r16},~{r17},~{r18},~{r19},~{r20},~{r21},~{r22},~{r23},~{r24},~{r25},~{r26},~{r27},~{r28},~{r29},~{r30},~{r31}"() #2, !dbg !14, !srcloc !15
+  br label %for.cond, !dbg !14
+
+for.cond:                                         ; preds = %for.cond, %entry
+  tail call void @bar(i32 signext %call) #2, !dbg !14
+  br label %for.cond, !dbg !14
+}
+
+declare signext i32 @foo() local_unnamed_addr
+
+declare void @bar(i32 signext) local_unnamed_addr
+
+; Function Attrs: nounwind readnone speculatable
+declare void @llvm.dbg.value(metadata, metadata, metadata) #1
+
+attributes #0 = { noreturn nounwind }
+attributes #1 = { nounwind readnone speculatable }
+attributes #2 = { nounwind }
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!3, !4, !5, !6}
+!llvm.ident = !{!7}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C99, file: !1, producer: "clang version 8.0.0", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, enums: !2, nameTableKind: None)
+!1 = !DIFile(filename: "live-debug-vars-subreg-offset.c", directory: "/")
+!2 = !{}
+!3 = !{i32 2, !"Dwarf Version", i32 4}
+!4 = !{i32 2, !"Debug Info Version", i32 3}
+!5 = !{i32 1, !"wchar_size", i32 4}
+!6 = !{i32 7, !"PIC Level", i32 2}
+!7 = !{!"clang version 8.0.0"}
+!8 = distinct !DISubprogram(name: "main", scope: !1, file: !1, line: 4, type: !9, isLocal: false, isDefinition: true, scopeLine: 4, isOptimized: true, unit: !0, retainedNodes: !12)
+!9 = !DISubroutineType(types: !10)
+!10 = !{!11}
+!11 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
+!12 = !{!13}
+!13 = !DILocalVariable(name: "spilled", scope: !8, file: !1, line: 5, type: !11)
+!14 = !DILocation(line: 5, column: 17, scope: !8)
+!15 = !{i32 125}