From 997c260bf80276fbd1e1ad58ae6052c3f727eadb Mon Sep 17 00:00:00 2001 From: Orlando Cazalet-Hyams Date: Thu, 15 May 2025 14:08:45 +0100 Subject: [PATCH 01/10] [BranchFolding] Kill common hoisted debug instructions branch-folder hoists common instructions from TBB and FBB into their pred. Without this patch it achieves this by splicing the instructions from TBB and deleting the common ones in FBB. That moves the debug locations and debug instructions from TBB into the pred without modification, which is not ideal. Debug locations are handled in #140063. This patch handles debug instructions - in the simplest way possible, which is to just kill (undef) them. We kill and hoist the ones in FBB as well as TBB because otherwise the fact there's an assignment on the code path is deleted (which might lead to a prior location extending further than it should). There's possibly something we could do to preserve some variable locations in some cases, but this is the easiest not-incorrect thing to do. Note I had to replace the constant DBG_VALUEs to use registers in the test- it turns out setDebugValueUndef doesn't undef constant DBG_VALUEs... which feels wrong to me, but isn't something I want to touch right now. --- llvm/lib/CodeGen/BranchFolding.cpp | 44 +++++++++++++++++-- llvm/test/DebugInfo/X86/branch-folder-dbg.mir | 34 ++++++++------ 2 files changed, 60 insertions(+), 18 deletions(-) diff --git a/llvm/lib/CodeGen/BranchFolding.cpp b/llvm/lib/CodeGen/BranchFolding.cpp index 3b3e7a418feb5..85712833d844a 100644 --- a/llvm/lib/CodeGen/BranchFolding.cpp +++ b/llvm/lib/CodeGen/BranchFolding.cpp @@ -25,6 +25,7 @@ #include "llvm/Analysis/ProfileSummaryInfo.h" #include "llvm/CodeGen/Analysis.h" #include "llvm/CodeGen/BranchFoldingPass.h" +#include "llvm/CodeGen/GlobalISel/MachineIRBuilder.h" #include "llvm/CodeGen/MBFIWrapper.h" #include "llvm/CodeGen/MachineBlockFrequencyInfo.h" #include "llvm/CodeGen/MachineBranchProbabilityInfo.h" @@ -2083,19 +2084,53 @@ bool BranchFolder::HoistCommonCodeInSuccs(MachineBasicBlock *MBB) { if (TBB == FBB) { MBB->splice(Loc, TBB, TBB->begin(), TIB); } else { + // Merge the debug locations, and hoist and kill the debug instructions from + // both branches. FIXME: We could probably try harder to preserve some debug + // instructions (but at least this isn't producing wrong locations). + MachineIRBuilder MIRBuilder(*MBB, Loc); + auto HoistAndKillDbgInstr = + [&MIRBuilder](MachineBasicBlock::iterator DI, + MachineBasicBlock::iterator InsertBefore) { + assert(DI->isDebugInstr() && "Expected a debug instruction"); + if (DI->isDebugRef()) { + MIRBuilder.setDebugLoc(DI->getDebugLoc()); + MIRBuilder.buildDirectDbgValue(0, DI->getDebugVariable(), + DI->getDebugExpression()); + return; + } + + DI->setDebugValueUndef(); + DI->moveBefore(&*InsertBefore); + }; + // 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 (and hoisted from BOTH blocks). - if (TI->isDebugOrPseudoInstr()) { + + // Hoist and kill debug instructions from FBB. Skip over pseudo + // instructions. After this loop FI points to the next non-debug + // instruction to hoist (checked in assert after the TBB debug + // instruction handling code). + while (FI->isDebugOrPseudoInstr()) { + assert(FI != FE && "Unexpected end of FBB range"); + MachineBasicBlock::iterator FINext = std::next(FI); + HoistAndKillDbgInstr(FI, Loc); + FI = FINext; + } + + // Move pseudo probes without modifying them. + if (TI->isPseudoProbe()) { TI->moveBefore(&*Loc); continue; } + // Kill debug instructions before moving. + if (TI->isDebugInstr()) { + HoistAndKillDbgInstr(TI, Loc); + continue; + } // Get the next non-meta instruction in FBB. FI = skipDebugInstructionsForward(FI, FE, false); @@ -2111,6 +2146,7 @@ bool BranchFolder::HoistCommonCodeInSuccs(MachineBasicBlock *MBB) { ++FI; } } + assert(FIB->getParent() == FBB && "Unexpectedly moved FIB?"); 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 index 5c38fd2a43829..831b263fdccd4 100644 --- a/llvm/test/DebugInfo/X86/branch-folder-dbg.mir +++ b/llvm/test/DebugInfo/X86/branch-folder-dbg.mir @@ -1,16 +1,21 @@ # 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. +## common pred `entry` get merged debug locations. The debug instructions from +## both branches should get hoisted and killed. +## +## The MIR debug instructions have been modified by hand in order to check they +## can be killed. # 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 -------------- +## --- Start splice from bb.2.if.else (and debug instructions from bb.1.if.then) --- +# CHECK-NEXT: DBG_VALUE $noreg, $noreg, ![[#]], !DIExpression(), debug-location ![[#]] +# CHECK-NEXT: DBG_VALUE $noreg, $noreg, ![[#]], !DIExpression(), debug-location ![[#]] +# CHECK-NEXT: $edi = MOV32r0 implicit-def dead $eflags, debug-instr-number 2, debug-location !DILocation(line: 0, scope: ![[#]]) +# CHECK-NEXT: DBG_VALUE $noreg, $noreg, ![[#]], !DIExpression(DW_OP_LLVM_arg, 0), debug-location ![[#]] +# CHECK-NEXT: DBG_VALUE $noreg, $noreg, ![[#]], !DIExpression(DW_OP_LLVM_arg, 0), debug-location ![[#]] +## --- 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 @@ -73,6 +78,8 @@ ... --- name: g +tracksRegLiveness: true +isSSA: false body: | bb.0 (%ir-block.0): successors: %bb.1(0x40000000), %bb.2(0x40000000) @@ -86,22 +93,21 @@ body: | 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 + DBG_VALUE $esi, $noreg, !11, !DIExpression(), debug-location !13 + $edi = MOV32r0 implicit-def dead $eflags, debug-instr-number 1, debug-location !14 + DBG_INSTR_REF !11, !DIExpression(DW_OP_LLVM_arg, 0), dbg-instr-ref(1, 0), debug-location !13 $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 + DBG_VALUE $esp, $noreg, !11, !DIExpression(), debug-location !13 + $edi = MOV32r0 implicit-def dead $eflags, debug-instr-number 2, debug-location !16 + DBG_INSTR_REF !11, !DIExpression(DW_OP_LLVM_arg, 0), dbg-instr-ref(2, 0), debug-location !13 $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 From 8810fc8cc22511dcfa2e2b97e08fe6595ceab4a3 Mon Sep 17 00:00:00 2001 From: Orlando Cazalet-Hyams Date: Thu, 29 May 2025 18:12:03 +0100 Subject: [PATCH 02/10] replace MachineIRBuilder with BuildMI --- llvm/lib/CodeGen/BranchFolding.cpp | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/llvm/lib/CodeGen/BranchFolding.cpp b/llvm/lib/CodeGen/BranchFolding.cpp index 85712833d844a..0b31db878803d 100644 --- a/llvm/lib/CodeGen/BranchFolding.cpp +++ b/llvm/lib/CodeGen/BranchFolding.cpp @@ -25,7 +25,6 @@ #include "llvm/Analysis/ProfileSummaryInfo.h" #include "llvm/CodeGen/Analysis.h" #include "llvm/CodeGen/BranchFoldingPass.h" -#include "llvm/CodeGen/GlobalISel/MachineIRBuilder.h" #include "llvm/CodeGen/MBFIWrapper.h" #include "llvm/CodeGen/MachineBlockFrequencyInfo.h" #include "llvm/CodeGen/MachineBranchProbabilityInfo.h" @@ -49,6 +48,8 @@ #include "llvm/IR/Function.h" #include "llvm/InitializePasses.h" #include "llvm/MC/LaneBitmask.h" +#include "llvm/MC/MCInstrDesc.h" +#include "llvm/MC/MCInstrInfo.h" #include "llvm/MC/MCRegisterInfo.h" #include "llvm/Pass.h" #include "llvm/Support/BlockFrequency.h" @@ -2087,15 +2088,18 @@ bool BranchFolder::HoistCommonCodeInSuccs(MachineBasicBlock *MBB) { // Merge the debug locations, and hoist and kill the debug instructions from // both branches. FIXME: We could probably try harder to preserve some debug // instructions (but at least this isn't producing wrong locations). - MachineIRBuilder MIRBuilder(*MBB, Loc); + MachineInstrBuilder MIRBuilder(*MBB->getParent(), Loc); auto HoistAndKillDbgInstr = - [&MIRBuilder](MachineBasicBlock::iterator DI, - MachineBasicBlock::iterator InsertBefore) { + [MBB](MachineBasicBlock::iterator DI, + MachineBasicBlock::iterator InsertBefore) { assert(DI->isDebugInstr() && "Expected a debug instruction"); if (DI->isDebugRef()) { - MIRBuilder.setDebugLoc(DI->getDebugLoc()); - MIRBuilder.buildDirectDbgValue(0, DI->getDebugVariable(), - DI->getDebugExpression()); + const TargetInstrInfo *TII = + MBB->getParent()->getSubtarget().getInstrInfo(); + const MCInstrDesc &DBGV = TII->get(TargetOpcode::DBG_VALUE); + DI = BuildMI(*MBB->getParent(), DI->getDebugLoc(), DBGV, false, 0, + DI->getDebugVariable(), DI->getDebugExpression()); + MBB->insert(InsertBefore, &*DI); return; } From fdc76f8d5948c3cb4b3c7689f4e368c0c605c765 Mon Sep 17 00:00:00 2001 From: Orlando Cazalet-Hyams Date: Thu, 29 May 2025 18:13:50 +0100 Subject: [PATCH 03/10] use capture instead of param --- llvm/lib/CodeGen/BranchFolding.cpp | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/llvm/lib/CodeGen/BranchFolding.cpp b/llvm/lib/CodeGen/BranchFolding.cpp index 0b31db878803d..2f16e960645cf 100644 --- a/llvm/lib/CodeGen/BranchFolding.cpp +++ b/llvm/lib/CodeGen/BranchFolding.cpp @@ -2090,8 +2090,7 @@ bool BranchFolder::HoistCommonCodeInSuccs(MachineBasicBlock *MBB) { // instructions (but at least this isn't producing wrong locations). MachineInstrBuilder MIRBuilder(*MBB->getParent(), Loc); auto HoistAndKillDbgInstr = - [MBB](MachineBasicBlock::iterator DI, - MachineBasicBlock::iterator InsertBefore) { + [MBB, Loc](MachineBasicBlock::iterator DI) { assert(DI->isDebugInstr() && "Expected a debug instruction"); if (DI->isDebugRef()) { const TargetInstrInfo *TII = @@ -2099,12 +2098,12 @@ bool BranchFolder::HoistCommonCodeInSuccs(MachineBasicBlock *MBB) { const MCInstrDesc &DBGV = TII->get(TargetOpcode::DBG_VALUE); DI = BuildMI(*MBB->getParent(), DI->getDebugLoc(), DBGV, false, 0, DI->getDebugVariable(), DI->getDebugExpression()); - MBB->insert(InsertBefore, &*DI); + MBB->insert(Loc, &*DI); return; } DI->setDebugValueUndef(); - DI->moveBefore(&*InsertBefore); + DI->moveBefore(&*Loc); }; // TIB and FIB point to the end of the regions to hoist/merge in TBB and @@ -2121,7 +2120,7 @@ bool BranchFolder::HoistCommonCodeInSuccs(MachineBasicBlock *MBB) { while (FI->isDebugOrPseudoInstr()) { assert(FI != FE && "Unexpected end of FBB range"); MachineBasicBlock::iterator FINext = std::next(FI); - HoistAndKillDbgInstr(FI, Loc); + HoistAndKillDbgInstr(FI); FI = FINext; } @@ -2132,7 +2131,7 @@ bool BranchFolder::HoistCommonCodeInSuccs(MachineBasicBlock *MBB) { } // Kill debug instructions before moving. if (TI->isDebugInstr()) { - HoistAndKillDbgInstr(TI, Loc); + HoistAndKillDbgInstr(TI); continue; } From 9d38e24961ef06a5fecae21e767e0defe8dcf436 Mon Sep 17 00:00:00 2001 From: Orlando Cazalet-Hyams Date: Thu, 29 May 2025 18:15:58 +0100 Subject: [PATCH 04/10] format --- llvm/lib/CodeGen/BranchFolding.cpp | 29 ++++++++++++++--------------- 1 file changed, 14 insertions(+), 15 deletions(-) diff --git a/llvm/lib/CodeGen/BranchFolding.cpp b/llvm/lib/CodeGen/BranchFolding.cpp index 2f16e960645cf..3a678425e6f26 100644 --- a/llvm/lib/CodeGen/BranchFolding.cpp +++ b/llvm/lib/CodeGen/BranchFolding.cpp @@ -2089,22 +2089,21 @@ bool BranchFolder::HoistCommonCodeInSuccs(MachineBasicBlock *MBB) { // both branches. FIXME: We could probably try harder to preserve some debug // instructions (but at least this isn't producing wrong locations). MachineInstrBuilder MIRBuilder(*MBB->getParent(), Loc); - auto HoistAndKillDbgInstr = - [MBB, Loc](MachineBasicBlock::iterator DI) { - assert(DI->isDebugInstr() && "Expected a debug instruction"); - if (DI->isDebugRef()) { - const TargetInstrInfo *TII = - MBB->getParent()->getSubtarget().getInstrInfo(); - const MCInstrDesc &DBGV = TII->get(TargetOpcode::DBG_VALUE); - DI = BuildMI(*MBB->getParent(), DI->getDebugLoc(), DBGV, false, 0, - DI->getDebugVariable(), DI->getDebugExpression()); - MBB->insert(Loc, &*DI); - return; - } + auto HoistAndKillDbgInstr = [MBB, Loc](MachineBasicBlock::iterator DI) { + assert(DI->isDebugInstr() && "Expected a debug instruction"); + if (DI->isDebugRef()) { + const TargetInstrInfo *TII = + MBB->getParent()->getSubtarget().getInstrInfo(); + const MCInstrDesc &DBGV = TII->get(TargetOpcode::DBG_VALUE); + DI = BuildMI(*MBB->getParent(), DI->getDebugLoc(), DBGV, false, 0, + DI->getDebugVariable(), DI->getDebugExpression()); + MBB->insert(Loc, &*DI); + return; + } - DI->setDebugValueUndef(); - DI->moveBefore(&*Loc); - }; + DI->setDebugValueUndef(); + DI->moveBefore(&*Loc); + }; // TIB and FIB point to the end of the regions to hoist/merge in TBB and // FBB. From bbd7c2625cf7bf68a1d34a11957ec3b0b4c35e4d Mon Sep 17 00:00:00 2001 From: Orlando Cazalet-Hyams Date: Fri, 18 Jul 2025 14:47:45 +0100 Subject: [PATCH 05/10] rm unecessary includes --- llvm/lib/CodeGen/BranchFolding.cpp | 2 -- 1 file changed, 2 deletions(-) diff --git a/llvm/lib/CodeGen/BranchFolding.cpp b/llvm/lib/CodeGen/BranchFolding.cpp index 3a678425e6f26..a2ac9643ce2af 100644 --- a/llvm/lib/CodeGen/BranchFolding.cpp +++ b/llvm/lib/CodeGen/BranchFolding.cpp @@ -48,8 +48,6 @@ #include "llvm/IR/Function.h" #include "llvm/InitializePasses.h" #include "llvm/MC/LaneBitmask.h" -#include "llvm/MC/MCInstrDesc.h" -#include "llvm/MC/MCInstrInfo.h" #include "llvm/MC/MCRegisterInfo.h" #include "llvm/Pass.h" #include "llvm/Support/BlockFrequency.h" From 9b26c488e39833c96ce5b5f5e194a6ea12ab3635 Mon Sep 17 00:00:00 2001 From: Orlando Cazalet-Hyams Date: Fri, 18 Jul 2025 16:51:21 +0100 Subject: [PATCH 06/10] Remove unecessary pseudo probe handling --- llvm/lib/CodeGen/BranchFolding.cpp | 21 +++++++++------------ 1 file changed, 9 insertions(+), 12 deletions(-) diff --git a/llvm/lib/CodeGen/BranchFolding.cpp b/llvm/lib/CodeGen/BranchFolding.cpp index a2ac9643ce2af..1234a9bac713e 100644 --- a/llvm/lib/CodeGen/BranchFolding.cpp +++ b/llvm/lib/CodeGen/BranchFolding.cpp @@ -2109,31 +2109,28 @@ bool BranchFolder::HoistCommonCodeInSuccs(MachineBasicBlock *MBB) { MachineBasicBlock::iterator FI = FBB->begin(); for (MachineBasicBlock::iterator TI : make_early_inc_range(make_range(TBB->begin(), TIB))) { - - // Hoist and kill debug instructions from FBB. Skip over pseudo - // instructions. After this loop FI points to the next non-debug - // instruction to hoist (checked in assert after the TBB debug - // instruction handling code). - while (FI->isDebugOrPseudoInstr()) { + // Hoist and kill debug instructions from FBB. After this loop FI points + // to the next non-debug instruction to hoist (checked in assert after the + // TBB debug instruction handling code). + while (FI->isDebugInstr()) { assert(FI != FE && "Unexpected end of FBB range"); MachineBasicBlock::iterator FINext = std::next(FI); HoistAndKillDbgInstr(FI); FI = FINext; } - // Move pseudo probes without modifying them. - if (TI->isPseudoProbe()) { - TI->moveBefore(&*Loc); - continue; - } // Kill debug instructions before moving. if (TI->isDebugInstr()) { HoistAndKillDbgInstr(TI); continue; } - // Get the next non-meta instruction in FBB. + // If FI is a debug instruction, skip forward to the next non-debug + // instruction. FI = skipDebugInstructionsForward(FI, FE, false); + // Pseudo probes are excluded from the range when identifying foldable + // instructions, so we don't expect to see one now. + assert(!TI->isPseudoProbe() && "Unexpected psuedo probe in range"); // NOTE: The loop above checks CheckKillDead but we can't do that here as // it modifies some kill markers after the check. assert(TI->isIdenticalTo(*FI, MachineInstr::CheckDefs) && From f2a869c19590d9693ec285a6f1823396e42c067c Mon Sep 17 00:00:00 2001 From: Orlando Cazalet-Hyams Date: Fri, 18 Jul 2025 16:57:38 +0100 Subject: [PATCH 07/10] spelllling --- 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 1234a9bac713e..99d5443416fe4 100644 --- a/llvm/lib/CodeGen/BranchFolding.cpp +++ b/llvm/lib/CodeGen/BranchFolding.cpp @@ -2130,7 +2130,7 @@ bool BranchFolder::HoistCommonCodeInSuccs(MachineBasicBlock *MBB) { FI = skipDebugInstructionsForward(FI, FE, false); // Pseudo probes are excluded from the range when identifying foldable // instructions, so we don't expect to see one now. - assert(!TI->isPseudoProbe() && "Unexpected psuedo probe in range"); + assert(!TI->isPseudoProbe() && "Unexpected pseudo probe in range"); // NOTE: The loop above checks CheckKillDead but we can't do that here as // it modifies some kill markers after the check. assert(TI->isIdenticalTo(*FI, MachineInstr::CheckDefs) && From 72b540db3d02858c6b94b7d4c1c958baa7ce10b1 Mon Sep 17 00:00:00 2001 From: Orlando Cazalet-Hyams Date: Mon, 21 Jul 2025 13:48:09 +0100 Subject: [PATCH 08/10] remove assert - FIB may be an end iterator --- 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 99d5443416fe4..c1e9a20328527 100644 --- a/llvm/lib/CodeGen/BranchFolding.cpp +++ b/llvm/lib/CodeGen/BranchFolding.cpp @@ -2143,7 +2143,7 @@ bool BranchFolder::HoistCommonCodeInSuccs(MachineBasicBlock *MBB) { ++FI; } } - assert(FIB->getParent() == FBB && "Unexpectedly moved FIB?"); + FBB->erase(FBB->begin(), FIB); if (UpdateLiveIns) From a446030aa9f5c65b3010d27335c4b5f3850c209e Mon Sep 17 00:00:00 2001 From: Orlando Cazalet-Hyams Date: Tue, 22 Jul 2025 12:09:38 +0100 Subject: [PATCH 09/10] fix crash --- llvm/lib/CodeGen/BranchFolding.cpp | 5 +++++ llvm/test/DebugInfo/X86/branch-folder-dbg.mir | 5 +++++ 2 files changed, 10 insertions(+) diff --git a/llvm/lib/CodeGen/BranchFolding.cpp b/llvm/lib/CodeGen/BranchFolding.cpp index c1e9a20328527..1cb0a239c3d48 100644 --- a/llvm/lib/CodeGen/BranchFolding.cpp +++ b/llvm/lib/CodeGen/BranchFolding.cpp @@ -2098,6 +2098,11 @@ bool BranchFolder::HoistCommonCodeInSuccs(MachineBasicBlock *MBB) { MBB->insert(Loc, &*DI); return; } + // Deleting a DBG_PHI results in an undef at the referenced DBG_INSTR_REF. + if (DI->isDebugPHI()) { + DI->eraseFromParent(); + return; + } DI->setDebugValueUndef(); DI->moveBefore(&*Loc); diff --git a/llvm/test/DebugInfo/X86/branch-folder-dbg.mir b/llvm/test/DebugInfo/X86/branch-folder-dbg.mir index 831b263fdccd4..717bfc28b4065 100644 --- a/llvm/test/DebugInfo/X86/branch-folder-dbg.mir +++ b/llvm/test/DebugInfo/X86/branch-folder-dbg.mir @@ -6,6 +6,8 @@ ## ## The MIR debug instructions have been modified by hand in order to check they ## can be killed. +## +## Check DBG_PHIs are deleted rather than hoisted. # CHECK: bb.0 # CHECK: CALL64pcrel32 @f, csr_64, implicit $rsp, implicit $ssp, implicit-def $rsp, implicit-def $ssp, implicit-def $rax @@ -93,6 +95,8 @@ body: | bb.1.if.then: successors: %bb.3(0x80000000) + + DBG_PHI $esp, 3 DBG_VALUE $esi, $noreg, !11, !DIExpression(), debug-location !13 $edi = MOV32r0 implicit-def dead $eflags, debug-instr-number 1, debug-location !14 DBG_INSTR_REF !11, !DIExpression(DW_OP_LLVM_arg, 0), dbg-instr-ref(1, 0), debug-location !13 @@ -103,6 +107,7 @@ body: | bb.2.if.else: successors: %bb.3(0x80000000) + DBG_PHI $esp, 4 DBG_VALUE $esp, $noreg, !11, !DIExpression(), debug-location !13 $edi = MOV32r0 implicit-def dead $eflags, debug-instr-number 2, debug-location !16 DBG_INSTR_REF !11, !DIExpression(DW_OP_LLVM_arg, 0), dbg-instr-ref(2, 0), debug-location !13 From f1aaf4dececfcc7ff918d57eacf790aefeb0ebea Mon Sep 17 00:00:00 2001 From: Orlando Cazalet-Hyams Date: Fri, 25 Jul 2025 13:26:16 +0100 Subject: [PATCH 10/10] implicit-check-not=DBG_PHI --- llvm/test/DebugInfo/X86/branch-folder-dbg.mir | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/llvm/test/DebugInfo/X86/branch-folder-dbg.mir b/llvm/test/DebugInfo/X86/branch-folder-dbg.mir index 717bfc28b4065..783259866a226 100644 --- a/llvm/test/DebugInfo/X86/branch-folder-dbg.mir +++ b/llvm/test/DebugInfo/X86/branch-folder-dbg.mir @@ -1,4 +1,5 @@ -# RUN: llc %s --start-before=branch-folder --stop-after=branch-folder -o - | FileCheck %s +# RUN: llc %s --start-before=branch-folder --stop-after=branch-folder -o - \ +# RUN: | FileCheck %s --implicit-check-not=DBG_PHI ## Check that common instructions hoisted from `if.then` and `if.else` into ## common pred `entry` get merged debug locations. The debug instructions from @@ -7,7 +8,7 @@ ## The MIR debug instructions have been modified by hand in order to check they ## can be killed. ## -## Check DBG_PHIs are deleted rather than hoisted. +## Check DBG_PHIs are deleted rather than hoisted (implicit-check-not). # CHECK: bb.0 # CHECK: CALL64pcrel32 @f, csr_64, implicit $rsp, implicit $ssp, implicit-def $rsp, implicit-def $ssp, implicit-def $rax