[PATCH] D43620: [Pipeliner] Fixed node order issue related to zero latency edges
Brendon Cahoon via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Tue Feb 27 20:13:12 PST 2018
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.
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)
More information about the llvm-commits