Skip to content

Commit 40e5a8c

Browse files
committed
[AIE] ReservedRegsLICM: fix exit-sink to scan the exiting block, not the latch
processForExitSink was scanning L.getLoopLatch() for sink candidates. When the latch and the exiting block differ, instructions in the latch are not on the exit path: sinking them to the exit block inserts a def that was never executed on that path, corrupting the register's value at the exit. Fix this by scanning L.getExitingBlock() instead. The guard that checks whether a register is live at the entry of the scanned block is also updated to use the exiting block (renamed LatchEntryLive -> ExitingBlockEntryLive). Add a test case (latch_vs_exiting_no_sink) that reproduces the bug: a loop where the latch (bb.3) defines $srssign0 = MOVX 0 but the exiting block (bb.2) exits directly to bb.4 without going through bb.3. The MOVX must remain in bb.3.
1 parent b939bc3 commit 40e5a8c

2 files changed

Lines changed: 94 additions & 15 deletions

File tree

llvm/lib/Target/AIE/ReservedRegsLICM.cpp

Lines changed: 23 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -341,36 +341,44 @@ static void moveInstruction(const CandidateInfo &Cand,
341341

342342
void ReservedRegsLICM::processForExitSink(MachineLoop &L,
343343
const BitVector &ReservedLiveins) {
344-
// Pre-compute what's live at the entry of the latch block by walking it
344+
// Sink candidates must come from the exiting block (the block that has the
345+
// edge to the exit block), not the latch. When the latch and the exiting
346+
// block differ, instructions in the latch are not on the exit path: sinking
347+
// them to the exit block would insert a def that was never executed on that
348+
// path, corrupting the register's exit value.
349+
MachineBasicBlock *ExitingBlock = L.getExitingBlock();
350+
assert(ExitingBlock);
351+
352+
// Pre-compute what's live at the entry of the exiting block by walking it
345353
// fully backward. For multi-block loops with conditional paths, a candidate
346354
// register may be defined in some branches but not others, making it live at
347-
// the latch entry on those paths. If the register is live at the latch entry
348-
// (i.e., used before its first def in the latch), sinking its last def to
349-
// the exit block would expose the wrong value on those paths.
350-
assert(L.getLoopLatch());
351-
LivePhysRegs LatchEntryLive(*TRI);
352-
for (const MachineInstr &MI : reverse(*L.getLoopLatch()))
353-
LatchEntryLive.stepBackward(MI);
355+
// the exiting block entry on those paths. If the register is live at the
356+
// exiting block entry (i.e., used before its first def in the exiting block,
357+
// possibly carrying a value from a conditional path that skips the def),
358+
// sinking its last def to the exit block would expose the wrong value.
359+
LivePhysRegs ExitingBlockEntryLive(*TRI);
360+
for (const MachineInstr &MI : reverse(*ExitingBlock))
361+
ExitingBlockEntryLive.stepBackward(MI);
354362

355363
RegDefMap PhysRegChanged(*TRI);
356364
LivePhysRegs LiveRegs(*TRI);
357365
Candidates SinkCandidates;
358366

359-
// Walk the latch block, track defs for each register, and
367+
// Walk the exiting block, track defs for each register, and
360368
// collect potential LICM candidates.
361-
for (MachineInstr &MI : reverse(*L.getLoopLatch())) {
369+
for (MachineInstr &MI : reverse(*ExitingBlock)) {
362370
CandidateInfo *CandInfo = SinkCandidates.getInfo(MI);
363371

364372
// First time we meet a reserved reg definition while iterating upwards.
365373
// If that def is not a loop livein, it isn't used in this block after the
366-
// def, and it isn't live at the latch entry (which would indicate it is
367-
// used before its def in the latch, possibly carrying a value from a
368-
// conditional path that skips the def), then one can move the instruction
369-
// to the exit BB of the loop.
374+
// def, and it isn't live at the exiting block entry (which would indicate
375+
// it is used before its def in the exiting block, possibly carrying a
376+
// value from a conditional path that skips the def), then one can move
377+
// the instruction to the exit BB of the loop.
370378
if (CandInfo && !PhysRegChanged.hasChanged(CandInfo->DefinedReg) &&
371379
!ReservedLiveins.test(CandInfo->DefinedReg) &&
372380
!LiveRegs.contains(CandInfo->DefinedReg) &&
373-
!LatchEntryLive.contains(CandInfo->DefinedReg)) {
381+
!ExitingBlockEntryLive.contains(CandInfo->DefinedReg)) {
374382
assert(!CandInfo->HoistCandidate);
375383
CandInfo->HoistCandidate = &MI;
376384
}

llvm/test/CodeGen/AIE/aie2ps/reserved-reg-licm.mir

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -436,6 +436,77 @@ body: |
436436
PseudoJ_jump_imm %bb.1
437437
...
438438

439+
# The latch (bb.3) defines $srssign0 = MOVX 0 but is NOT the exiting block.
440+
# The exiting block (bb.2) exits to bb.4 without going through bb.3, so the
441+
# MOVX is not on the exit path. processForExitSink must scan the exiting block
442+
# (bb.2), not the latch (bb.3), to avoid sinking the MOVX to bb.4.
443+
---
444+
name: latch_vs_exiting_no_sink
445+
tracksRegLiveness: true
446+
body: |
447+
; CHECK-LABEL: name: latch_vs_exiting_no_sink
448+
; CHECK: bb.0:
449+
; CHECK-NEXT: successors: %bb.1(0x80000000)
450+
; CHECK-NEXT: liveins: $p0, $r0, $r1, $dm0, $s0
451+
; CHECK-NEXT: {{ $}}
452+
; CHECK-NEXT: [[COPY:%[0-9]+]]:er = COPY $r0
453+
; CHECK-NEXT: [[COPY1:%[0-9]+]]:ep = COPY $p0
454+
; CHECK-NEXT: [[COPY2:%[0-9]+]]:er = COPY $r1
455+
; CHECK-NEXT: [[COPY3:%[0-9]+]]:edm = COPY $dm0
456+
; CHECK-NEXT: [[COPY4:%[0-9]+]]:es = COPY $s0
457+
; CHECK-NEXT: PseudoJ_jump_imm %bb.1
458+
; CHECK-NEXT: {{ $}}
459+
; CHECK-NEXT: bb.1:
460+
; CHECK-NEXT: successors: %bb.2(0x80000000)
461+
; CHECK-NEXT: {{ $}}
462+
; CHECK-NEXT: $srssign0 = COPY [[COPY2]]
463+
; CHECK-NEXT: PseudoJ_jump_imm %bb.2
464+
; CHECK-NEXT: {{ $}}
465+
; CHECK-NEXT: bb.2:
466+
; CHECK-NEXT: successors: %bb.3(0x7c000000), %bb.4(0x04000000)
467+
; CHECK-NEXT: {{ $}}
468+
; CHECK-NEXT: [[VSRS_4x_mv_x_srs_dm_srsSign0_:%[0-9]+]]:mxs = VSRS_4x_mv_x_srs_dm_srsSign0 [[COPY3]], [[COPY4]], implicit-def $srsrs_of, implicit $crrnd, implicit $crsrsmode, implicit $crsat, implicit $srssign0
469+
; CHECK-NEXT: PseudoJNZ [[COPY]], %bb.3
470+
; CHECK-NEXT: PseudoJ_jump_imm %bb.4
471+
; CHECK-NEXT: {{ $}}
472+
; CHECK-NEXT: bb.3:
473+
; CHECK-NEXT: successors: %bb.1(0x80000000)
474+
; CHECK-NEXT: {{ $}}
475+
; CHECK-NEXT: $srssign0 = MOVX_mvx_cr_imm 0
476+
; CHECK-NEXT: PseudoJ_jump_imm %bb.1
477+
; CHECK-NEXT: {{ $}}
478+
; CHECK-NEXT: bb.4:
479+
; CHECK-NEXT: PseudoRET implicit $lr
480+
bb.0:
481+
successors: %bb.1(0x80000000)
482+
liveins: $p0, $r0, $r1, $dm0, $s0
483+
%0:er = COPY $r0
484+
%1:ep = COPY $p0
485+
%2:er = COPY $r1
486+
%3:edm = COPY $dm0
487+
%4:es = COPY $s0
488+
PseudoJ_jump_imm %bb.1
489+
490+
bb.1:
491+
successors: %bb.2(0x80000000)
492+
$srssign0 = COPY %2
493+
PseudoJ_jump_imm %bb.2
494+
495+
bb.2:
496+
successors: %bb.3(0x7c000000), %bb.4(0x04000000)
497+
%5:mxs = VSRS_4x_mv_x_srs_dm_srsSign0 %3, %4, implicit-def $srsrs_of, implicit $crrnd, implicit $crsrsmode, implicit $crsat, implicit $srssign0
498+
PseudoJNZ %0, %bb.3
499+
PseudoJ_jump_imm %bb.4
500+
501+
bb.3:
502+
successors: %bb.1(0x80000000)
503+
$srssign0 = MOVX_mvx_cr_imm 0
504+
PseudoJ_jump_imm %bb.1
505+
506+
bb.4:
507+
PseudoRET implicit $lr
508+
...
509+
439510
# $srssign0 is used in one conditional branch (bb.2) but defined in the other
440511
# (bb.3). The latch (bb.4) defines $srssign0 = MOVX 0 without using it first,
441512
# so LatchEntryLive does NOT flag it. However, $srssign0 IS a true loop livein

0 commit comments

Comments
 (0)