[GVNSink] Fix #77415: GVNSink fails to optimize LLVM IR with debug info #77562
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@llvm/pr-subscribers-debuginfo @llvm/pr-subscribers-llvm-transforms Author: Shan Huang (Apochens) ChangesThis PR fixes issue #77415 and is revised from PR #77419 . PR #77419 breaks the newly added test in the same PR on windows, because GVNSink is non-deterministic when sorting BasicBlock* pointers. This is reflected in the failure report. According to the report, what the CheckFile want to match is the So I refined the regression test with a more general check rule to only detect whether there is a Full diff: https://github.com/llvm/llvm-project/pull/77562.diff 2 Files Affected:
diff --git a/llvm/lib/Transforms/Scalar/GVNSink.cpp b/llvm/lib/Transforms/Scalar/GVNSink.cpp
index 2b38831139a580..9db66b3793b8dd 100644
--- a/llvm/lib/Transforms/Scalar/GVNSink.cpp
+++ b/llvm/lib/Transforms/Scalar/GVNSink.cpp
@@ -132,7 +132,7 @@ class LockstepReverseIterator {
ActiveBlocks.remove(BB);
continue;
}
- Insts.push_back(BB->getTerminator()->getPrevNode());
+ Insts.push_back(BB->getTerminator()->getPrevNonDebugInstruction());
}
if (Insts.empty())
Fail = true;
@@ -168,7 +168,7 @@ class LockstepReverseIterator {
if (Inst == &Inst->getParent()->front())
ActiveBlocks.remove(Inst->getParent());
else
- NewInsts.push_back(Inst->getPrevNode());
+ NewInsts.push_back(Inst->getPrevNonDebugInstruction());
}
if (NewInsts.empty()) {
Fail = true;
@@ -834,7 +834,7 @@ void GVNSink::sinkLastInstruction(ArrayRef<BasicBlock *> Blocks,
BasicBlock *BBEnd) {
SmallVector<Instruction *, 4> Insts;
for (BasicBlock *BB : Blocks)
- Insts.push_back(BB->getTerminator()->getPrevNode());
+ Insts.push_back(BB->getTerminator()->getPrevNonDebugInstruction());
Instruction *I0 = Insts.front();
SmallVector<Value *, 4> NewOperands;
diff --git a/llvm/test/Transforms/GVNSink/sink-ignore-dbg-intrinsics.ll b/llvm/test/Transforms/GVNSink/sink-ignore-dbg-intrinsics.ll
new file mode 100644
index 00000000000000..ee6039a0d806ab
--- /dev/null
+++ b/llvm/test/Transforms/GVNSink/sink-ignore-dbg-intrinsics.ll
@@ -0,0 +1,86 @@
+; RUN: opt < %s -passes=gvn-sink -S | FileCheck %s
+
+; Function Attrs: noinline nounwind uwtable
+define dso_local i32 @fun(i32 noundef %a, i32 noundef %b) #0 !dbg !10 {
+entry:
+ tail call void @llvm.dbg.value(metadata i32 %a, metadata !15, metadata !DIExpression()), !dbg !16
+ tail call void @llvm.dbg.value(metadata i32 %b, metadata !17, metadata !DIExpression()), !dbg !16
+ %cmp = icmp sgt i32 %b, 10, !dbg !18
+ br i1 %cmp, label %if.then, label %if.else, !dbg !20
+
+if.then: ; preds = %entry
+ %add = add nsw i32 %a, 1, !dbg !21
+ tail call void @llvm.dbg.value(metadata i32 %add, metadata !23, metadata !DIExpression()), !dbg !24
+ %xor = xor i32 %add, 1, !dbg !25
+ tail call void @llvm.dbg.value(metadata i32 %xor, metadata !26, metadata !DIExpression()), !dbg !24
+ tail call void @llvm.dbg.value(metadata i32 %xor, metadata !27, metadata !DIExpression()), !dbg !16
+ br label %if.end, !dbg !28
+
+if.else: ; preds = %entry
+ %add1 = add nsw i32 %b, 1, !dbg !29
+ tail call void @llvm.dbg.value(metadata i32 %add1, metadata !31, metadata !DIExpression()), !dbg !32
+ %xor2 = xor i32 %add1, 1, !dbg !33
+ tail call void @llvm.dbg.value(metadata i32 %xor2, metadata !34, metadata !DIExpression()), !dbg !32
+ tail call void @llvm.dbg.value(metadata i32 %xor2, metadata !27, metadata !DIExpression()), !dbg !16
+ br label %if.end
+
+; CHECK-LABEL: if.end:
+; CHECK: [[SINK:%.*.sink]] = phi i32 [ %a, %if.then ], [ %b, %if.else ]
+; CHECK: %add = add nsw i32 [[SINK]], 1
+; CHECK: %xor = xor i32 %add, 1
+if.end: ; preds = %if.else, %if.then
+ %ret.0 = phi i32 [ %xor, %if.then ], [ %xor2, %if.else ], !dbg !35
+ tail call void @llvm.dbg.value(metadata i32 %ret.0, metadata !27, metadata !DIExpression()), !dbg !16
+ ret i32 %ret.0, !dbg !36
+}
+
+; Function Attrs: nocallback nofree nosync nounwind speculatable willreturn memory(none)
+declare void @llvm.dbg.declare(metadata, metadata, metadata) #1
+
+; Function Attrs: nocallback nofree nosync nounwind speculatable willreturn memory(none)
+declare void @llvm.dbg.value(metadata, metadata, metadata) #1
+
+attributes #0 = { noinline nounwind uwtable "frame-pointer"="all" "min-legal-vector-width"="0" "no-trapping-math"="true" "stack-protector-buffer-size"="8" "target-cpu"="x86-64" "target-features"="+cmov,+cx8,+fxsr,+mmx,+sse,+sse2,+x87" "tune-cpu"="generic" }
+attributes #1 = { nocallback nofree nosync nounwind speculatable willreturn memory(none) }
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!2, !3, !4, !5, !6, !7, !8}
+!llvm.ident = !{!9}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C11, file: !1, producer: "clang version 18.0.0git (https://github.com/llvm/llvm-project.git 5dfcb3e5d1d16bb4f8fce52b3c089119ed977e7f)", isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug, splitDebugInlining: false, nameTableKind: None)
+!1 = !DIFile(filename: "main.c", directory: "/home/hs/llvm-test", checksumkind: CSK_MD5, checksum: "68c28c3d0877bed08ff43db70c573802")
+!2 = !{i32 7, !"Dwarf Version", i32 5}
+!3 = !{i32 2, !"Debug Info Version", i32 3}
+!4 = !{i32 1, !"wchar_size", i32 4}
+!5 = !{i32 8, !"PIC Level", i32 2}
+!6 = !{i32 7, !"PIE Level", i32 2}
+!7 = !{i32 7, !"uwtable", i32 2}
+!8 = !{i32 7, !"frame-pointer", i32 2}
+!9 = !{!"clang version 18.0.0git (https://github.com/llvm/llvm-project.git 5dfcb3e5d1d16bb4f8fce52b3c089119ed977e7f)"}
+!10 = distinct !DISubprogram(name: "fun", scope: !1, file: !1, line: 1, type: !11, scopeLine: 1, flags: DIFlagPrototyped, spFlags: DISPFlagDefinition, unit: !0, retainedNodes: !14)
+!11 = !DISubroutineType(types: !12)
+!12 = !{!13, !13, !13}
+!13 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
+!14 = !{}
+!15 = !DILocalVariable(name: "a", arg: 1, scope: !10, file: !1, line: 1, type: !13)
+!16 = !DILocation(line: 0, scope: !10)
+!17 = !DILocalVariable(name: "b", arg: 2, scope: !10, file: !1, line: 1, type: !13)
+!18 = !DILocation(line: 3, column: 11, scope: !19)
+!19 = distinct !DILexicalBlock(scope: !10, file: !1, line: 3, column: 9)
+!20 = !DILocation(line: 3, column: 9, scope: !10)
+!21 = !DILocation(line: 4, column: 20, scope: !22)
+!22 = distinct !DILexicalBlock(scope: !19, file: !1, line: 3, column: 17)
+!23 = !DILocalVariable(name: "a1", scope: !22, file: !1, line: 4, type: !13)
+!24 = !DILocation(line: 0, scope: !22)
+!25 = !DILocation(line: 5, column: 21, scope: !22)
+!26 = !DILocalVariable(name: "a2", scope: !22, file: !1, line: 5, type: !13)
+!27 = !DILocalVariable(name: "ret", scope: !10, file: !1, line: 2, type: !13)
+!28 = !DILocation(line: 7, column: 5, scope: !22)
+!29 = !DILocation(line: 8, column: 20, scope: !30)
+!30 = distinct !DILexicalBlock(scope: !19, file: !1, line: 7, column: 12)
+!31 = !DILocalVariable(name: "b1", scope: !30, file: !1, line: 8, type: !13)
+!32 = !DILocation(line: 0, scope: !30)
+!33 = !DILocation(line: 9, column: 21, scope: !30)
+!34 = !DILocalVariable(name: "b2", scope: !30, file: !1, line: 9, type: !13)
+!35 = !DILocation(line: 0, scope: !19)
+!36 = !DILocation(line: 12, column: 5, scope: !10)
\ No newline at end of file
|
|
Emm, I think the right way to fix this is to avoid sorting pointers and make |
|
Ok, I will close this PR. Could you please give me a link to inspect the cause of the failure just like in #77419 ? |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR fixes issue #77415 and is revised from PR #77419 .
PR #77419 breaks the newly added test in the same PR on windows, because GVNSink is non-deterministic when sorting BasicBlock* pointers. This is reflected in the failure report.
According to the report, what the CheckFile want to match is the
%a.sink, however there is%b.sink. But this mismatch does not mean that this commit is wrong, since the occurance of either%a.sinkor%b.sinkis correct. The root cause of this test failure is the stricted check rule in the regression test commited.So I refined the regression test with a more general check rule to only detect whether there is a
.sinkinstruction in theif.endblock. Hope this won't fail the test. If this PR still fails to build, I will close this PR and try to find another right way to fix this.