[PATCH] D43620: [Pipeliner] Fixed node order issue related to zero latency edges

[Jan-Willem Roorda via Phabricator via llvm-commits]

jwroorda marked 2 inline comments as done.
jwroorda added a comment.

Thanks for your valuable feedback. I have made the changes that you have suggested.



================
Comment at: lib/CodeGen/MachinePipeliner.cpp:1615
                                   getDistance(pred, SU, *IP) * MII));
+      if (getLatency(SU, *IP) == 0)
+        zeroLatencyDepth =
----------------
bcahoon wrote:
> I think the zeroLatencyDepth (and Height) calculation needs to be done prior to the ignoreDependence() check. Otherwise, Anti dependence edges won't be considered in the ZLD/ZLH calculation.
I agree. This is fixed now.


================
Comment at: lib/CodeGen/MachinePipeliner.cpp:2130
             else if (getDepth(I) == getDepth(maxDepth) &&
-                     getMOV(I) < getMOV(maxDepth) &&
-                     !hasDataDependence(I, maxDepth))
+                     getZeroLatencyDepth(I) > getZeroLatencyHeight(maxDepth))
               maxDepth = I;
----------------
bcahoon wrote:
> this is the place where I meant to add the change - 
> getZeroLatencyHeight(maxDepth) with getZeroLatencyDepth(maxDepth)
Well spotted. This is fixed now.


https://reviews.llvm.org/D43620