[llvm] [HEXAGON] [MachinePipeliner] Fix the DAG in case of dependent phis. (PR #135925)
Ryotaro Kasuga via llvm-commits
llvm-commits at lists.llvm.org
Wed Oct 29 18:56:53 PDT 2025
================
@@ -1214,8 +1214,27 @@ void SwingSchedulerDAG::updatePhiDependences() {
HasPhiDef = Reg;
// Add a chain edge to a dependent Phi that isn't an existing
// predecessor.
- if (SU->NodeNum < I.NodeNum && !I.isPred(SU))
- I.addPred(SDep(SU, SDep::Barrier));
+
+ // %3:intregs = PHI %21:intregs, %bb.6, %7:intregs, %bb.1 - SU0
+ // %7:intregs = PHI %21:intregs, %bb.6, %13:intregs, %bb.1 - SU1
+ // %27:intregs = A2_zxtb %3:intregs - SU2
+ // %13:intregs = C2_muxri %45:predregs, 0, %46:intreg
+ // If we have dependent phis, SU0 should be the successor of SU1
+ // not the other way around. (it used to be SU1 is the successor
+ // of SU0). In some cases, SU0 is scheduled earlier than SU1
+ // resulting in bad IR as we do not have a value that can be used
+ // by SU2.
+
+ // Reachability check is to ensure that we do not violate DAG.
+ // %1:intregs = PHI %10:intregs, %bb.0, %3:intregs, %bb.1 - SU0
+ // %2:intregs = PHI %10:intregs, %bb.0, %1:intregs, %bb.1 - SU1
+ // %3:intregs = PHI %11:intregs, %bb.0, %2:intregs, %bb.1 - SU2
+ // S2_storerb_io %0:intregs, 0, %2:intregs
+ // Make sure we do not create an edge between SU2 and SU0.
----------------
kasuga-fj wrote:
I think it's incorrect to avoid adding a dependency just because it would create a cycle. In this case, if we have `SU0 -> SU1` and `SU1 -> SU2`, then `SU2 -> SU0` would not be appended since `IsReachable(SU0, SU2)` would return true. This implies that `SU0` may be scheduled before `SU2`. Is that correct? If so, why? It feels like it contradicts the argument made in the former example in the comment.
`findCircuits` is irrelevant here. It's just a heuristic to determine the node order during scheduling. It doesn't change the dependencies.
https://github.com/llvm/llvm-project/pull/135925
More information about the llvm-commits
mailing list