Skip to content

Commit f225272

Browse files
OCHyamsOCHyams
committed
[Assignment Tracking] Fix mem2reg misidentifying unlinked stores
updateForDeletedStore updates the assignment tracking debug info for a store that is about to be deleted by mem2reg. For each variable backed by the target alloca, if a dbg.assign exists it is kept (well - it's downgraded to a dbg.value). A dbg.value is inserted if there's not a linked dbg.assign for a variable which is backed by the target alloca. This patch fixes a bug whereby a store with a linked dbg.assign that describes a fragment different to the one linked to the alloca was not counted for the variable, leading to both keeping the dbg.assign (downgrading it) and inserting a new dbg.value. Reviewed By: StephenTozer Differential Revision: https://reviews.llvm.org/D146299
1 parent a17b71d commit f225272

File tree

3 files changed

+87
-4
lines changed

3 files changed

+87
-4
lines changed

‎llvm/lib/Transforms/Utils/PromoteMemoryToRegister.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -132,9 +132,9 @@ class AssignmentTrackingInfo {
132132
// unnecessary function-local metadata. Remember that we've seen a
133133
// dbg.assign for each variable fragment for the untracked store handling
134134
// (after this loop).
135-
SmallSet<DebugVariable, 2> VarHasDbgAssignForStore;
135+
SmallSet<DebugVariableAggregate, 2> VarHasDbgAssignForStore;
136136
for (DbgAssignIntrinsic *DAI : at::getAssignmentMarkers(ToDelete)) {
137-
VarHasDbgAssignForStore.insert(DebugVariable(DAI));
137+
VarHasDbgAssignForStore.insert(DebugVariableAggregate(DAI));
138138
DbgAssignsToDelete->insert(DAI);
139139
DIB.insertDbgValueIntrinsic(DAI->getValue(), DAI->getVariable(),
140140
DAI->getExpression(), DAI->getDebugLoc(),
@@ -150,7 +150,7 @@ class AssignmentTrackingInfo {
150150
// size) or one that is trackable but has had its DIAssignID attachment
151151
// dropped accidentally.
152152
for (auto *DAI : DbgAssigns) {
153-
if (VarHasDbgAssignForStore.contains(DebugVariable(DAI)))
153+
if (VarHasDbgAssignForStore.contains(DebugVariableAggregate(DAI)))
154154
continue;
155155
ConvertDebugDeclareToDebugValue(DAI, ToDelete, DIB);
156156
}

‎llvm/test/DebugInfo/Generic/assignment-tracking/mem2reg/store-to-part-of-alloca.ll

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,84 @@
1+
; RUN: opt -passes=mem2reg -S %s -o - | FileCheck %s --implicit-check-not="call void @llvm.dbg"
2+
3+
; CHECK: llvm.dbg.value(metadata i64 0, metadata ![[#]], metadata !DIExpression(DW_OP_LLVM_fragment, 0, 32))
4+
5+
;; The store has a debug intrinsic attached to it with a fragment size
6+
;; different to the base alloca debug intrinsic fragment size. Check that
7+
;; mem2reg doesn't think this store is "untagged" for that base variable. If
8+
;; that were the case mem2reg would insert a dbg.value covering the entire
9+
;; variable, which isn't the right thing to do here. This example looks weird
10+
;; and not particularly compelling, but this was encountered in the wild on
11+
;; "real code".
12+
13+
;; Reduced from this C++ (which itself has been reduced).
14+
;; class a {
15+
;; public:
16+
;; a(float, float);
17+
;; };
18+
;; class d {
19+
;; protected:
20+
;; float b[4];
21+
;;
22+
;; public:
23+
;; float e() { return b[0]; }
24+
;; float f() { return b[1]; }
25+
;; };
26+
;; class g : public d {
27+
;; public:
28+
;; void operator*=(g) {
29+
;; {
30+
;; float __attribute__((nodebug)) c = b[2], __attribute__((nodebug)) h = b[0];
31+
;; b[0] = c;
32+
;; b[1] = h;
33+
;; }
34+
;; }
35+
;; };
36+
;; g get();
37+
;; void i() {
38+
;; g __attribute__((nodebug)) j = get();
39+
;; g k = j;
40+
;; k *= j;
41+
;; a(k.e(), k.f());
42+
;; }
43+
44+
define dso_local i64 @_Z3funv() #0 !dbg !10 {
45+
entry:
46+
%retval.sroa.0 = alloca i64, align 8, !DIAssignID !20
47+
call void @llvm.dbg.assign(metadata i1 undef, metadata !19, metadata !DIExpression(), metadata !20, metadata ptr %retval.sroa.0, metadata !DIExpression()), !dbg !21
48+
store i64 0, ptr %retval.sroa.0, align 8, !dbg !22, !DIAssignID !23
49+
call void @llvm.dbg.assign(metadata i64 0, metadata !19, metadata !DIExpression(DW_OP_LLVM_fragment, 0, 32), metadata !23, metadata ptr %retval.sroa.0, metadata !DIExpression()), !dbg !21
50+
ret i64 0
51+
}
52+
53+
declare void @llvm.dbg.declare(metadata, metadata, metadata) #1
54+
declare void @llvm.dbg.assign(metadata, metadata, metadata, metadata, metadata, metadata) #1
55+
56+
!llvm.dbg.cu = !{!0}
57+
!llvm.module.flags = !{!2, !3, !4, !5, !6, !7, !8}
58+
!llvm.ident = !{!9}
59+
60+
!0 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus_14, file: !1, producer: "clang version 17.0.0", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, splitDebugInlining: false, nameTableKind: None)
61+
!1 = !DIFile(filename: "test.cpp", directory: "/")
62+
!2 = !{i32 7, !"Dwarf Version", i32 5}
63+
!3 = !{i32 2, !"Debug Info Version", i32 3}
64+
!4 = !{i32 1, !"wchar_size", i32 4}
65+
!5 = !{i32 8, !"PIC Level", i32 2}
66+
!6 = !{i32 7, !"PIE Level", i32 2}
67+
!7 = !{i32 7, !"uwtable", i32 2}
68+
!8 = !{i32 7, !"debug-info-assignment-tracking", i1 true}
69+
!9 = !{!"clang version 17.0.0"}
70+
!10 = distinct !DISubprogram(name: "fun", linkageName: "_Z3funv", scope: !1, file: !1, line: 2, type: !11, scopeLine: 2, flags: DIFlagPrototyped | DIFlagAllCallsDescribed, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !0, retainedNodes: !18)
71+
!11 = !DISubroutineType(types: !12)
72+
!12 = !{!13}
73+
!13 = distinct !DICompositeType(tag: DW_TAG_structure_type, name: "Pair", file: !1, line: 1, size: 64, flags: DIFlagTypePassByValue, elements: !14, identifier: "_ZTS4Pair")
74+
!14 = !{!15, !17}
75+
!15 = !DIDerivedType(tag: DW_TAG_member, name: "A", scope: !13, file: !1, line: 1, baseType: !16, size: 32)
76+
!16 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
77+
!17 = !DIDerivedType(tag: DW_TAG_member, name: "B", scope: !13, file: !1, line: 1, baseType: !16, size: 32, offset: 32)
78+
!18 = !{!19}
79+
!19 = !DILocalVariable(name: "X", scope: !10, file: !1, line: 3, type: !13)
80+
!20 = distinct !DIAssignID()
81+
!21 = !DILocation(line: 0, scope: !10)
82+
!22 = !DILocation(line: 3, column: 8, scope: !10)
83+
!23 = distinct !DIAssignID()
84+
!24 = !DILocation(line: 4, column: 3, scope: !10)

‎llvm/test/DebugInfo/Generic/assignment-tracking/sroa/vec-2.ll

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,6 @@
3030
;; There's a few dbg intrinsics we're not interested in testing wedged in here.
3131
; CHECK-NEXT: dbg.value
3232
; CHECK-NEXT: dbg.value
33-
; CHECK-NEXT: dbg.value
3433
; CHECK-NEXT: call void @llvm.dbg.value(metadata float %2,{{.+}}, metadata !DIExpression(DW_OP_LLVM_fragment, 96, 32))
3534

3635
%class.d = type { %class.a }

0 commit comments

Comments
 (0)