[LoopUnroll] Remove redundant debug instructions after blocks have been merged #91246
Conversation
|
@llvm/pr-subscribers-llvm-transforms Author: chenlin (coderchenlin) ChangesRemove redundant debug instructions after blocks have been merged into the predecessor, It can reduce some compile time in some cases. This change only fixes the situation of loop unrolling, and other situations are not considered. "RemoveRedundantDbgInstrs" seems to be very time-consuming. Thus, we just add here after the "Dest" has been merged into the "Fold", this may be a more targeted solution!!! fixes: #89073 Full diff: https://github.com/llvm/llvm-project/pull/91246.diff 4 Files Affected:
diff --git a/llvm/lib/Transforms/Utils/LoopUnroll.cpp b/llvm/lib/Transforms/Utils/LoopUnroll.cpp
index 20978cf2e748ab..13d388f3b0a6ae 100644
--- a/llvm/lib/Transforms/Utils/LoopUnroll.cpp
+++ b/llvm/lib/Transforms/Utils/LoopUnroll.cpp
@@ -974,6 +974,9 @@ llvm::UnrollLoop(Loop *L, UnrollLoopOptions ULO, LoopInfo *LI,
/*MSSAU=*/nullptr, /*MemDep=*/nullptr,
/*PredecessorWithTwoSuccessors=*/false,
DTUToUse ? nullptr : DT)) {
+ // Remove redundant Dbg instructions for reducing compile time.
+ if (Fold->getParent()->getSubprogram())
+ RemoveRedundantDbgInstrs(Fold);
// Dest has been folded into Fold. Update our worklists accordingly.
std::replace(Latches.begin(), Latches.end(), Dest, Fold);
llvm::erase(UnrolledLoopBlocks, Dest);
diff --git a/llvm/test/Transforms/LoopUnroll/unroll-remove-redundant-dbg.ll b/llvm/test/Transforms/LoopUnroll/unroll-remove-redundant-dbg.ll
new file mode 100644
index 00000000000000..66cd4d45444360
--- /dev/null
+++ b/llvm/test/Transforms/LoopUnroll/unroll-remove-redundant-dbg.ll
@@ -0,0 +1,45 @@
+; RUN: opt < %s -S -passes=loop-unroll | FileCheck %s
+
+define i64 @d(i1 %tobool.not, i32 %add, i64 %conv23) !dbg !14{
+entry:
+ br label %for.body
+
+for.body: ; preds = %for.body, %entry
+ ; There should be only one "llvm.dbg.vale" after loop unrolling
+ ; CHECK: call void @llvm.dbg.value
+ ; CHECK-NOT: call void @llvm.dbg.value
+
+ %k.045 = phi i64 [ 0, %entry ], [ %k.046, %for.body ]
+ tail call void @llvm.dbg.value(metadata i32 0, metadata !13, metadata !DIExpression()), !dbg !17
+ %k.046 = add nuw nsw i64 %k.045, 1
+ %exitcond = icmp ne i64 %k.046, 5
+ br i1 %exitcond, label %for.body, label %for.end22
+
+for.end22: ; preds = %for.body
+ ret i64 %k.046
+}
+
+; Function Attrs: nocallback nofree nosync nounwind speculatable willreturn memory(none)
+declare void @llvm.dbg.value(metadata, metadata, metadata)
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!12}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C89, file: !1, producer: "clang version 19.0.0git (https://github.com/llvm/llvm-project.git ec062f5b33ed22c61742e3c1486f6cba915801e0)", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, globals: !2, splitDebugInlining: false, nameTableKind: None)
+!1 = !DIFile(filename: "unroll-remove-redundant-dbg.c", directory: "", checksumkind: CSK_MD5, checksum: "aa30a1d8c04deb9b0f3885c258d2b674")
+!2 = !{!3, !8, !10}
+!3 = !DIGlobalVariableExpression(var: !4, expr: !DIExpression())
+!4 = distinct !DIGlobalVariable(name: "a", scope: !0, file: !1, line: 2, type: !5, isLocal: false, isDefinition: true)
+!5 = !DIDerivedType(tag: DW_TAG_typedef, name: "uint32_t", file: !6, line: 198, baseType: !7)
+!6 = !DIFile(filename: "/usr/include/stdint.h", directory: "", checksumkind: CSK_MD5, checksum: "da031bcff2d0c1d65aa92e7e68a44ef3")
+!7 = !DIBasicType(name: "unsigned int", size: 32, encoding: DW_ATE_unsigned)
+!8 = !DIGlobalVariableExpression(var: !9, expr: !DIExpression())
+!9 = distinct !DIGlobalVariable(name: "c", scope: !0, file: !1, line: 2, type: !5, isLocal: false, isDefinition: true)
+!10 = !DIGlobalVariableExpression(var: !11, expr: !DIExpression())
+!11 = distinct !DIGlobalVariable(name: "b", scope: !0, file: !1, line: 2, type: !5, isLocal: false, isDefinition: true)
+!12 = !{i32 2, !"Debug Info Version", i32 3}
+!13 = !DILocalVariable(name: "f", scope: !14, file: !1, line: 4, type: !5)
+!14 = distinct !DISubprogram(name: "d", scope: !1, file: !1, line: 3, type: !15, scopeLine: 3, flags: DIFlagAllCallsDescribed, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !0, retainedNodes: !16)
+!15 = !DISubroutineType(types: !16)
+!16 = !{}
+!17 = !DILocation(line: 0, scope: !14)
diff --git a/llvm/test/Transforms/SimplifyCFG/hoist-dbgvalue-inlined.ll b/llvm/test/Transforms/SimplifyCFG/hoist-dbgvalue-inlined.ll
index e00d1daf71de58..5af73e789f11ce 100644
--- a/llvm/test/Transforms/SimplifyCFG/hoist-dbgvalue-inlined.ll
+++ b/llvm/test/Transforms/SimplifyCFG/hoist-dbgvalue-inlined.ll
@@ -9,7 +9,6 @@ init:
; CHECK: %vala = load i64, ptr %ptr
; CHECK-NEXT: call void @llvm.dbg.value(metadata i64 %vala, metadata [[MD:![0-9]*]]
-; CHECK-NEXT: call void @llvm.dbg.value(metadata i64 %vala, metadata [[MD]]
; CHECK-NEXT: %valbmasked = and i64 %vala, 1
a: ; preds = %init
diff --git a/llvm/test/Transforms/SimplifyCFG/hoist-dbgvalue.ll b/llvm/test/Transforms/SimplifyCFG/hoist-dbgvalue.ll
index af7da45ec089cc..c5d723c4e3dd61 100644
--- a/llvm/test/Transforms/SimplifyCFG/hoist-dbgvalue.ll
+++ b/llvm/test/Transforms/SimplifyCFG/hoist-dbgvalue.ll
@@ -47,7 +47,6 @@ define i1 @hoist_with_debug2(i32 %x) !dbg !22 {
; CHECK-NEXT: entry:
; CHECK-NEXT: [[TOBOOL_NOT:%.*]] = icmp ugt i32 [[X:%.*]], 2
; CHECK-NEXT: call void @llvm.dbg.value(metadata i32 [[X]], metadata [[META21:![0-9]+]], metadata !DIExpression()), !dbg [[DBG23:![0-9]+]]
-; CHECK-NEXT: call void @llvm.dbg.value(metadata i32 [[X]], metadata [[META21]], metadata !DIExpression()), !dbg [[DBG23]]
; CHECK-NEXT: [[DOT:%.*]] = select i1 [[TOBOOL_NOT]], i1 false, i1 true
; CHECK-NEXT: ret i1 [[DOT]]
;
|
coderchenlin
force-pushed
the
br-loop-unroll-remove-redundant-debug-instrs
branch
from
1e53508 to
0cc0242
Compare
|
@slackito @aeubanks , Hi, guys, I'm sorry to bother you. I just give a more targeted solution for fixing compile-time. and I also test my case and your provided case, the compile-time is Ok. Could you help me to review this change, and give some advices? Uploading slowdown.ll.txt… |
|
@fhahn, Apologies for your time, Could you help me to review this change again? As @slackito found, the original solution will introduce quadratic behavior in SimplifyCFG and make compilation time increase. Thus, I change the location, and put the changes in loop-unrolling, and it won't have any effects in SimplifyCFG. |
There was a problem hiding this comment.
this seems like it would still run into a similar issue where we may repeatedly call RemoveRedundantDbgInstrs on some set of growing dbg instructions, if loop-unroll manages to create a bunch of mergeable blocks. can we ensure that we only call it on an instruction at most once by calling RemoveRedundantDbgInstrs not every time we merge a block, but on a set of merged blocks?
e.g. if we merge A->B->C, then D->E->F, we should call RemoveRedundantDbgInstrs on C once we see we can't merge C->D, and then we call RemoveRedundantDbgInstrs on F when we're done (and we saw that F did indeed have a block merged into it)
| @@ -0,0 +1,45 @@ | |||
| ; RUN: opt < %s -S -passes=loop-unroll | FileCheck %s | |||
There was a problem hiding this comment.
use llvm/utils/update_test_checks.py
There was a problem hiding this comment.
this isn't properly using update_test_checks.py, see the comments in that file. I believe the CHECK lines below need to be removed first. as the comments in update_test_checks.py say, please verify that there is a difference in IR with and without the loop unroll cleanup change
There was a problem hiding this comment.
Thanks, I use update_test_checks.py to generate CHECKs, and keep some important contents
There was a problem hiding this comment.
you shouldn't manually adjust CHECK lines generated by update_test_checks.py, just keep whatever it generates
There was a problem hiding this comment.
Thanks, keep all the CHECK lines.
coderchenlin
force-pushed
the
br-loop-unroll-remove-redundant-debug-instrs
branch
from
0cc0242 to
ee4bf10
Compare
Thanks for your advice, @aeubanks , I put the process in Before: Now： |
coderchenlin
force-pushed
the
br-loop-unroll-remove-redundant-debug-instrs
branch
from
ee4bf10 to
7a91ca1
Compare
| @@ -0,0 +1,45 @@ | |||
| ; RUN: opt < %s -S -passes=loop-unroll | FileCheck %s | |||
There was a problem hiding this comment.
this isn't properly using update_test_checks.py, see the comments in that file. I believe the CHECK lines below need to be removed first. as the comments in update_test_checks.py say, please verify that there is a difference in IR with and without the loop unroll cleanup change
coderchenlin
force-pushed
the
br-loop-unroll-remove-redundant-debug-instrs
branch
from
7a91ca1 to
1af13e4
Compare
been merged into the predecessor, It can reduce some compile time in some cases. This change only fixes the situation of loop unrolling, and other situations are not considered. "RemoveRedundantDbgInstrs" seems to be very time-consuming. Thus, we call 'RemoveRedundantDbgInstrs' in 'simplifyLoopAfterUnroll'. fixes: llvm#89073
coderchenlin
force-pushed
the
br-loop-unroll-remove-redundant-debug-instrs
branch
from
1af13e4 to
0aeaa59
Compare
Remove redundant debug instructions after blocks have been merged into the predecessor, It can reduce some compile time in some cases.
This change only fixes the situation of loop unrolling, and other situations are not considered. "RemoveRedundantDbgInstrs" seems to be very time-consuming. Thus, we just add here after the "Dest" has been merged into the "Fold", this may be a more targeted solution!!!
fixes: #89073