Skip to content

Commit b211752

Browse files
jmorsejmorse
authored
[DebugInfo][RemoveDIs] Avoid crash and output-difference in loop-rotate (llvm#74093)
Avoid editing a range of DPValues and then remapping them. This occurs when we try to de-duplicate dbg.values, but then re-use the same iterator range. We can instead remap them, and then erase any duplicates. At the same time refactor the computation of seen-intrinsic hashes, and account for a peculiarity of loop-rotates existing behaviour: it will only deduplicate dbg.values that are immediately before the preheaders branch instruction, not just any dbg.value in the preheader.
1 parent 40381d1 commit b211752

File tree

2 files changed

+85
-18
lines changed

2 files changed

+85
-18
lines changed

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

Lines changed: 22 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -541,31 +541,31 @@ bool LoopRotate::rotateLoop(Loop *L, bool SimplifiedLatch) {
541541
// duplication.
542542
using DbgIntrinsicHash =
543543
std::pair<std::pair<hash_code, DILocalVariable *>, DIExpression *>;
544-
auto makeHash = [](DbgVariableIntrinsic *D) -> DbgIntrinsicHash {
544+
auto makeHash = [](auto *D) -> DbgIntrinsicHash {
545545
auto VarLocOps = D->location_ops();
546546
return {{hash_combine_range(VarLocOps.begin(), VarLocOps.end()),
547547
D->getVariable()},
548548
D->getExpression()};
549549
};
550+
550551
SmallDenseSet<DbgIntrinsicHash, 8> DbgIntrinsics;
551552
for (Instruction &I : llvm::drop_begin(llvm::reverse(*OrigPreheader))) {
552-
if (auto *DII = dyn_cast<DbgVariableIntrinsic>(&I))
553+
if (auto *DII = dyn_cast<DbgVariableIntrinsic>(&I)) {
553554
DbgIntrinsics.insert(makeHash(DII));
554-
else
555+
// Until RemoveDIs supports dbg.declares in DPValue format, we'll need
556+
// to collect DPValues attached to any other debug intrinsics.
557+
for (const DPValue &DPV : DII->getDbgValueRange())
558+
DbgIntrinsics.insert(makeHash(&DPV));
559+
} else {
555560
break;
561+
}
556562
}
557563

558-
// Duplicate implementation for DPValues, the non-instruction format of
559-
// debug-info records in RemoveDIs.
560-
auto makeHashDPV = [](const DPValue &D) -> DbgIntrinsicHash {
561-
auto VarLocOps = D.location_ops();
562-
return {{hash_combine_range(VarLocOps.begin(), VarLocOps.end()),
563-
D.getVariable()},
564-
D.getExpression()};
565-
};
566-
for (Instruction &I : llvm::drop_begin(llvm::reverse(*OrigPreheader)))
567-
for (const DPValue &DPV : I.getDbgValueRange())
568-
DbgIntrinsics.insert(makeHashDPV(DPV));
564+
// Build DPValue hashes for DPValues attached to the terminator, which isn't
565+
// considered in the loop above.
566+
for (const DPValue &DPV :
567+
OrigPreheader->getTerminator()->getDbgValueRange())
568+
DbgIntrinsics.insert(makeHash(&DPV));
569569

570570
// Remember the local noalias scope declarations in the header. After the
571571
// rotation, they must be duplicated and the scope must be cloned. This
@@ -616,6 +616,10 @@ bool LoopRotate::rotateLoop(Loop *L, bool SimplifiedLatch) {
616616
LoopEntryBranch->cloneDebugInfoFrom(Inst, NextDbgInst);
617617
RemapDPValueRange(M, DbgValueRange, ValueMap,
618618
RF_NoModuleLevelChanges | RF_IgnoreMissingLocals);
619+
// Erase anything we've seen before.
620+
for (DPValue &DPV : make_early_inc_range(DbgValueRange))
621+
if (DbgIntrinsics.count(makeHash(&DPV)))
622+
DPV.eraseFromParent();
619623
}
620624

621625
NextDbgInst = I->getDbgValueRange().begin();
@@ -633,13 +637,13 @@ bool LoopRotate::rotateLoop(Loop *L, bool SimplifiedLatch) {
633637

634638
if (LoopEntryBranch->getParent()->IsNewDbgInfoFormat) {
635639
auto Range = C->cloneDebugInfoFrom(Inst, NextDbgInst);
636-
// Erase anything we've seen before.
637-
for (DPValue &DPV : make_early_inc_range(Range))
638-
if (DbgIntrinsics.count(makeHashDPV(DPV)))
639-
DPV.eraseFromParent();
640640
RemapDPValueRange(M, Range, ValueMap,
641641
RF_NoModuleLevelChanges | RF_IgnoreMissingLocals);
642642
NextDbgInst = std::nullopt;
643+
// Erase anything we've seen before.
644+
for (DPValue &DPV : make_early_inc_range(Range))
645+
if (DbgIntrinsics.count(makeHash(&DPV)))
646+
DPV.eraseFromParent();
643647
}
644648

645649
// Eagerly remap the operands of the instruction.

‎llvm/test/Transforms/LoopRotate/delete-dbg-values.ll

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
; RUN: opt --passes=loop-rotate -o - -S %s | FileCheck %s --implicit-check-not=dbg.value
2+
; RUN: opt --passes=loop-rotate -o - -S %s --try-experimental-debuginfo-iterators | FileCheck %s --implicit-check-not=dbg.value
3+
;
4+
;; Test some fine-grained behaviour of loop-rotate's de-duplication of
5+
;; dbg.values. The intrinsic on the first branch should be seen and
6+
;; prevent the rotation of the dbg.value for "sink" into the entry block.
7+
;; However the other dbg.value, for "source", should not be seen, and we'll
8+
;; get a duplicate.
9+
;
10+
target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-i128:128-f80:128-n8:16:32:64-S128"
11+
target triple = "x86_64-unknown-linux-gnu"
12+
13+
; CHECK: declare void @llvm.dbg.value(metadata,
14+
15+
; CHECK-LABEL: define void @_ZNK4llvm5APInt4sextEj(ptr
16+
; CHECK-LABEL: entry:
17+
; CHECK: call void @llvm.dbg.value(metadata i32 0, metadata ![[SRC:[0-9]+]],
18+
; CHECK-NEXT: load
19+
; CHECK-NEXT: call void @llvm.dbg.value(metadata i32 0, metadata ![[SINK:[0-9]+]],
20+
; CHECK-NEXT: call void @llvm.dbg.value(metadata i32 0, metadata ![[SRC]],
21+
; CHECK-LABEL: for.body:
22+
; CHECK: call void @llvm.dbg.value(metadata i32 0, metadata ![[SINK]],
23+
; CHECK-NEXT: call void @llvm.dbg.value(metadata i32 0, metadata ![[SRC]],
24+
25+
declare void @llvm.dbg.value(metadata, metadata, metadata)
26+
27+
define void @_ZNK4llvm5APInt4sextEj(ptr %agg.result) !dbg !5 {
28+
entry:
29+
tail call void @llvm.dbg.value(metadata i32 0, metadata !4, metadata !DIExpression()), !dbg !10
30+
%.pre = load i32, ptr %agg.result, align 8
31+
tail call void @llvm.dbg.value(metadata i32 0, metadata !11, metadata !DIExpression()), !dbg !10
32+
br label %for.cond
33+
34+
for.cond: ; preds = %for.body, %entry
35+
%i.0 = phi i32 [ 0, %entry ], [ 1, %for.body ]
36+
tail call void @llvm.dbg.value(metadata i32 0, metadata !11, metadata !DIExpression()), !dbg !10
37+
tail call void @llvm.dbg.value(metadata i32 0, metadata !4, metadata !DIExpression()), !dbg !10
38+
%cmp12.not = icmp eq i32 %i.0, %.pre, !dbg !10
39+
br i1 %cmp12.not, label %for.end, label %for.body
40+
41+
for.body: ; preds = %for.cond
42+
store i64 0, ptr %agg.result, align 8
43+
br label %for.cond
44+
45+
for.end: ; preds = %for.cond
46+
ret void
47+
}
48+
49+
!llvm.dbg.cu = !{!0}
50+
!llvm.module.flags = !{!3}
51+
52+
!0 = distinct !DICompileUnit(language: DW_LANG_C11, file: !1, isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, enums: !2, retainedTypes: !2, globals: !2, splitDebugInlining: false, nameTableKind: None)
53+
!1 = !DIFile(filename: "foo", directory: "bar")
54+
!2 = !{}
55+
!3 = !{i32 2, !"Debug Info Version", i32 3}
56+
!4 = !DILocalVariable(name: "source", scope: !5, file: !6, line: 170, type: !8)
57+
!5 = distinct !DISubprogram(name: "ConvertUTF16toUTF32", scope: !6, file: !6, line: 166, type: !7, scopeLine: 168, flags: DIFlagPrototyped | DIFlagAllCallsDescribed, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !0, retainedNodes: !2)
58+
!6 = !DIFile(filename: "fooo", directory: ".")
59+
!7 = !DISubroutineType(types: !2)
60+
!8 = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: !9, size: 64)
61+
!9 = !DIBasicType(name: "unsigned short", size: 16, encoding: DW_ATE_unsigned)
62+
!10 = !DILocation(line: 0, scope: !5)
63+
!11 = !DILocalVariable(name: "sink", scope: !5, file: !6, line: 170, type: !8)

0 commit comments

Comments
 (0)