[DebugInfo][InstrRef] Don't generate redundant DBG_PHIs

In SelectionDAG, DBG_PHI instructions are created to "read" physreg values
and give them an instruction number, when they can't be traced back to a
defining instruction. The most common scenario if arguments to a function.
Unfortunately, if you have 100 inlined methods, each of which has the same
"this" pointer, then the 100 dbg.value instructions become 100
DBG_INSTR_REFs plus 100 DBG_PHIs, where only one DBG_PHI would suffice.

This patch adds a vreg cache for MachienFunction::salvageCopySSA, if we've
already traced a value back to the start of a block and created a DBG_PHI
then it allows us to re-use the DBG_PHI, as well as reducing work.

Differential Revision: https://reviews.llvm.org/D124517

llvm/include/llvm/CodeGen/MachineFunction.h

@@ -531,8 +531,13 @@ public:
   /// the copied value; or for parameters, creates a DBG_PHI on entry.
   /// May insert instructions into the entry block!
   /// \p MI The copy-like instruction to salvage.
+  /// \p DbgPHICache A container to cache already-solved COPYs.
   /// \returns An instruction/operand pair identifying the defining value.
-  DebugInstrOperandPair salvageCopySSA(MachineInstr &MI);
+  DebugInstrOperandPair
+  salvageCopySSA(MachineInstr &MI,
+                 DenseMap<Register, DebugInstrOperandPair> &DbgPHICache);
+
+  DebugInstrOperandPair salvageCopySSAImpl(MachineInstr &MI);
 
   /// Finalise any partially emitted debug instructions. These are DBG_INSTR_REF
   /// instructions where we only knew the vreg of the value they use, not the

llvm/lib/CodeGen/MachineFunction.cpp

@@ -1033,7 +1033,32 @@ void MachineFunction::substituteDebugValuesForInst(const MachineInstr &Old,
   }
 }
 
-auto MachineFunction::salvageCopySSA(MachineInstr &MI)
+auto MachineFunction::salvageCopySSA(
+    MachineInstr &MI, DenseMap<Register, DebugInstrOperandPair> &DbgPHICache)
+    -> DebugInstrOperandPair {
+  const TargetInstrInfo &TII = *getSubtarget().getInstrInfo();
+
+  // Check whether this copy-like instruction has already been salvaged into
+  // an operand pair.
+  Register Dest;
+  if (auto CopyDstSrc = TII.isCopyInstr(MI)) {
+    Dest = CopyDstSrc->Destination->getReg();
+  } else {
+    assert(MI.isSubregToReg());
+    Dest = MI.getOperand(0).getReg();
+  }
+
+  auto CacheIt = DbgPHICache.find(Dest);
+  if (CacheIt != DbgPHICache.end())
+    return CacheIt->second;
+
+  // Calculate the instruction number to use, or install a DBG_PHI.
+  auto OperandPair = salvageCopySSAImpl(MI);
+  DbgPHICache.insert({Dest, OperandPair});
+  return OperandPair;
+}
+
+auto MachineFunction::salvageCopySSAImpl(MachineInstr &MI)
     -> DebugInstrOperandPair {
   MachineRegisterInfo &MRI = getRegInfo();
   const TargetRegisterInfo &TRI = *MRI.getTargetRegisterInfo();
@@ -1189,6 +1214,7 @@ void MachineFunction::finalizeDebugInstrRefs() {
     MI.getOperand(1).ChangeToRegister(0, false);
   };
 
+  DenseMap<Register, DebugInstrOperandPair> ArgDbgPHIs;
   for (auto &MBB : *this) {
     for (auto &MI : MBB) {
       if (!MI.isDebugRef() || !MI.getOperand(0).isReg())
@@ -1211,7 +1237,7 @@ void MachineFunction::finalizeDebugInstrRefs() {
       // instruction that defines the source value, see salvageCopySSA docs
       // for why this is important.
       if (DefMI.isCopyLike() || TII->isCopyInstr(DefMI)) {
-        auto Result = salvageCopySSA(DefMI);
+        auto Result = salvageCopySSA(DefMI, ArgDbgPHIs);
         MI.getOperand(0).ChangeToImmediate(Result.first);
         MI.getOperand(1).setImm(Result.second);
       } else {

llvm/test/DebugInfo/X86/dbg-value-funcarg.ll

@@ -122,14 +122,13 @@ define dso_local void @foo_same_param(i32 %t3a) local_unnamed_addr #0 !dbg !31 {
 ; CHECK: DBG_VALUE %0, $noreg, ![[T3A]], !DIExpression(),
 ; CHECK: TCRETURNdi64 @bar,
 ; INSTRREF-LABEL: name:            foo_same_param
-; INSTRREF: DBG_PHI $edi, 2
 ; INSTRREF: DBG_PHI $edi, 1
 ; INSTRREF: DBG_VALUE $edi, $noreg, ![[T3A]], !DIExpression(),
 ; INSTRREF: CALL64pcrel32 @bar,
 ; INSTRREF: DBG_INSTR_REF 1, 0, ![[TMP]], !DIExpression(),
 ; INSTRREF: DBG_VALUE 123, $noreg, ![[T3A]], !DIExpression(),
 ; INSTRREF: CALL64pcrel32 @bar,
-; INSTRREF: DBG_INSTR_REF 2, 0, ![[T3A]], !DIExpression(),
+; INSTRREF: DBG_INSTR_REF 1, 0, ![[T3A]], !DIExpression(),
 ; INSTRREF: TCRETURNdi64 @bar,
 entry:
   call void @llvm.dbg.value(metadata i32 %t3a, metadata !33, metadata !DIExpression()), !dbg !35

llvm/test/DebugInfo/X86/dbg-value-funcarg4.ll

@@ -0,0 +1,51 @@
+; RUN: llc -mtriple=x86_64-unknown-linux-gnu -start-after=codegenprepare -stop-before=finalize-isel -o - %s -experimental-debug-variable-locations=true | FileCheck %s --implicit-check-not=DBG_PHI
+
+; Test that for multiple dbg.values referring to the same argument, we emit a
+; single DBG_PHI and refer to it twice. (Using more than one DBG_PHI is fine,
+; but inefficient).
+
+; CHECK-DAG: ![[LOCAL:.*]] = !DILocalVariable(name: "local"
+; CHECK-DAG: ![[LOCAL2:.*]] = !DILocalVariable(name: "local2"
+
+; CHECK: DBG_PHI $edi, 1
+
+; CHECK: DBG_INSTR_REF 1, 0, ![[LOCAL]], !DIExpression(),
+; CHECK: DBG_INSTR_REF 1, 0, ![[LOCAL2]], !DIExpression(),
+
+declare void @bar(i32)
+declare void @llvm.dbg.value(metadata, metadata, metadata)
+
+define dso_local void @foo_local(i32 %t1a) local_unnamed_addr !dbg !7 {
+entry:
+  tail call void @bar(i32 %t1a) #3, !dbg !17
+  %bees = add i32 %t1a, 3
+  call void @llvm.dbg.value(metadata i32 %t1a, metadata !13, metadata !DIExpression()), !dbg !14
+  call void @llvm.dbg.value(metadata i32 %t1a, metadata !19, metadata !DIExpression()), !dbg !14
+  tail call void @bar(i32 %bees) #3, !dbg !17
+  ret void, !dbg !18
+}
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!3, !4, !5}
+!llvm.ident = !{!6}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C99, file: !1, producer: "clang", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, enums: !2, nameTableKind: None)
+!1 = !DIFile(filename: "foo.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 = !{!"clang"}
+!7 = distinct !DISubprogram(name: "foo_local", scope: !1, file: !1, line: 3, type: !8, scopeLine: 3, flags: DIFlagPrototyped, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !0, retainedNodes: !11)
+!8 = !DISubroutineType(types: !9)
+!9 = !{null, !10}
+!10 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
+!11 = !{!12, !13}
+!12 = !DILocalVariable(name: "t1a", arg: 1, scope: !7, file: !1, line: 3, type: !10)
+!13 = !DILocalVariable(name: "local", scope: !7, file: !1, line: 4, type: !10)
+!14 = !DILocation(line: 3, column: 20, scope: !7)
+!15 = !DILocation(line: 4, column: 7, scope: !7)
+!16 = !DILocation(line: 5, column: 3, scope: !7)
+!17 = !DILocation(line: 7, column: 3, scope: !7)
+!18 = !DILocation(line: 8, column: 1, scope: !7)
+!19 = !DILocalVariable(name: "local2", scope: !7, file: !1, line: 4, type: !10)

llvm/test/DebugInfo/X86/instr-ref-selectiondag.ll

@@ -237,12 +237,11 @@ shoes:
 
 ; FASTISEL-INSTRREF-LABEL: name: qux
 
-; FASTISEL-INSTRREF:      DBG_PHI $rdi, 2
-; FASTISEL-INSTRREF-NEXT: DBG_PHI $rdi, 1
+; FASTISEL-INSTRREF:      DBG_PHI $rdi, 1
 ; FASTISEL-INSTRREF:      DBG_INSTR_REF 1, 0, ![[SOCKS]], !DIExpression(DW_OP_deref),
 
 ; FASTISEL-INSTRREF-LABEL: bb.1.lala:
-; FASTISEL-INSTRREF:      DBG_INSTR_REF 2, 0, ![[KNEES]], !DIExpression(DW_OP_deref),
+; FASTISEL-INSTRREF:      DBG_INSTR_REF 1, 0, ![[KNEES]], !DIExpression(DW_OP_deref),
 declare i64 @cheddar(i32 *%arg)
 
 define void @qux(i32* noalias sret(i32) %agg.result) !dbg !40 {