[PATCH] D53106: [SelectionDAG] Fix behavior of glued nodes in hasPredecessorHelper.

[Nirav Dave via Phabricator via llvm-commits]

niravd added a comment.

Good point. Scenario 1 may happen if we have nodes A, B, C, GlueOp, and GlueUser such that there's uses (A-> GlueUse, GlueOp->GlueUser, GlueOp->B, B-> C) and the node id ordering is GOp < B < A < GU < C then checking if A is a predecessor to C would stop searching at B and fail to see the remaining. I am unsure if this is possible currently given our id scheme, but I'll take a look. I believe it will always bias towards selecting A before B.

If it's not safe, we can preclude this case by labeling Glued node should have the same node id.

Scenario 2 is not possible as m is +infinitty.

  A
   |
  v 

GOp -> GU

     |
    v
    B
    |
   v
  C

In https://reviews.llvm.org/D53106#1261497, @TimNN wrote:

> The approach in general looks good to me, however I don't think this interacts correctly with the `TopologicalPrune` optimization.
>
> (At this time, I'm not familiar enough with the code to intuitively know in which direction the topological sorting is used, but one of the two scenarios below //will// apply).
>
> Assume that the optimization limits the nodes to be consider to the range [`a`, `b`].
>
> Scenario 1) A non-glue edge goes past `a` to a node `n`. Since `n` is outside the range considered, it won't be processed even though it may have reverse glue edges back into the range.
>
> Scenario 2) A reverse glue edge goes past `b` to a node `m`. Since `m` is outside the range considered, it won't be processed even though it may have normal edges back into the range.





Repository:
  rL LLVM

https://reviews.llvm.org/D53106