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

[Brendon Cahoon via Phabricator via llvm-commits]

bcahoon added a comment.

Thanks for adding a patch to the pipeliner! I think what the patch is attempting is a good thing to add to the heuristic.  Though I would have expected that the check for hasDataDependence would generate the correct node order for the example provided in the comment? Maybe only if hasDataDependence is extended to handle other types of dependences?  But, this patch is probably cheaper since it pre-computes the information. I like the addition of the isValidNode as well.

I'm still evaluating this patch on some of our internal tests, so I may have some additional comments.  I'll reply in the next day or so.

Thanks,
Brendon



================
Comment at: lib/CodeGen/MachinePipeliner.cpp:2084
             else if (getHeight(I) == getHeight(maxHeight) &&
-                     getMOV(I) < getMOV(maxHeight) &&
-                     !hasDataDependence(maxHeight, I))
+                     getZeroLatencyHeight(I) > getZeroLatencyHeight(maxHeight))
               maxHeight = I;
----------------
I think this should be getZeroLatencyDepth(maxHeight)


Repository:
  rL LLVM

https://reviews.llvm.org/D43620