Skip to content

Commit fc2a8d9

Browse files
OCHyamsJaddyen
OCHyams
authored and
Jaddyen
committed
Revert "[BranchFolding] Kill common hoisted debug instructions" (llvm#150632)
Reverts llvm#149999 https://lab.llvm.org/buildbot/#/builders/139/builds/17622
1 parent fa2637f commit fc2a8d9

File tree

2 files changed

+20
-70
lines changed

2 files changed

+20
-70
lines changed

‎llvm/lib/CodeGen/BranchFolding.cpp

Lines changed: 6 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -2083,59 +2083,22 @@ bool BranchFolder::HoistCommonCodeInSuccs(MachineBasicBlock *MBB) {
20832083
if (TBB == FBB) {
20842084
MBB->splice(Loc, TBB, TBB->begin(), TIB);
20852085
} else {
2086-
// Merge the debug locations, and hoist and kill the debug instructions from
2087-
// both branches. FIXME: We could probably try harder to preserve some debug
2088-
// instructions (but at least this isn't producing wrong locations).
2089-
MachineInstrBuilder MIRBuilder(*MBB->getParent(), Loc);
2090-
auto HoistAndKillDbgInstr = [MBB, Loc](MachineBasicBlock::iterator DI) {
2091-
assert(DI->isDebugInstr() && "Expected a debug instruction");
2092-
if (DI->isDebugRef()) {
2093-
const TargetInstrInfo *TII =
2094-
MBB->getParent()->getSubtarget().getInstrInfo();
2095-
const MCInstrDesc &DBGV = TII->get(TargetOpcode::DBG_VALUE);
2096-
DI = BuildMI(*MBB->getParent(), DI->getDebugLoc(), DBGV, false, 0,
2097-
DI->getDebugVariable(), DI->getDebugExpression());
2098-
MBB->insert(Loc, &*DI);
2099-
return;
2100-
}
2101-
// Deleting a DBG_PHI results in an undef at the referenced DBG_INSTR_REF.
2102-
if (DI->isDebugPHI()) {
2103-
DI->eraseFromParent();
2104-
return;
2105-
}
2106-
2107-
DI->setDebugValueUndef();
2108-
DI->moveBefore(&*Loc);
2109-
};
2110-
21112086
// TIB and FIB point to the end of the regions to hoist/merge in TBB and
21122087
// FBB.
21132088
MachineBasicBlock::iterator FE = FIB;
21142089
MachineBasicBlock::iterator FI = FBB->begin();
21152090
for (MachineBasicBlock::iterator TI :
21162091
make_early_inc_range(make_range(TBB->begin(), TIB))) {
2117-
// Hoist and kill debug instructions from FBB. After this loop FI points
2118-
// to the next non-debug instruction to hoist (checked in assert after the
2119-
// TBB debug instruction handling code).
2120-
while (FI->isDebugInstr()) {
2121-
assert(FI != FE && "Unexpected end of FBB range");
2122-
MachineBasicBlock::iterator FINext = std::next(FI);
2123-
HoistAndKillDbgInstr(FI);
2124-
FI = FINext;
2125-
}
2126-
2127-
// Kill debug instructions before moving.
2128-
if (TI->isDebugInstr()) {
2129-
HoistAndKillDbgInstr(TI);
2092+
// Move debug instructions and pseudo probes without modifying them.
2093+
// FIXME: This is the wrong thing to do for debug locations, which
2094+
// should at least be killed (and hoisted from BOTH blocks).
2095+
if (TI->isDebugOrPseudoInstr()) {
2096+
TI->moveBefore(&*Loc);
21302097
continue;
21312098
}
21322099

2133-
// If FI is a debug instruction, skip forward to the next non-debug
2134-
// instruction.
2100+
// Get the next non-meta instruction in FBB.
21352101
FI = skipDebugInstructionsForward(FI, FE, false);
2136-
// Pseudo probes are excluded from the range when identifying foldable
2137-
// instructions, so we don't expect to see one now.
2138-
assert(!TI->isPseudoProbe() && "Unexpected pseudo probe in range");
21392102
// NOTE: The loop above checks CheckKillDead but we can't do that here as
21402103
// it modifies some kill markers after the check.
21412104
assert(TI->isIdenticalTo(*FI, MachineInstr::CheckDefs) &&
@@ -2148,7 +2111,6 @@ bool BranchFolder::HoistCommonCodeInSuccs(MachineBasicBlock *MBB) {
21482111
++FI;
21492112
}
21502113
}
2151-
21522114
FBB->erase(FBB->begin(), FIB);
21532115

21542116
if (UpdateLiveIns)

‎llvm/test/DebugInfo/X86/branch-folder-dbg.mir

Lines changed: 14 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -1,24 +1,16 @@
1-
# RUN: llc %s --start-before=branch-folder --stop-after=branch-folder -o - \
2-
# RUN: | FileCheck %s --implicit-check-not=DBG_PHI
1+
# RUN: llc %s --start-before=branch-folder --stop-after=branch-folder -o - | FileCheck %s
32

43
## Check that common instructions hoisted from `if.then` and `if.else` into
5-
## common pred `entry` get merged debug locations. The debug instructions from
6-
## both branches should get hoisted and killed.
7-
##
8-
## The MIR debug instructions have been modified by hand in order to check they
9-
## can be killed.
10-
##
11-
## Check DBG_PHIs are deleted rather than hoisted (implicit-check-not).
4+
## common pred `entry` get merged debug locations.
5+
6+
## FIXME: The debug instructions handling here is wrong.
127

138
# CHECK: bb.0
149
# CHECK: CALL64pcrel32 @f, csr_64, implicit $rsp, implicit $ssp, implicit-def $rsp, implicit-def $ssp, implicit-def $rax
15-
## --- Start splice from bb.2.if.else (and debug instructions from bb.1.if.then) ---
16-
# CHECK-NEXT: DBG_VALUE $noreg, $noreg, ![[#]], !DIExpression(), debug-location ![[#]]
17-
# CHECK-NEXT: DBG_VALUE $noreg, $noreg, ![[#]], !DIExpression(), debug-location ![[#]]
18-
# CHECK-NEXT: $edi = MOV32r0 implicit-def dead $eflags, debug-instr-number 2, debug-location !DILocation(line: 0, scope: ![[#]])
19-
# CHECK-NEXT: DBG_VALUE $noreg, $noreg, ![[#]], !DIExpression(DW_OP_LLVM_arg, 0), debug-location ![[#]]
20-
# CHECK-NEXT: DBG_VALUE $noreg, $noreg, ![[#]], !DIExpression(DW_OP_LLVM_arg, 0), debug-location ![[#]]
21-
## --- End splice ------------------------------------------------------------------
10+
## --- Start splice from bb.2.if.else ---
11+
# CHECK-NEXT: DBG_VALUE 2, $noreg, ![[#]], !DIExpression(), debug-location ![[#]]
12+
# CHECK-NEXT: $edi = MOV32r0 implicit-def dead $eflags, debug-location !DILocation(line: 0, scope: ![[#]])
13+
## --- End splice --------------
2214
# CHECK-NEXT: TEST64rr killed renamable $rax, renamable $rax, implicit-def $eflags
2315
# CHECK-NEXT: JCC_1 %bb.2, 9, implicit killed $eflags
2416
# CHECK: bb.1
@@ -81,8 +73,6 @@
8173
...
8274
---
8375
name: g
84-
tracksRegLiveness: true
85-
isSSA: false
8676
body: |
8777
bb.0 (%ir-block.0):
8878
successors: %bb.1(0x40000000), %bb.2(0x40000000)
@@ -97,23 +87,21 @@ body: |
9787
bb.1.if.then:
9888
successors: %bb.3(0x80000000)
9989
100-
DBG_PHI $esp, 3
101-
DBG_VALUE $esi, $noreg, !11, !DIExpression(), debug-location !13
102-
$edi = MOV32r0 implicit-def dead $eflags, debug-instr-number 1, debug-location !14
103-
DBG_INSTR_REF !11, !DIExpression(DW_OP_LLVM_arg, 0), dbg-instr-ref(1, 0), debug-location !13
90+
DBG_VALUE 0, $noreg, !11, !DIExpression(), debug-location !13
91+
$edi = MOV32r0 implicit-def dead $eflags, debug-location !14
10492
$esi = MOV32r0 implicit-def dead $eflags, debug-location !14
10593
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
94+
DBG_VALUE 1, $noreg, !11, !DIExpression(), debug-location !13
10695
JMP_1 %bb.3, debug-location !15
10796
10897
bb.2.if.else:
10998
successors: %bb.3(0x80000000)
11099
111-
DBG_PHI $esp, 4
112-
DBG_VALUE $esp, $noreg, !11, !DIExpression(), debug-location !13
113-
$edi = MOV32r0 implicit-def dead $eflags, debug-instr-number 2, debug-location !16
114-
DBG_INSTR_REF !11, !DIExpression(DW_OP_LLVM_arg, 0), dbg-instr-ref(2, 0), debug-location !13
100+
DBG_VALUE 2, $noreg, !11, !DIExpression(), debug-location !13
101+
$edi = MOV32r0 implicit-def dead $eflags, debug-location !16
115102
$esi = MOV32ri 1, debug-location !16
116103
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
104+
DBG_VALUE 3, $noreg, !11, !DIExpression(), debug-location !13
117105
118106
bb.3.if.end:
119107
$eax = MOV32ri 2

0 commit comments

Comments
 (0)