[llvm] [MachinePipeliner] Detect a cycle in PHI dependencies early on (PR #167095)
Abinaya Saravanan via llvm-commits
llvm-commits at lists.llvm.org
Sun Nov 16 23:42:34 PST 2025
https://github.com/quic-asaravan updated https://github.com/llvm/llvm-project/pull/167095
>From 06c013f526cb5c7953a25ff76ad53eee04c64ae4 Mon Sep 17 00:00:00 2001
From: quic-asaravan <quic_asaravan at quicinc.com>
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<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);
+ }
+ }
+
+ // DFS to detect cycles
+ SmallSet<unsigned, 8> Visited, RecStack;
+
+ std::function<bool(unsigned)> 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 <quic_asaravan at quicinc.com>
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<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);
+ }
+ }
+
+ // DFS to detect cycles
+ SmallSet<unsigned, 8> Visited, RecStack;
+
+ std::function<bool(unsigned)> 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 <quic_asaravan at quicinc.com>
Date: Tue, 11 Nov 2025 15:50:16 +0530
Subject: [PATCH 3/8] Update llvm/lib/CodeGen/MachinePipeliner.cpp
Co-authored-by: Ryotaro Kasuga <kasuga.ryotaro at fujitsu.com>
---
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<unsigned, 8> 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 <quic_asaravan at quicinc.com>
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 <kasuga.ryotaro at fujitsu.com>
---
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 <quic_asaravan at quicinc.com>
Date: Tue, 11 Nov 2025 15:51:07 +0530
Subject: [PATCH 5/8] Update llvm/lib/CodeGen/MachinePipeliner.cpp
Co-authored-by: Ryotaro Kasuga <kasuga.ryotaro at fujitsu.com>
---
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<unsigned, SmallVector<unsigned, 2>> PhiDeps;
SmallSet<unsigned, 8> PhiRegs;
>From f7084ce1636f0ec198fd159d1b9e94b8b53abde1 Mon Sep 17 00:00:00 2001
From: quic-asaravan <quic_asaravan at quicinc.com>
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<unsigned, SmallVector<unsigned, 2>> PhiDeps;
- SmallSet<unsigned, 8> 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<unsigned, SmallVector<unsigned, 2>> &PhiDeps,
+ SmallSet<unsigned, 8> &Visited,
+ SmallSet<unsigned, 8> &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<unsigned, 8> Visited, RecStack;
+ RecStack.erase(Reg);
+ return false;
+}
- std::function<bool(unsigned)> 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<unsigned, SmallVector<unsigned, 2>> 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<unsigned, 8> 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 <quic_asaravan at quicinc.com>
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<unsigned, SmallVector<unsigned, 2>> &PhiDeps,
- SmallSet<unsigned, 8> &Visited,
- SmallSet<unsigned, 8> &RecStack) {
+ unsigned Reg, const DenseMap<unsigned, SmallVector<unsigned, 2>> &PhiDeps,
+ SmallSet<unsigned, 8> &Visited, SmallSet<unsigned, 8> &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<unsigned, SmallVector<unsigned, 2>> PhiDeps;
>From 9bac48e980e396a395949b9757803a9812c4a3d8 Mon Sep 17 00:00:00 2001
From: Abinaya Saravanan <quic_asaravan at quicinc.com>
Date: Mon, 17 Nov 2025 13:12:09 +0530
Subject: [PATCH 8/8] Update llvm/lib/CodeGen/MachinePipeliner.cpp
Co-authored-by: Ryotaro Kasuga <kasuga.ryotaro at fujitsu.com>
---
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.
More information about the llvm-commits
mailing list