From 06c013f526cb5c7953a25ff76ad53eee04c64ae4 Mon Sep 17 00:00:00 2001 From: quic-asaravan Date: Fri, 7 Nov 2025 20:58:39 -0800 Subject: [PATCH 1/8] [MachinePipeliner] Detect a cycle in PHI dependencies early on 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 ] --- llvm/lib/CodeGen/MachinePipeliner.cpp | 54 ++++++++++++++++++++++ llvm/test/CodeGen/Hexagon/swp-phi-cycle.ll | 22 +++++++++ 2 files changed, 76 insertions(+) create mode 100644 llvm/test/CodeGen/Hexagon/swp-phi-cycle.ll diff --git a/llvm/lib/CodeGen/MachinePipeliner.cpp b/llvm/lib/CodeGen/MachinePipeliner.cpp index a717d9e4a618d..b327ba8900f03 100644 --- a/llvm/lib/CodeGen/MachinePipeliner.cpp +++ b/llvm/lib/CodeGen/MachinePipeliner.cpp @@ -485,6 +485,55 @@ void MachinePipeliner::setPragmaPipelineOptions(MachineLoop &L) { } } +bool hasPHICycle(const MachineBasicBlock *LoopHeader, const MachineRegisterInfo &MRI) { + DenseMap> PhiDeps; + SmallSet 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); + } + } + + // DFS to detect cycles + SmallSet Visited, RecStack; + + std::function DFS = [&](unsigned Reg) -> bool { + if (!PhiRegs.count(Reg)) + return false; + 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. @@ -499,6 +548,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", diff --git a/llvm/test/CodeGen/Hexagon/swp-phi-cycle.ll b/llvm/test/CodeGen/Hexagon/swp-phi-cycle.ll new file mode 100644 index 0000000000000..a92d113f01c88 --- /dev/null +++ b/llvm/test/CodeGen/Hexagon/swp-phi-cycle.ll @@ -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 +} From 68a3d5f6a3d58ccb592794360e865e8648fb1352 Mon Sep 17 00:00:00 2001 From: quic-asaravan Date: Fri, 7 Nov 2025 20:58:39 -0800 Subject: [PATCH 2/8] [MachinePipeliner] Detect a cycle in PHI dependencies early on 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 ] --- llvm/lib/CodeGen/MachinePipeliner.cpp | 55 ++++++++++++++++++++++ llvm/test/CodeGen/Hexagon/swp-phi-cycle.ll | 22 +++++++++ 2 files changed, 77 insertions(+) create mode 100644 llvm/test/CodeGen/Hexagon/swp-phi-cycle.ll diff --git a/llvm/lib/CodeGen/MachinePipeliner.cpp b/llvm/lib/CodeGen/MachinePipeliner.cpp index a717d9e4a618d..ad6fd0d204d7a 100644 --- a/llvm/lib/CodeGen/MachinePipeliner.cpp +++ b/llvm/lib/CodeGen/MachinePipeliner.cpp @@ -485,6 +485,56 @@ void MachinePipeliner::setPragmaPipelineOptions(MachineLoop &L) { } } +bool hasPHICycle(const MachineBasicBlock *LoopHeader, + const MachineRegisterInfo &MRI) { + DenseMap> PhiDeps; + SmallSet 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); + } + } + + // DFS to detect cycles + SmallSet Visited, RecStack; + + std::function DFS = [&](unsigned Reg) -> bool { + if (!PhiRegs.count(Reg)) + return false; + 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. @@ -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", diff --git a/llvm/test/CodeGen/Hexagon/swp-phi-cycle.ll b/llvm/test/CodeGen/Hexagon/swp-phi-cycle.ll new file mode 100644 index 0000000000000..a92d113f01c88 --- /dev/null +++ b/llvm/test/CodeGen/Hexagon/swp-phi-cycle.ll @@ -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 +} From 1060047034787c8815861f4fe79784e75a2a7e3b Mon Sep 17 00:00:00 2001 From: Abinaya Saravanan Date: Tue, 11 Nov 2025 15:50:16 +0530 Subject: [PATCH 3/8] Update llvm/lib/CodeGen/MachinePipeliner.cpp Co-authored-by: Ryotaro Kasuga --- llvm/lib/CodeGen/MachinePipeliner.cpp | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/llvm/lib/CodeGen/MachinePipeliner.cpp b/llvm/lib/CodeGen/MachinePipeliner.cpp index ad6fd0d204d7a..0837c73a21147 100644 --- a/llvm/lib/CodeGen/MachinePipeliner.cpp +++ b/llvm/lib/CodeGen/MachinePipeliner.cpp @@ -491,9 +491,7 @@ bool hasPHICycle(const MachineBasicBlock *LoopHeader, SmallSet PhiRegs; // Collect PHI nodes and their dependencies - for (const MachineInstr &MI : *LoopHeader) { - if (!MI.isPHI()) - continue; + for (const MachineInstr &MI : LoopHeader->phis()) { unsigned DefReg = MI.getOperand(0).getReg(); PhiRegs.insert(DefReg); From 4354dd40ca26a7591fe122156a48307ee2cdb541 Mon Sep 17 00:00:00 2001 From: Abinaya Saravanan Date: Tue, 11 Nov 2025 15:50:42 +0530 Subject: [PATCH 4/8] Update llvm/test/CodeGen/Hexagon/swp-phi-cycle.ll Co-authored-by: Ryotaro Kasuga --- llvm/test/CodeGen/Hexagon/swp-phi-cycle.ll | 1 + 1 file changed, 1 insertion(+) diff --git a/llvm/test/CodeGen/Hexagon/swp-phi-cycle.ll b/llvm/test/CodeGen/Hexagon/swp-phi-cycle.ll index a92d113f01c88..1b4fc464fa092 100644 --- a/llvm/test/CodeGen/Hexagon/swp-phi-cycle.ll +++ b/llvm/test/CodeGen/Hexagon/swp-phi-cycle.ll @@ -1,4 +1,5 @@ ; RUN: llc -mtriple=hexagon-unknown-linux-gnu -enable-pipeliner -debug-only=pipeliner < %s 2>&1 | FileCheck %s +; REQUIRES: asserts ; CHECK: Cannot pipeline loop due to PHI cycle From 10698b7b8a75a6553cbbc3f528a38146d7652746 Mon Sep 17 00:00:00 2001 From: Abinaya Saravanan Date: Tue, 11 Nov 2025 15:51:07 +0530 Subject: [PATCH 5/8] Update llvm/lib/CodeGen/MachinePipeliner.cpp Co-authored-by: Ryotaro Kasuga --- llvm/lib/CodeGen/MachinePipeliner.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/llvm/lib/CodeGen/MachinePipeliner.cpp b/llvm/lib/CodeGen/MachinePipeliner.cpp index 0837c73a21147..a144675d5d9fb 100644 --- a/llvm/lib/CodeGen/MachinePipeliner.cpp +++ b/llvm/lib/CodeGen/MachinePipeliner.cpp @@ -485,7 +485,7 @@ void MachinePipeliner::setPragmaPipelineOptions(MachineLoop &L) { } } -bool hasPHICycle(const MachineBasicBlock *LoopHeader, +static bool hasPHICycle(const MachineBasicBlock *LoopHeader, const MachineRegisterInfo &MRI) { DenseMap> PhiDeps; SmallSet PhiRegs; From f7084ce1636f0ec198fd159d1b9e94b8b53abde1 Mon Sep 17 00:00:00 2001 From: quic-asaravan Date: Wed, 12 Nov 2025 07:53:56 -0800 Subject: [PATCH 6/8] Taking review suggestions --- llvm/lib/CodeGen/MachinePipeliner.cpp | 75 +++++++++++++++------------ 1 file changed, 43 insertions(+), 32 deletions(-) diff --git a/llvm/lib/CodeGen/MachinePipeliner.cpp b/llvm/lib/CodeGen/MachinePipeliner.cpp index a144675d5d9fb..3acd71a530e1f 100644 --- a/llvm/lib/CodeGen/MachinePipeliner.cpp +++ b/llvm/lib/CodeGen/MachinePipeliner.cpp @@ -485,48 +485,59 @@ void MachinePipeliner::setPragmaPipelineOptions(MachineLoop &L) { } } -static bool hasPHICycle(const MachineBasicBlock *LoopHeader, - const MachineRegisterInfo &MRI) { - DenseMap> PhiDeps; - SmallSet PhiRegs; - // Collect PHI nodes and their dependencies - for (const MachineInstr &MI : LoopHeader->phis()) { +/// Depth-first search to detect cycles among PHI dependencies. +/// Returns true if a cycle is detected within the PHI-only subgraph. +static bool hasPHICycleDFS( + unsigned Reg, + const DenseMap> &PhiDeps, + SmallSet &Visited, + SmallSet &RecStack) { + + // If Reg is not a PHI-def it cannot contribute to a PHI cycle. + auto It = PhiDeps.find(Reg); + if (It == PhiDeps.end()) + return false; - unsigned DefReg = MI.getOperand(0).getReg(); - PhiRegs.insert(DefReg); + if (RecStack.count(Reg)) + return true; // backedge. + if (Visited.count(Reg)) + return false; - for (unsigned i = 1; i < MI.getNumOperands(); i += 2) { - unsigned SrcReg = MI.getOperand(i).getReg(); - PhiDeps[DefReg].push_back(SrcReg); - } + Visited.insert(Reg); + RecStack.insert(Reg); + + for (unsigned Dep : It->second) { + if (hasPHICycleDFS(Dep, PhiDeps, Visited, RecStack)) + return true; } - // DFS to detect cycles - SmallSet Visited, RecStack; + RecStack.erase(Reg); + return false; +} - std::function DFS = [&](unsigned Reg) -> bool { - if (!PhiRegs.count(Reg)) - return false; - if (RecStack.count(Reg)) - return true; - if (Visited.count(Reg)) - return false; - Visited.insert(Reg); - RecStack.insert(Reg); +static bool hasPHICycle(const MachineBasicBlock *LoopHeader, + const MachineRegisterInfo &MRI) { + DenseMap> PhiDeps; - for (unsigned Dep : PhiDeps[Reg]) { - if (DFS(Dep)) - return true; - } + // Collect PHI nodes and their dependencies. + for (const MachineInstr &MI : LoopHeader->phis()) { + unsigned DefReg = MI.getOperand(0).getReg(); + auto Ins = PhiDeps.try_emplace(DefReg).first; - RecStack.erase(Reg); - return false; - }; + // PHI operands are (Reg, MBB) pairs starting at index 1. + for (unsigned i = 1; i < MI.getNumOperands(); i += 2) + Ins->second.push_back(MI.getOperand(i).getReg()); + } + + // DFS to detect cycles among PHI nodes. + SmallSet Visited, RecStack; - for (unsigned Reg : PhiRegs) { - if (DFS(Reg)) + // Start DFS from each PHI-def. + for (const auto &KV : PhiDeps) { + unsigned Reg = KV.first; + if (hasPHICycleDFS(Reg, PhiDeps, Visited, RecStack)) return true; } From 0d0d3e1d0d339451cda23592377afb8dfff468c6 Mon Sep 17 00:00:00 2001 From: quic-asaravan Date: Wed, 12 Nov 2025 07:55:43 -0800 Subject: [PATCH 7/8] Running clang format --- llvm/lib/CodeGen/MachinePipeliner.cpp | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/llvm/lib/CodeGen/MachinePipeliner.cpp b/llvm/lib/CodeGen/MachinePipeliner.cpp index 3acd71a530e1f..dc88e6c1f1bc0 100644 --- a/llvm/lib/CodeGen/MachinePipeliner.cpp +++ b/llvm/lib/CodeGen/MachinePipeliner.cpp @@ -485,14 +485,11 @@ void MachinePipeliner::setPragmaPipelineOptions(MachineLoop &L) { } } - /// Depth-first search to detect cycles among PHI dependencies. /// Returns true if a cycle is detected within the PHI-only subgraph. static bool hasPHICycleDFS( - unsigned Reg, - const DenseMap> &PhiDeps, - SmallSet &Visited, - SmallSet &RecStack) { + unsigned Reg, const DenseMap> &PhiDeps, + SmallSet &Visited, SmallSet &RecStack) { // If Reg is not a PHI-def it cannot contribute to a PHI cycle. auto It = PhiDeps.find(Reg); @@ -516,7 +513,6 @@ static bool hasPHICycleDFS( return false; } - static bool hasPHICycle(const MachineBasicBlock *LoopHeader, const MachineRegisterInfo &MRI) { DenseMap> PhiDeps; From 9bac48e980e396a395949b9757803a9812c4a3d8 Mon Sep 17 00:00:00 2001 From: Abinaya Saravanan Date: Mon, 17 Nov 2025 13:12:09 +0530 Subject: [PATCH 8/8] Update llvm/lib/CodeGen/MachinePipeliner.cpp Co-authored-by: Ryotaro Kasuga --- llvm/lib/CodeGen/MachinePipeliner.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/llvm/lib/CodeGen/MachinePipeliner.cpp b/llvm/lib/CodeGen/MachinePipeliner.cpp index dc88e6c1f1bc0..dfb2f7d8e29df 100644 --- a/llvm/lib/CodeGen/MachinePipeliner.cpp +++ b/llvm/lib/CodeGen/MachinePipeliner.cpp @@ -523,8 +523,8 @@ static bool hasPHICycle(const MachineBasicBlock *LoopHeader, auto Ins = PhiDeps.try_emplace(DefReg).first; // PHI operands are (Reg, MBB) pairs starting at index 1. - for (unsigned i = 1; i < MI.getNumOperands(); i += 2) - Ins->second.push_back(MI.getOperand(i).getReg()); + for (unsigned I = 1; I < MI.getNumOperands(); I += 2) + Ins->second.push_back(MI.getOperand(I).getReg()); } // DFS to detect cycles among PHI nodes.