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

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

jwroorda added a comment.

Thanks for your comments!

> I would have expected that the check for hasDataDependence would generate the correct node order for the example provided in the comment?

I see your point. I agree that intuitively/at first sight, one might expect that the check for hasDataDependence would guarantee a correct node order.
However, I don't think this is the case.  I have tried to illustrate this in the example. That is, I have tried to explain why the algorithm 
(using the hasDataDependence check) can arrive at a node-order in which node n3 is scheduled after its predecessor n0 and after its successor n2.

The crucial point in the example is that hasDataDependence is never called to check for a dependence between node n2 and n3. 
Instead, it only checks for dependencies between nodes n4 and n3 and between nodes n4 and n2.

I don’t think that extending the hasDataDependence-check to other types of dependencies would help. 
For the sake of argument, you can consider all dependencies in the example to be data-dependencies.

I have tried to explain the issue in more detail in the commit-message, see below.

Can you please check if you agree with the example in the commit-message? 
If there is anything unclear, or if you believe that there is a mistake somewhere, please let me know.

Below follows the relevant part of the commit message:
"The reason that the algorithm can put node n2 in the order before node n3,
even though they have an edge between them in which node n3 is the source,
is the following: Suppose the algorithm has constructed the partial node
order n0, n1. Then, the nodes left to be ordered are nodes n2, n3, and n4. Suppose
that the while-loop in the implemented algorithm considers the nodes in
the order n4, n3, n2. The algorithm will start with node n4, and look for
more preferable nodes. First, node n4 will be compared with node n3. As the nodes
have equal Height and Movability and have no edge between them, the algorithm
will stick with node n4. Then node n4 is compared with node n2. Again the
Height and Movability are equal. But, this time, there is an edge between
the two nodes, and the algorithm will prefer the source node n2.
As there are no nodes left to compare, the algorithm will add node n2 to
the node order, yielding the partial node order n0, n1, n2. In this way node n2
arrives in the node-order before node n3."



================
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;
----------------
bcahoon wrote:
> I think this should be getZeroLatencyDepth(maxHeight)
Can you explain why? I think that in, in top-down mode, nodes with greater height should be scheduled first.


Repository:
  rL LLVM

https://reviews.llvm.org/D43620