Skip to content
Merged
55 changes: 55 additions & 0 deletions llvm/lib/CodeGen/MachinePipeliner.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -485,6 +485,56 @@ void MachinePipeliner::setPragmaPipelineOptions(MachineLoop &L) {
}
}

bool hasPHICycle(const MachineBasicBlock *LoopHeader,
const MachineRegisterInfo &MRI) {
DenseMap<unsigned, SmallVector<unsigned, 2>> PhiDeps;
SmallSet<unsigned, 8> PhiRegs;

// Collect PHI nodes and their dependencies
for (const MachineInstr &MI : *LoopHeader) {
if (!MI.isPHI())
continue;

unsigned DefReg = MI.getOperand(0).getReg();
PhiRegs.insert(DefReg);

for (unsigned i = 1; i < MI.getNumOperands(); i += 2) {
unsigned SrcReg = MI.getOperand(i).getReg();
PhiDeps[DefReg].push_back(SrcReg);
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
PhiRegs.insert(DefReg);
for (unsigned i = 1; i < MI.getNumOperands(); i += 2) {
unsigned SrcReg = MI.getOperand(i).getReg();
PhiDeps[DefReg].push_back(SrcReg);
}
auto Ite = PhiDeps.try_emplace(DefReg).first;
for (unsigned i = 1; i < MI.getNumOperands(); i += 2)
Ite->second.push_back(MI.getOperand(i).getReg());

To avoid repeated hash lookup.

}

// DFS to detect cycles
SmallSet<unsigned, 8> Visited, RecStack;

std::function<bool(unsigned)> DFS = [&](unsigned Reg) -> bool {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd prefer to define a function like static bool foo(unsigned Reg, ...) rather than using std::function for recursive function.

if (!PhiRegs.count(Reg))
return false;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider calling PhiDeps.find(Reg) instead and remove PhiRegs.

if (RecStack.count(Reg))
return true;
if (Visited.count(Reg))
return false;

Visited.insert(Reg);
RecStack.insert(Reg);

for (unsigned Dep : PhiDeps[Reg]) {
if (DFS(Dep))
return true;
}

RecStack.erase(Reg);
return false;
};

for (unsigned Reg : PhiRegs) {
if (DFS(Reg))
return true;
}

return false;
}

/// Return true if the loop can be software pipelined. The algorithm is
/// restricted to loops with a single basic block. Make sure that the
/// branch in the loop can be analyzed.
Expand All @@ -499,6 +549,11 @@ bool MachinePipeliner::canPipelineLoop(MachineLoop &L) {
return false;
}

if (hasPHICycle(L.getHeader(), MF->getRegInfo())) {
LLVM_DEBUG(dbgs() << "Cannot pipeline loop due to PHI cycle\n");
return false;
}

if (disabledByPragma) {
ORE->emit([&]() {
return MachineOptimizationRemarkAnalysis(DEBUG_TYPE, "canPipelineLoop",
Expand Down
22 changes: 22 additions & 0 deletions llvm/test/CodeGen/Hexagon/swp-phi-cycle.ll
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
; RUN: llc -mtriple=hexagon-unknown-linux-gnu -enable-pipeliner -debug-only=pipeliner < %s 2>&1 | FileCheck %s

; CHECK: Cannot pipeline loop due to PHI cycle

define void @phi_cycle_loop(i32 %a, i32 %b) {
entry:
br label %loop

loop:
%1 = phi i32 [ %a, %entry ], [ %3, %loop ]
%2 = phi i32 [ %a, %entry ], [ %1, %loop ]
%3 = phi i32 [ %b, %entry ], [ %2, %loop ]

; Prevent PHI elimination by using all values
%add1 = add i32 %1, %2
%add2 = add i32 %add1, %3
%cmp = icmp slt i32 %add2, 100
br i1 %cmp, label %loop, label %exit

exit:
ret void
}