Skip to content

Commit c946418

Browse files
[MachinePipeliner] Detect a cycle in PHI dependencies early on (#167095)
- This patch detects cycles by phis and bails out if one is found. - It prevents to violate DAG restrictions. Abort pipelining in the below case %1 = phi i32 [ %a, %entry ], [ %3, %loop ] %2 = phi i32 [ %a, %entry ], [ %1, %loop ] %3 = phi i32 [ %b, %entry ], [ %2, %loop ] --------- Co-authored-by: Ryotaro Kasuga <[email protected]>
1 parent 15958f2 commit c946418

File tree

2 files changed

+83
-0
lines changed

2 files changed

+83
-0
lines changed

llvm/lib/CodeGen/MachinePipeliner.cpp

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -485,6 +485,61 @@ void MachinePipeliner::setPragmaPipelineOptions(MachineLoop &L) {
485485
}
486486
}
487487

488+
/// Depth-first search to detect cycles among PHI dependencies.
489+
/// Returns true if a cycle is detected within the PHI-only subgraph.
490+
static bool hasPHICycleDFS(
491+
unsigned Reg, const DenseMap<unsigned, SmallVector<unsigned, 2>> &PhiDeps,
492+
SmallSet<unsigned, 8> &Visited, SmallSet<unsigned, 8> &RecStack) {
493+
494+
// If Reg is not a PHI-def it cannot contribute to a PHI cycle.
495+
auto It = PhiDeps.find(Reg);
496+
if (It == PhiDeps.end())
497+
return false;
498+
499+
if (RecStack.count(Reg))
500+
return true; // backedge.
501+
if (Visited.count(Reg))
502+
return false;
503+
504+
Visited.insert(Reg);
505+
RecStack.insert(Reg);
506+
507+
for (unsigned Dep : It->second) {
508+
if (hasPHICycleDFS(Dep, PhiDeps, Visited, RecStack))
509+
return true;
510+
}
511+
512+
RecStack.erase(Reg);
513+
return false;
514+
}
515+
516+
static bool hasPHICycle(const MachineBasicBlock *LoopHeader,
517+
const MachineRegisterInfo &MRI) {
518+
DenseMap<unsigned, SmallVector<unsigned, 2>> PhiDeps;
519+
520+
// Collect PHI nodes and their dependencies.
521+
for (const MachineInstr &MI : LoopHeader->phis()) {
522+
unsigned DefReg = MI.getOperand(0).getReg();
523+
auto Ins = PhiDeps.try_emplace(DefReg).first;
524+
525+
// PHI operands are (Reg, MBB) pairs starting at index 1.
526+
for (unsigned I = 1; I < MI.getNumOperands(); I += 2)
527+
Ins->second.push_back(MI.getOperand(I).getReg());
528+
}
529+
530+
// DFS to detect cycles among PHI nodes.
531+
SmallSet<unsigned, 8> Visited, RecStack;
532+
533+
// Start DFS from each PHI-def.
534+
for (const auto &KV : PhiDeps) {
535+
unsigned Reg = KV.first;
536+
if (hasPHICycleDFS(Reg, PhiDeps, Visited, RecStack))
537+
return true;
538+
}
539+
540+
return false;
541+
}
542+
488543
/// Return true if the loop can be software pipelined. The algorithm is
489544
/// restricted to loops with a single basic block. Make sure that the
490545
/// branch in the loop can be analyzed.
@@ -499,6 +554,11 @@ bool MachinePipeliner::canPipelineLoop(MachineLoop &L) {
499554
return false;
500555
}
501556

557+
if (hasPHICycle(L.getHeader(), MF->getRegInfo())) {
558+
LLVM_DEBUG(dbgs() << "Cannot pipeline loop due to PHI cycle\n");
559+
return false;
560+
}
561+
502562
if (disabledByPragma) {
503563
ORE->emit([&]() {
504564
return MachineOptimizationRemarkAnalysis(DEBUG_TYPE, "canPipelineLoop",
Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
; RUN: llc -mtriple=hexagon-unknown-linux-gnu -enable-pipeliner -debug-only=pipeliner < %s 2>&1 | FileCheck %s
2+
; REQUIRES: asserts
3+
4+
; CHECK: Cannot pipeline loop due to PHI cycle
5+
6+
define void @phi_cycle_loop(i32 %a, i32 %b) {
7+
entry:
8+
br label %loop
9+
10+
loop:
11+
%1 = phi i32 [ %a, %entry ], [ %3, %loop ]
12+
%2 = phi i32 [ %a, %entry ], [ %1, %loop ]
13+
%3 = phi i32 [ %b, %entry ], [ %2, %loop ]
14+
15+
; Prevent PHI elimination by using all values
16+
%add1 = add i32 %1, %2
17+
%add2 = add i32 %add1, %3
18+
%cmp = icmp slt i32 %add2, 100
19+
br i1 %cmp, label %loop, label %exit
20+
21+
exit:
22+
ret void
23+
}

0 commit comments

Comments
 (0)