[llvm] 1d712c3 - [DebugInfo][InstrRef] Don't generate redundant DBG_PHIs
Jeremy Morse via llvm-commits
llvm-commits at lists.llvm.org
Tue May 3 01:56:23 PDT 2022
Author: Jeremy Morse
Date: 2022-05-03T09:56:12+01:00
New Revision: 1d712c381819498940ee592b670a81eb0f762f7d
URL: https://github.com/llvm/llvm-project/commit/1d712c381819498940ee592b670a81eb0f762f7d
DIFF: https://github.com/llvm/llvm-project/commit/1d712c381819498940ee592b670a81eb0f762f7d.diff
LOG: [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
Added:
llvm/test/DebugInfo/X86/dbg-value-funcarg4.ll
Modified:
llvm/include/llvm/CodeGen/MachineFunction.h
llvm/lib/CodeGen/MachineFunction.cpp
llvm/test/DebugInfo/X86/dbg-value-funcarg.ll
llvm/test/DebugInfo/X86/instr-ref-selectiondag.ll
Removed:
################################################################################
diff --git a/llvm/include/llvm/CodeGen/MachineFunction.h b/llvm/include/llvm/CodeGen/MachineFunction.h
index 6363d3e0de148..55873c267dbdb 100644
--- a/llvm/include/llvm/CodeGen/MachineFunction.h
+++ b/llvm/include/llvm/CodeGen/MachineFunction.h
@@ -531,8 +531,13 @@ class LLVM_EXTERNAL_VISIBILITY MachineFunction {
/// 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
diff --git a/llvm/lib/CodeGen/MachineFunction.cpp b/llvm/lib/CodeGen/MachineFunction.cpp
index 06830e852a286..f2b0024ee4093 100644
--- a/llvm/lib/CodeGen/MachineFunction.cpp
+++ b/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 {
diff --git a/llvm/test/DebugInfo/X86/dbg-value-funcarg.ll b/llvm/test/DebugInfo/X86/dbg-value-funcarg.ll
index 4818fbf80d262..f1edf095e90f7 100644
--- a/llvm/test/DebugInfo/X86/dbg-value-funcarg.ll
+++ b/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
diff --git a/llvm/test/DebugInfo/X86/dbg-value-funcarg4.ll b/llvm/test/DebugInfo/X86/dbg-value-funcarg4.ll
new file mode 100644
index 0000000000000..df52a39a3497d
--- /dev/null
+++ b/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)
diff --git a/llvm/test/DebugInfo/X86/instr-ref-selectiondag.ll b/llvm/test/DebugInfo/X86/instr-ref-selectiondag.ll
index 465d4967c5fc9..34eab5e2beea9 100644
--- a/llvm/test/DebugInfo/X86/instr-ref-selectiondag.ll
+++ b/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 {
More information about the llvm-commits
mailing list