[DebugInfo][6/N][InstrRef] Ignore stack-pointer clobbers on call instructions even more

[jmorse]

Summary

Avoid un-necessarily recreating DBG_VALUEs on call instructions.

In LiveDebugValues, we choose to ignore any clobbers of the stack pointer by call instructions, because they're irrelevant to our model of the machine. Otherwise, SP based locations could be terminated un-necessarily. In InstrRefBasedLDV::transferRegisterDef, we correctly ignore such clobbers when updating what values are in which registers (MTracker), but we tell the block variable location tracker (TTracker) that the stack pointer has been clobbered. This caused the modified test to fail as an un-necessary DBG_VALUE was created.

This patch just corrects what we're already trying to do: as well as ignoring stack-pointer clobbers by calls for register tracking, also ignore them for variable location tracking, and don't report the clobber to TTracker.

[jmorse]

(Add a CHECK-NOT: line to actually ensure that this behaviour doesn't regress)

[Orlando]

Based on our discussion in D114362, is it not worth preserving the -experimental-debug-variable-locations=false path for llvm/test/DebugInfo/X86/dbg-addr.ll? Otherwise LGTM.

llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.cpp

1349

nit: full stop removed from comment?

[jmorse]

In D114365#3151198, @Orlando wrote:

Based on our discussion in D114362, is it not worth preserving the -experimental-debug-variable-locations=false path for llvm/test/DebugInfo/X86/dbg-addr.ll? Otherwise LGTM.

I guess so, will fold that in, cheers

llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.cpp

if (MI.isImplicitDef()) {
     ValueIDNum Num = MTracker->readReg(MI.getOperand(0).getReg());
     // Has a legitimate value -> ignore the implicit def.
     if (Num.getLoc() != 0)
       return;
     // Otherwise, def it here.
   } else if (MI.isMetaInstruction())
     return;
 
+  auto IgnoreSPAlias = [this, &MI](Register R) -> bool {
+    return MI.isCall() && MTracker->SPAliases.count(R);
+  };
+
   // Find the regs killed by MI, and find regmasks of preserved regs.
   // Max out the number of statically allocated elements in `DeadRegs`, as this
   // prevents fallback to std::set::count() operations.
   SmallSet<uint32_t, 32> DeadRegs;
   SmallVector<const uint32_t *, 4> RegMasks;
   SmallVector<const MachineOperand *, 4> RegMaskPtrs;
   for (const MachineOperand &MO : MI.operands()) {
     // Determine whether the operand is a register def.
     if (MO.isReg() && MO.isDef() && MO.getReg() &&
         Register::isPhysicalRegister(MO.getReg()) &&
-        !(MI.isCall() && MTracker->SPAliases.count(MO.getReg()))) {
+        !IgnoreSPAlias(MO.getReg())) {
       // Remove ranges of all aliased registers.
       for (MCRegAliasIterator RAI(MO.getReg(), TRI, true); RAI.isValid(); ++RAI)
         // FIXME: Can we break out of this loop early if no insertion occurs?
         DeadRegs.insert(*RAI);
     } else if (MO.isRegMask()) {
       RegMasks.push_back(MO.getRegMask());
       RegMaskPtrs.push_back(&MO);
     }
void InstrRefBasedLDV::transferRegisterDef(MachineInstr &MI) {
   for (uint32_t DeadReg : DeadRegs) {
     LocIdx Loc = MTracker->lookupOrTrackRegister(DeadReg);
     TTracker->clobberMloc(Loc, MI.getIterator(), false);
   }
 
   // Look for any clobbers performed by a register mask. Only test locations
   // that are actually being tracked.
   for (auto L : MTracker->locations()) {
     // Stack locations can't be clobbered by regmasks.
     if (MTracker->isSpill(L.Idx))
       continue;
 
     Register Reg = MTracker->LocIdxToLocID[L.Idx];
+    if (IgnoreSPAlias(Reg))
+      continue;
+
     for (auto *MO : RegMaskPtrs)
       if (MO->clobbersPhysReg(Reg))
         TTracker->clobberMloc(L.Idx, MI.getIterator(), false);
   }
 
   // Tell TTracker about any folded stack store.
   if (hasFoldedStackStore(MI)) {
     SpillLocationNo SpillNo = extractSpillBaseRegAndOffset(MI);

llvm/test/DebugInfo/X86/dbg-addr.ll

+;; Run twice -- once with DBG_VALUEs, once with instruction referencing.
 ; RUN: llc %s -o %t.s -experimental-debug-variable-locations=false
 ; RUN: llvm-mc -triple x86_64--linux %t.s -filetype=obj -o %t.o
 ; RUN: FileCheck < %t.s %s
 ; RUN: llvm-dwarfdump %t.o | FileCheck %s --check-prefix=DWARF
+; RUN: llc %s -o %t.s -experimental-debug-variable-locations=true
+; RUN: llvm-mc -triple x86_64--linux %t.s -filetype=obj -o %t.o
+; RUN: FileCheck < %t.s %s
+; RUN: llvm-dwarfdump %t.o | FileCheck %s --check-prefix=DWARF
+
 
 ; Unlike dbg.declare, dbg.addr should be lowered to DBG_VALUE instructions. It
 ; is control-dependent.
 
 ; CHECK-LABEL: use_dbg_addr:
 ; CHECK: #DEBUG_VALUE: use_dbg_addr:o <- [$rsp+0]
+; CHECK-NOT: #DEBUG_VALUE:
 
 ; DWARF: DW_TAG_variable
 ; DWARF-NEXT:              DW_AT_location (DW_OP_fbreg +0)
 ; DWARF-NEXT:              DW_AT_name ("o")
 
 
 ; ModuleID = 't.c'
 source_filename = "t.c"