[llvm] 8e56a19 - [Assignment Tracking] Improve removeRedundantDbgLocsUsingBackwardScan
via llvm-commits
llvm-commits at lists.llvm.org
Wed Mar 29 08:00:11 PDT 2023
Author: OCHyams
Date: 2023-03-29T15:59:46+01:00
New Revision: 8e56a196fb5c26e556ac267986868a3f63b7c486
URL: https://github.com/llvm/llvm-project/commit/8e56a196fb5c26e556ac267986868a3f63b7c486
DIFF: https://github.com/llvm/llvm-project/commit/8e56a196fb5c26e556ac267986868a3f63b7c486.diff
LOG: [Assignment Tracking] Improve removeRedundantDbgLocsUsingBackwardScan
`removeRedundantDbgLocsUsingBackwardScan` removes redundant dbg loc definitions
by scanning backwards through contiguous sets of them (a "wedge"), removing
earlier (in IR order terms) defs for fragments of variables that are defined
later in the wedge.
In this patch we use a `Bitvector` for each variable to track which bits have
definitions to more accurately determine whether a loc def is redundant. This
patch increases compile time by itself, but reduces it when combined with the
follow-up patch.
Reviewed By: jmorse
Differential Revision: https://reviews.llvm.org/D146978
Added:
llvm/test/DebugInfo/assignment-tracking/X86/remove-redundant-defs-bwd-scan.ll
Modified:
llvm/include/llvm/IR/DebugInfoMetadata.h
llvm/lib/CodeGen/AssignmentTrackingAnalysis.cpp
llvm/test/DebugInfo/assignment-tracking/X86/nested-loop-frags.ll
llvm/test/DebugInfo/assignment-tracking/X86/untagged-store-frag.ll
Removed:
################################################################################
diff --git a/llvm/include/llvm/IR/DebugInfoMetadata.h b/llvm/include/llvm/IR/DebugInfoMetadata.h
index 258eda717e198..deabd9b0efbdf 100644
--- a/llvm/include/llvm/IR/DebugInfoMetadata.h
+++ b/llvm/include/llvm/IR/DebugInfoMetadata.h
@@ -2786,6 +2786,9 @@ class DIExpression : public MDNode {
/// Holds the characteristics of one fragment of a larger variable.
struct FragmentInfo {
+ FragmentInfo() = default;
+ FragmentInfo(uint64_t SizeInBits, uint64_t OffsetInBits)
+ : SizeInBits(SizeInBits), OffsetInBits(OffsetInBits) {}
uint64_t SizeInBits;
uint64_t OffsetInBits;
/// Return the index of the first bit of the fragment.
diff --git a/llvm/lib/CodeGen/AssignmentTrackingAnalysis.cpp b/llvm/lib/CodeGen/AssignmentTrackingAnalysis.cpp
index 02b113bf55652..059db6a76c10d 100644
--- a/llvm/lib/CodeGen/AssignmentTrackingAnalysis.cpp
+++ b/llvm/lib/CodeGen/AssignmentTrackingAnalysis.cpp
@@ -2191,15 +2191,14 @@ static bool
removeRedundantDbgLocsUsingBackwardScan(const BasicBlock *BB,
FunctionVarLocsBuilder &FnVarLocs) {
bool Changed = false;
- SmallDenseSet<DebugVariable> VariableSet;
-
+ SmallDenseMap<DebugAggregate, BitVector> VariableDefinedBits;
// Scan over the entire block, not just over the instructions mapped by
// FnVarLocs, because wedges in FnVarLocs may only be seperated by debug
// instructions.
for (const Instruction &I : reverse(*BB)) {
if (!isa<DbgVariableIntrinsic>(I)) {
// Sequence of consecutive defs ended. Clear map for the next one.
- VariableSet.clear();
+ VariableDefinedBits.clear();
}
// Get the location defs that start just before this instruction.
@@ -2215,21 +2214,44 @@ removeRedundantDbgLocsUsingBackwardScan(const BasicBlock *BB,
// Iterate over the existing defs in reverse.
for (auto RIt = Locs->rbegin(), REnd = Locs->rend(); RIt != REnd; ++RIt) {
NumDefsScanned++;
- const DebugVariable &Key = FnVarLocs.getVariable(RIt->VariableID);
- bool FirstDefOfFragment = VariableSet.insert(Key).second;
+ DebugAggregate Aggr =
+ getAggregate(FnVarLocs.getVariable(RIt->VariableID));
+ uint64_t SizeInBits = Aggr.first->getSizeInBits().value_or(0);
- // If the same variable fragment is described more than once it is enough
- // to keep the last one (i.e. the first found in this reverse iteration).
- if (FirstDefOfFragment) {
- // New def found: keep it.
+ if (SizeInBits == 0) {
+ // If the size is unknown (0) then keep this location def to be safe.
NewDefsReversed.push_back(*RIt);
- } else {
- // Redundant def found: throw it away. Since the wedge of defs is being
- // rebuilt, doing nothing is the same as deleting an entry.
- ChangedThisWedge = true;
- NumDefsRemoved++;
+ continue;
}
- continue;
+
+ // Only keep this location definition if it is not fully eclipsed by
+ // other definitions in this wedge that come after it
+
+ // Inert the bits the location definition defines.
+ auto InsertResult =
+ VariableDefinedBits.try_emplace(Aggr, BitVector(SizeInBits));
+ bool FirstDefinition = InsertResult.second;
+ BitVector &DefinedBits = InsertResult.first->second;
+
+ DIExpression::FragmentInfo Fragment =
+ RIt->Expr->getFragmentInfo().value_or(
+ DIExpression::FragmentInfo(SizeInBits, 0));
+ bool InvalidFragment = Fragment.endInBits() > SizeInBits;
+
+ // If this defines any previously undefined bits, keep it.
+ if (FirstDefinition || InvalidFragment ||
+ DefinedBits.find_first_unset_in(Fragment.startInBits(),
+ Fragment.endInBits()) != -1) {
+ if (!InvalidFragment)
+ DefinedBits.set(Fragment.startInBits(), Fragment.endInBits());
+ NewDefsReversed.push_back(*RIt);
+ continue;
+ }
+
+ // Redundant def found: throw it away. Since the wedge of defs is being
+ // rebuilt, doing nothing is the same as deleting an entry.
+ ChangedThisWedge = true;
+ NumDefsRemoved++;
}
// Un-reverse the defs and replace the wedge with the pruned version.
diff --git a/llvm/test/DebugInfo/assignment-tracking/X86/nested-loop-frags.ll b/llvm/test/DebugInfo/assignment-tracking/X86/nested-loop-frags.ll
index eedb0c6704370..db57f5f1a7400 100644
--- a/llvm/test/DebugInfo/assignment-tracking/X86/nested-loop-frags.ll
+++ b/llvm/test/DebugInfo/assignment-tracking/X86/nested-loop-frags.ll
@@ -121,13 +121,7 @@ entry:
; CHECK-NEXT: DBG_VALUE %stack.1.b.addr, $noreg, ![[b]], !DIExpression(DW_OP_deref)
; CHECK-NEXT: DBG_VALUE %stack.4.e.addr, $noreg, ![[e]], !DIExpression(DW_OP_deref)
; CHECK-NEXT: MOV64mi32 %stack.0.a.addr, 1, $noreg, 0, $noreg, 1
-; CHECK-NEXT: DBG_VALUE $noreg, $noreg, ![[a]], !DIExpression(DW_OP_LLVM_fragment, 32, 32)
-; CHECK-NEXT: DBG_VALUE %stack.0.a.addr, $noreg, ![[a]], !DIExpression(DW_OP_deref, DW_OP_LLVM_fragment, 0, 32)
-; CHECK-NEXT: DBG_VALUE %stack.0.a.addr, $noreg, ![[a]], !DIExpression(DW_OP_deref)
; CHECK-NEXT: MOV64mi32 %stack.1.b.addr, 1, $noreg, 0, $noreg, 2
-; CHECK-NEXT: DBG_VALUE $noreg, $noreg, ![[b]], !DIExpression(DW_OP_LLVM_fragment, 32, 32)
-; CHECK-NEXT: DBG_VALUE %stack.1.b.addr, $noreg, ![[b]], !DIExpression(DW_OP_deref, DW_OP_LLVM_fragment, 0, 32)
-; CHECK-NEXT: DBG_VALUE %stack.1.b.addr, $noreg, ![[b]], !DIExpression(DW_OP_deref)
; CHECK-NEXT: DBG_VALUE %stack.4.e.addr, $noreg, ![[e]], !DIExpression(DW_OP_plus_uconst, 4, DW_OP_deref, DW_OP_LLVM_fragment, 32, 32)
; CHECK-NEXT: DBG_VALUE %stack.4.e.addr, $noreg, ![[e]], !DIExpression(DW_OP_deref, DW_OP_LLVM_fragment, 0, 32)
; CHECK-NEXT: MOV32mi %stack.0.a.addr, 1, $noreg, 0, $noreg, 3
@@ -213,8 +207,6 @@ do.cond: ; preds = %if.then, %if.else
; CHECK-LABEL: bb.5.do.cond:
; CHECK-NEXT: successors
; CHECK-NEXT: {{^ *$}}
-; xCHECK-NEXT: XXX ?DBG_VALUE %stack.2.c.addr, $noreg, ![[c]], !DIExpression(DW_OP_deref, DW_OP_LLVM_fragment, 32, 32)
-; xCHECK-NEXT: DBG_VALUE 11, $noreg, ![[f]], !DIExpression(DW_OP_LLVM_fragment, 32, 32)
; CHECK-NOT: DBG_VALUE
; CHECK: {{^ *$}}
@@ -244,8 +236,6 @@ do.end6: ; preds = %do.cond4
; CHECK-NEXT: DBG_VALUE %stack.0.a.addr, $noreg, ![[a]], !DIExpression(DW_OP_deref, DW_OP_LLVM_fragment, 0, 32)
; CHECK-NEXT: DBG_VALUE 4, $noreg, ![[b]], !DIExpression(DW_OP_LLVM_fragment, 0, 32)
; CHECK-NEXT: DBG_VALUE %stack.1.b.addr, $noreg, ![[b]], !DIExpression(DW_OP_deref, DW_OP_LLVM_fragment, 32, 32)
-; xCHECK-NEXT: XXX ? DBG_VALUE %stack.3.d.addr, $noreg, ![[d]], !DIExpression(DW_OP_deref, DW_OP_LLVM_fragment, 32, 32)
-; xCHECK-NEXT: DBG_VALUE 11, $noreg, ![[f]], !DIExpression(DW_OP_LLVM_fragment, 32, 32)
}
declare !dbg !54 void @_Z4calli(i32 noundef) local_unnamed_addr #1
diff --git a/llvm/test/DebugInfo/assignment-tracking/X86/remove-redundant-defs-bwd-scan.ll b/llvm/test/DebugInfo/assignment-tracking/X86/remove-redundant-defs-bwd-scan.ll
new file mode 100644
index 0000000000000..dc2638c69969d
--- /dev/null
+++ b/llvm/test/DebugInfo/assignment-tracking/X86/remove-redundant-defs-bwd-scan.ll
@@ -0,0 +1,70 @@
+; RUN: llc %s -stop-after=finalize-isel -o - \
+; RUN: | FileCheck %s --implicit-check-not=DBG_
+
+;; Hand-written to test assignment tracking analysis' removal of redundant
+;; debug loc definitions. Checks written inline.
+
+target triple = "x86_64-unknown-linux-gnu"
+
+define dso_local void @f() #0 !dbg !8 {
+entry:
+ call void @llvm.dbg.value(metadata i32 0, metadata !14, metadata !DIExpression(DW_OP_LLVM_fragment, 0, 32)), !dbg !15
+ call void @llvm.dbg.value(metadata i64 1, metadata !14, metadata !DIExpression()), !dbg !15
+;; def [0 -> 32)
+;; def [0 -> 64)
+;; Second frag fully contains the first so first should be removed.
+; CHECK: DBG_VALUE 1,
+ call void @step()
+ call void @llvm.dbg.value(metadata i32 2, metadata !14, metadata !DIExpression(DW_OP_LLVM_fragment, 0, 32)), !dbg !15
+ call void @llvm.dbg.value(metadata i32 3, metadata !14, metadata !DIExpression(DW_OP_LLVM_fragment, 32, 32)), !dbg !15
+;; def [0 -> 32)
+;; def [32 -> 64)
+;; Frags don't overlap so no removal should take place:
+; CHECK: DBG_VALUE 2,
+; CHECK-NEXT: DBG_VALUE 3,
+ call void @step()
+ call void @llvm.dbg.value(metadata i16 4, metadata !14, metadata !DIExpression(DW_OP_LLVM_fragment, 0, 16)), !dbg !15
+ call void @llvm.dbg.value(metadata i8 5, metadata !14, metadata !DIExpression(DW_OP_LLVM_fragment, 8, 8)), !dbg !15
+;; def [0 -> 16)
+;; def [8 -> 16)
+;; Second frag doesn't fully contain first so no removal should take place:
+; CHECK: DBG_VALUE 4,
+; CHECK-NEXT: DBG_VALUE 5,
+ call void @step()
+ call void @llvm.dbg.value(metadata i16 6, metadata !14, metadata !DIExpression(DW_OP_LLVM_fragment, 0, 16)), !dbg !15
+ call void @llvm.dbg.value(metadata i8 7, metadata !14, metadata !DIExpression(DW_OP_LLVM_fragment, 8, 8)), !dbg !15
+ call void @llvm.dbg.value(metadata i32 8, metadata !14, metadata !DIExpression(DW_OP_LLVM_fragment, 8, 16)), !dbg !15
+;; def [0 -> 16)
+;; def [8 -> 16)
+;; def [8 -> 24)
+;; Middle frag is fully contained by the last so should be removed.
+; CHECK: DBG_VALUE 6,
+; CHECK-NEXT: DBG_VALUE 8,
+ ret void
+}
+
+declare void @step()
+declare void @llvm.dbg.value(metadata, metadata, metadata) #1
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!2, !3, !4, !5, !6, !1000}
+!llvm.ident = !{!7}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus_14, file: !1, producer: "clang version 14.0.0", isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug, splitDebugInlining: false, nameTableKind: None)
+!1 = !DIFile(filename: "test.cpp", directory: "/")
+!2 = !{i32 7, !"Dwarf Version", i32 5}
+!3 = !{i32 2, !"Debug Info Version", i32 3}
+!4 = !{i32 1, !"wchar_size", i32 4}
+!5 = !{i32 7, !"uwtable", i32 1}
+!6 = !{i32 7, !"frame-pointer", i32 2}
+!7 = !{!"clang version 14.0.0"}
+!8 = distinct !DISubprogram(name: "f", linkageName: "_Z1fl", scope: !1, file: !1, line: 1, type: !9, scopeLine: 1, flags: DIFlagPrototyped, spFlags: DISPFlagDefinition, unit: !0, retainedNodes: !12)
+!9 = !DISubroutineType(types: !10)
+!10 = !{!11, !11}
+!11 = !DIBasicType(name: "long", size: 64, encoding: DW_ATE_signed)
+!12 = !{}
+!13 = distinct !DIAssignID()
+!14 = !DILocalVariable(name: "a", arg: 1, scope: !8, file: !1, line: 1, type: !11)
+!15 = !DILocation(line: 0, scope: !8)
+!16 = distinct !DIAssignID()
+!1000 = !{i32 7, !"debug-info-assignment-tracking", i1 true}
diff --git a/llvm/test/DebugInfo/assignment-tracking/X86/untagged-store-frag.ll b/llvm/test/DebugInfo/assignment-tracking/X86/untagged-store-frag.ll
index 5e02700a2cc2e..b7e713c7224f4 100644
--- a/llvm/test/DebugInfo/assignment-tracking/X86/untagged-store-frag.ll
+++ b/llvm/test/DebugInfo/assignment-tracking/X86/untagged-store-frag.ll
@@ -27,6 +27,9 @@
; CHECK-DAG: ![[A:[0-9]+]] = !DILocalVariable(name: "a",
; CHECK: DBG_VALUE %stack.0.a.addr, $noreg, ![[A]], !DIExpression(DW_OP_deref)
+; CHECK-NEXT: ADJCALLSTACKDOWN
+; CHECK-NEXT: @step
+; CHECK-NEXT: ADJCALLSTACKUP
; CHECK-NEXT: DBG_VALUE 5, $noreg, ![[A]], !DIExpression(DW_OP_LLVM_fragment, 0, 32)
; CHECK-NEXT: DBG_VALUE $noreg, $noreg, ![[A]], !DIExpression(DW_OP_LLVM_fragment, 32, 32)
; CHECK-NEXT: MOV32mi %stack.0.a.addr, 1, $noreg, 0, $noreg, 123
@@ -34,16 +37,13 @@
; CHECK-NEXT: MOV32mr %stack.0.a.addr, 1, $noreg, 4, $noreg, %1 :: (store (s32) into %ir.add.ptr, align 8)
; CHECK-NEXT: DBG_VALUE %stack.0.a.addr, $noreg, ![[A]], !DIExpression(DW_OP_deref, DW_OP_LLVM_fragment, 32, 32)
-;; NOTE: The second and third DBG_VALUE combined make the first redundant. If
-;; removeRedundantDbgInstrs gets smarter, add an instruction between the first
-;; dbg.assign and the subsequent dbg.value.
-
target triple = "x86_64-unknown-linux-gnu"
define dso_local noundef i64 @_Z1fl(i64 noundef %a, i32 %b) #0 !dbg !8 {
entry:
%a.addr = alloca i64, align 8, !DIAssignID !13
call void @llvm.dbg.assign(metadata i1 undef, metadata !14, metadata !DIExpression(), metadata !13, metadata ptr %a.addr, metadata !DIExpression()), !dbg !15
+ call void @step()
call void @llvm.dbg.value(metadata i64 5, metadata !14, metadata !DIExpression(DW_OP_LLVM_fragment, 0, 32)), !dbg !15
call void @llvm.dbg.assign(metadata i1 undef, metadata !14, metadata !DIExpression(DW_OP_LLVM_fragment, 32, 32), metadata !16, metadata ptr %a.addr, metadata !DIExpression()), !dbg !15
%frag.addr = bitcast ptr %a.addr to ptr
@@ -55,6 +55,7 @@ entry:
ret i64 %1
}
+declare void @step()
declare void @llvm.dbg.value(metadata, metadata, metadata) #1
declare void @llvm.dbg.assign(metadata, metadata, metadata, metadata, metadata, metadata) #1
More information about the llvm-commits
mailing list