- Notifications
You must be signed in to change notification settings - Fork14.5k
[DebugInfo][NewGVN] Salvage debug values of trivially dead instructions#149304
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
base:main
Are you sure you want to change the base?
[DebugInfo][NewGVN] Salvage debug values of trivially dead instructions#149304
Uh oh!
There was an error while loading.Please reload this page.
Conversation
llvmbot commentedJul 17, 2025 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
@llvm/pr-subscribers-llvm-transforms @llvm/pr-subscribers-debuginfo Author: Shan Huang (Apochens) Changesfix #147634 Full diff:https://github.com/llvm/llvm-project/pull/149304.diff 2 Files Affected:
diff --git a/llvm/lib/Transforms/Scalar/NewGVN.cpp b/llvm/lib/Transforms/Scalar/NewGVN.cppindex 7eeaaa0d99602..17c4fd9c2aae9 100644--- a/llvm/lib/Transforms/Scalar/NewGVN.cpp+++ b/llvm/lib/Transforms/Scalar/NewGVN.cpp@@ -3044,6 +3044,7 @@ std::pair<unsigned, unsigned> NewGVN::assignDFSNumbers(BasicBlock *B, if (isInstructionTriviallyDead(&I, TLI)) { InstrDFS[&I] = 0; LLVM_DEBUG(dbgs() << "Skipping trivially dead instruction " << I << "\n");+ salvageDebugInfo(I); markInstructionForDeletion(&I); continue; }diff --git a/llvm/test/Transforms/NewGVN/salvage-trivially-dead-inst.ll b/llvm/test/Transforms/NewGVN/salvage-trivially-dead-inst.llnew file mode 100644index 0000000000000..1845cf6f0852c--- /dev/null+++ b/llvm/test/Transforms/NewGVN/salvage-trivially-dead-inst.ll@@ -0,0 +1,58 @@+; RUN: opt -passes=newgvn -S %s | FileCheck %s++; Check that assignDFSNumbers() in NewGVN salvages the debug values of the+; trivially dead instructions that are marked for deletion.++; CHECK: #dbg_value(i8 %tmp, [[META11:![0-9]+]], !DIExpression(DW_OP_constu, 8, DW_OP_eq, DW_OP_stack_value), [[META26:![0-9]+]])+; CHECK: [[META11]] = !DILocalVariable(name: "2"+; CHECK: [[META26]] = !DILocation(line: 3++define void @test13() !dbg !5 {+bb:+ br label %bb1++bb1:+ %tmp = load i8, ptr null, align 1+ %tmp2 = icmp eq i8 %tmp, 8, !dbg !26+ #dbg_value(i1 %tmp2, !11, !DIExpression(), !26)+ br label %bb3++bb3:+ %tmp4 = phi ptr [ null, %bb1 ], [ %tmp6, %bb3 ]+ %tmp5 = phi i32 [ undef, %bb1 ], [ %tmp9, %bb3 ]+ %tmp6 = getelementptr i8, ptr %tmp4, i64 1+ %tmp7 = load i8, ptr %tmp4, align 1+ %tmp8 = sext i8 %tmp7 to i32+ %tmp9 = mul i32 %tmp5, %tmp8+ %tmp10 = load i8, ptr %tmp6, align 1+ %tmp11 = icmp eq i8 %tmp10, 0+ br i1 %tmp11, label %bb12, label %bb3++bb12:+ %tmp13 = phi i32 [ %tmp9, %bb3 ]+ %tmp14 = icmp eq i32 %tmp13, 0+ br i1 %tmp14, label %bb1, label %bb15++bb15:+ call void (...) @bar()+ br label %bb1+}++declare void @bar(...)++!llvm.dbg.cu = !{!0}+!llvm.debugify = !{!2, !3}+!llvm.module.flags = !{!4}++!0 = distinct !DICompileUnit(language: DW_LANG_C, file: !1, producer: "debugify", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug)+!1 = !DIFile(filename: "/app/example.ll", directory: "/")+!2 = !{i32 18}+!3 = !{i32 12}+!4 = !{i32 2, !"Debug Info Version", i32 3}+!5 = distinct !DISubprogram(name: "test13", linkageName: "test13", scope: null, file: !1, line: 1, type: !6, scopeLine: 1, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !0, retainedNodes: !8)+!6 = !DISubroutineType(types: !7)+!7 = !{}+!8 = !{!11}+!10 = !DIBasicType(name: "ty8", size: 8, encoding: DW_ATE_unsigned)+!11 = !DILocalVariable(name: "2", scope: !5, file: !1, line: 3, type: !10)+!26 = !DILocation(line: 3, column: 1, scope: !5)\ No newline at end of file |
github-actionsbot commentedJul 17, 2025 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
✅ With the latest revision this PR passed the undef deprecator. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Are there are other calls tomarkInstructionForDeletion
that should be accompanied by a salvage? What's the consequences of putting the salvage call inmarkInstructionForDeletion
instead?
#dbg_value(i1 %tmp2, !11, !DIExpression(), !26) | ||
br label %bb3 | ||
bb3: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Do we need all the instructions below, or is the following sufficient to exercise the behaviour?
definevoid@test13()!dbg!5 {entry:%tmp =loadi8,ptrnull,align1%tmp2 =icmpeqi8%tmp,8,!dbg!26 #dbg_value(i1%tmp2,!11,!DIExpression(),!26)retvoid}
If you do need the extra instructions, I think you'll want to replace theundef
withpoison
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Thanks. I think this concise test case is enough.
For now, I found this call to
As discussed in the prior issue (#147511), placing the salvage call into the following loop would introduce unnecessary computation overhead of salvaging.
Similarly, putting the salvage call into |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
LGTM, thanks
Uh oh!
There was an error while loading.Please reload this page.
fix#149301