[llvm] [BranchFolding] Merge debug locs on common hoisted code (PR #140063)
Orlando Cazalet-Hyams via llvm-commits
llvm-commits at lists.llvm.org
Mon May 19 02:46:09 PDT 2025
https://github.com/OCHyams updated https://github.com/llvm/llvm-project/pull/140063
>From c1ab54e275b3a76de111c368e589e8e41fdab619 Mon Sep 17 00:00:00 2001
From: Orlando Cazalet-Hyams <orlando.hyams at sony.com>
Date: Thu, 15 May 2025 14:13:04 +0100
Subject: [PATCH 1/4] [BranchFolding] Merge debug locs on common hoisted code
---
llvm/lib/CodeGen/BranchFolding.cpp | 29 ++++-
llvm/test/DebugInfo/X86/branch-folder-dbg.mir | 111 ++++++++++++++++++
2 files changed, 139 insertions(+), 1 deletion(-)
create mode 100644 llvm/test/DebugInfo/X86/branch-folder-dbg.mir
diff --git a/llvm/lib/CodeGen/BranchFolding.cpp b/llvm/lib/CodeGen/BranchFolding.cpp
index 6f5afbd2a996a..7dc87dbabf400 100644
--- a/llvm/lib/CodeGen/BranchFolding.cpp
+++ b/llvm/lib/CodeGen/BranchFolding.cpp
@@ -2071,7 +2071,34 @@ bool BranchFolder::HoistCommonCodeInSuccs(MachineBasicBlock *MBB) {
if (!HasDups)
return false;
- MBB->splice(Loc, TBB, TBB->begin(), TIB);
+ // Hoist the instructions from [T.begin, TIB) and then delete [F.begin, FIB).
+ // Merge the debug locations. FIXME: We should do something with the
+ // debug instructions too (from BOTH branches).
+ {
+ // TIB and FIB point to the end of the regions to hoist/merge in TBB and
+ // FBB.
+ MachineBasicBlock::iterator FE = FIB;
+ MachineBasicBlock::iterator FI = FBB->begin();
+ for (MachineBasicBlock::iterator TI :
+ make_early_inc_range(make_range(TBB->begin(), TIB))) {
+ // Move debug instructions and pseudo probes without modifying them.
+ // FIXME: This is the wrong thing to do for debug locations, which
+ // should at least be killed.
+ if (TI->isDebugOrPseudoInstr()) {
+ TI->moveBefore(&*Loc);
+ continue;
+ }
+
+ // Get the next non-meta instruction in FBB.
+ FI = skipDebugInstructionsForward(FI, FE, false);
+ assert(FI != FE && "Expected non-debug lockstep");
+
+ // Merge debug locs on hoisted instructions.
+ TI->setDebugLoc(
+ DILocation::getMergedLocation(TI->getDebugLoc(), FI->getDebugLoc()));
+ TI->moveBefore(&*Loc);
+ }
+ }
FBB->erase(FBB->begin(), FIB);
if (UpdateLiveIns)
diff --git a/llvm/test/DebugInfo/X86/branch-folder-dbg.mir b/llvm/test/DebugInfo/X86/branch-folder-dbg.mir
new file mode 100644
index 0000000000000..5c38fd2a43829
--- /dev/null
+++ b/llvm/test/DebugInfo/X86/branch-folder-dbg.mir
@@ -0,0 +1,111 @@
+# RUN: llc %s --start-before=branch-folder --stop-after=branch-folder -o - | FileCheck %s
+
+## Check that common instructions hoisted from `if.then` and `if.else` into
+## common pred `entry` get merged debug locations.
+
+## FIXME: The debug instructions handling here is wrong.
+
+# CHECK: bb.0
+# CHECK: CALL64pcrel32 @f, csr_64, implicit $rsp, implicit $ssp, implicit-def $rsp, implicit-def $ssp, implicit-def $rax
+## --- Start splice from bb.2.if.else ---
+# CHECK-NEXT: DBG_VALUE 2, $noreg, ![[#]], !DIExpression(), debug-location ![[#]]
+# CHECK-NEXT: $edi = MOV32r0 implicit-def dead $eflags, debug-location !DILocation(line: 0, scope: ![[#]])
+## --- End splice --------------
+# CHECK-NEXT: TEST64rr killed renamable $rax, renamable $rax, implicit-def $eflags
+# CHECK-NEXT: JCC_1 %bb.2, 9, implicit killed $eflags
+# CHECK: bb.1
+
+--- |
+ 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"
+ target triple = "x86_64-unknown-linux-gnu"
+
+ declare dso_local noundef i64 @f() local_unnamed_addr
+
+ define dso_local noundef i32 @g() local_unnamed_addr !dbg !7 {
+ %call = tail call noundef i64 @f()
+ %cmp1 = icmp sgt i64 0, %call
+ %conv2 = trunc i64 0 to i32
+ br i1 %cmp1, label %if.then, label %if.else
+
+ if.then: ; preds = %0
+ #dbg_value(i64 0, !11, !DIExpression(), !13)
+ tail call void @_Z3fooii(i32 noundef %conv2, i32 noundef 0), !dbg !14
+ #dbg_value(i64 1, !11, !DIExpression(), !13)
+ br label %if.end, !dbg !15
+
+ if.else: ; preds = %0
+ #dbg_value(i64 2, !11, !DIExpression(), !13)
+ tail call void @_Z3barii(i32 noundef %conv2, i32 noundef 1), !dbg !16
+ #dbg_value(i64 3, !11, !DIExpression(), !13)
+ br label %if.end, !dbg !17
+
+ if.end: ; preds = %if.else, %if.then
+ ret i32 2
+ }
+
+ declare void @_Z3fooii(i32 noundef, i32 noundef) local_unnamed_addr
+
+ declare void @_Z3barii(i32 noundef, i32 noundef) local_unnamed_addr
+
+ !llvm.module.flags = !{!0, !1}
+ !llvm.ident = !{!2}
+ !llvm.dbg.cu = !{!3}
+ !llvm.debugify = !{!5, !6}
+
+ !0 = !{i32 7, !"Dwarf Version", i32 5}
+ !1 = !{i32 2, !"Debug Info Version", i32 3}
+ !2 = !{!"clang version 21.0.0"}
+ !3 = distinct !DICompileUnit(language: DW_LANG_C, file: !4, producer: "debugify", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug)
+ !4 = !DIFile(filename: "test.nodbg.ll", directory: "/")
+ !5 = !{i32 15}
+ !6 = !{i32 7}
+ !7 = distinct !DISubprogram(name: "g", linkageName: "g", scope: null, file: !4, line: 1, type: !8, scopeLine: 1, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !3, retainedNodes: !10)
+ !8 = !DISubroutineType(types: !9)
+ !9 = !{}
+ !10 = !{!11}
+ !11 = !DILocalVariable(name: "1", scope: !7, file: !4, line: 3, type: !12)
+ !12 = !DIBasicType(name: "ty64", size: 64, encoding: DW_ATE_unsigned)
+ !13 = !DILocation(line: 3, column: 1, scope: !7)
+ !14 = !DILocation(line: 9, column: 1, scope: !7)
+ !15 = !DILocation(line: 10, column: 1, scope: !7)
+ !16 = !DILocation(line: 11, column: 1, scope: !7)
+ !17 = !DILocation(line: 12, column: 1, scope: !7)
+...
+---
+name: g
+body: |
+ bb.0 (%ir-block.0):
+ successors: %bb.1(0x40000000), %bb.2(0x40000000)
+
+ frame-setup PUSH64r undef $rax, implicit-def $rsp, implicit $rsp
+ frame-setup CFI_INSTRUCTION def_cfa_offset 16
+ CALL64pcrel32 @f, csr_64, implicit $rsp, implicit $ssp, implicit-def $rsp, implicit-def $ssp, implicit-def $rax
+ TEST64rr killed renamable $rax, renamable $rax, implicit-def $eflags
+ JCC_1 %bb.2, 9, implicit killed $eflags
+ JMP_1 %bb.1
+
+ bb.1.if.then:
+ successors: %bb.3(0x80000000)
+
+ DBG_VALUE 0, $noreg, !11, !DIExpression(), debug-location !13
+ $edi = MOV32r0 implicit-def dead $eflags, debug-location !14
+ $esi = MOV32r0 implicit-def dead $eflags, debug-location !14
+ CALL64pcrel32 target-flags(x86-plt) @_Z3fooii, csr_64, implicit $rsp, implicit $ssp, implicit killed $edi, implicit killed $esi, implicit-def $rsp, implicit-def $ssp, debug-location !14
+ DBG_VALUE 1, $noreg, !11, !DIExpression(), debug-location !13
+ JMP_1 %bb.3, debug-location !15
+
+ bb.2.if.else:
+ successors: %bb.3(0x80000000)
+
+ DBG_VALUE 2, $noreg, !11, !DIExpression(), debug-location !13
+ $edi = MOV32r0 implicit-def dead $eflags, debug-location !16
+ $esi = MOV32ri 1, debug-location !16
+ CALL64pcrel32 target-flags(x86-plt) @_Z3barii, csr_64, implicit $rsp, implicit $ssp, implicit killed $edi, implicit killed $esi, implicit-def $rsp, implicit-def $ssp, debug-location !16
+ DBG_VALUE 3, $noreg, !11, !DIExpression(), debug-location !13
+
+ bb.3.if.end:
+ $eax = MOV32ri 2
+ $rcx = frame-destroy POP64r implicit-def $rsp, implicit $rsp
+ frame-destroy CFI_INSTRUCTION def_cfa_offset 8
+ RET 0, $eax
+...
>From 0d44dea999c98d2396b68bb7fe745d373349fa47 Mon Sep 17 00:00:00 2001
From: Orlando Cazalet-Hyams <orlando.hyams at sony.com>
Date: Thu, 15 May 2025 16:53:05 +0100
Subject: [PATCH 2/4] Improve assert (and fix bug - FI needs to advance)
Covered by existings tests e.g.
test/CodeGen/ARM/aes-erratum-fix.ll
---
llvm/lib/CodeGen/BranchFolding.cpp | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/llvm/lib/CodeGen/BranchFolding.cpp b/llvm/lib/CodeGen/BranchFolding.cpp
index 7dc87dbabf400..14fd121df1303 100644
--- a/llvm/lib/CodeGen/BranchFolding.cpp
+++ b/llvm/lib/CodeGen/BranchFolding.cpp
@@ -2091,12 +2091,14 @@ bool BranchFolder::HoistCommonCodeInSuccs(MachineBasicBlock *MBB) {
// Get the next non-meta instruction in FBB.
FI = skipDebugInstructionsForward(FI, FE, false);
- assert(FI != FE && "Expected non-debug lockstep");
+ assert(TI->isIdenticalTo(*FI, MachineInstr::CheckKillDead) &&
+ "Expected non-debug lockstep");
// Merge debug locs on hoisted instructions.
TI->setDebugLoc(
DILocation::getMergedLocation(TI->getDebugLoc(), FI->getDebugLoc()));
TI->moveBefore(&*Loc);
+ ++FI;
}
}
FBB->erase(FBB->begin(), FIB);
>From bf6e365778d769255615b64e7e00325642ad14a4 Mon Sep 17 00:00:00 2001
From: Orlando Cazalet-Hyams <orlando.hyams at sony.com>
Date: Mon, 19 May 2025 10:44:28 +0100
Subject: [PATCH 3/4] soften isIdenticalTo check in assert to account for the
previous loop modifying some kill markers after its own isIdenticalTo checks
---
llvm/lib/CodeGen/BranchFolding.cpp | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/llvm/lib/CodeGen/BranchFolding.cpp b/llvm/lib/CodeGen/BranchFolding.cpp
index 14fd121df1303..e799bad93a565 100644
--- a/llvm/lib/CodeGen/BranchFolding.cpp
+++ b/llvm/lib/CodeGen/BranchFolding.cpp
@@ -2091,7 +2091,9 @@ bool BranchFolder::HoistCommonCodeInSuccs(MachineBasicBlock *MBB) {
// Get the next non-meta instruction in FBB.
FI = skipDebugInstructionsForward(FI, FE, false);
- assert(TI->isIdenticalTo(*FI, MachineInstr::CheckKillDead) &&
+ // NOTE: The loop above checks CheckKillDead but we can't do that here as
+ // it goes on to modifies some kill markers.
+ assert(TI->isIdenticalTo(*FI, MachineInstr::CheckDefs) &&
"Expected non-debug lockstep");
// Merge debug locs on hoisted instructions.
>From 76d342d2613d97a9d3f1d535290030f201f74960 Mon Sep 17 00:00:00 2001
From: Orlando Cazalet-Hyams <orlando.hyams at sony.com>
Date: Mon, 19 May 2025 10:45:51 +0100
Subject: [PATCH 4/4] fix comment
---
llvm/lib/CodeGen/BranchFolding.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/llvm/lib/CodeGen/BranchFolding.cpp b/llvm/lib/CodeGen/BranchFolding.cpp
index e799bad93a565..d10e6722f4548 100644
--- a/llvm/lib/CodeGen/BranchFolding.cpp
+++ b/llvm/lib/CodeGen/BranchFolding.cpp
@@ -2092,7 +2092,7 @@ bool BranchFolder::HoistCommonCodeInSuccs(MachineBasicBlock *MBB) {
// Get the next non-meta instruction in FBB.
FI = skipDebugInstructionsForward(FI, FE, false);
// NOTE: The loop above checks CheckKillDead but we can't do that here as
- // it goes on to modifies some kill markers.
+ // it modifies some kill markers after the check.
assert(TI->isIdenticalTo(*FI, MachineInstr::CheckDefs) &&
"Expected non-debug lockstep");
More information about the llvm-commits
mailing list