[PATCH] D35267: Pass Divergence Analysis data to selection DAG to drive divergence dependent instruction selection

[Alexander via Phabricator via llvm-commits]

alex-t added a comment.

In https://reviews.llvm.org/D35267#960250, @efriedma wrote:

> Actually, what I'd really like to see here is some sort of verifier for the divergent bit.  It should be possible to recompute the divergence of the SelectionDAG at any point from first principles. There's a small set of operations which are fundamentally divergent: CopyFromReg where the register contains a divergent value (you should be able to derive this from DivergenceAnalysis), divergent memory accesses, and some target-specific intrinsics.  (Not sure that's a complete list, but should be close.)  All other operations are divergent if and only if they have a divergent predecessor.


What you described is exactly the way how the  Divergence Analysis works.  Do you really consider creating one more DA upon the Selection DAG?

In https://reviews.llvm.org/D35267#952801, @efriedma wrote:

> In https://reviews.llvm.org/D35267#950795, @alex-t wrote:
>
> > In this case w/o bit propagation we're still correct but sub-optimal.
>
>
> I'm worried that you're covering up bugs by accepting "sub-optimal" results.  Specifically, if you have a node which is marked divergent, but doesn't actually have any divergent operands, it will stay marked divergent in most cases... but if DAGCombine or legalization transforms it to some other equivalent operation, it'll erase the "divergent" marking.  So your markings will look mostly correct in simple cases, but break for more complicated cases.


I still insist that divergence bit propagation over the use replacement is not necessary. In most cases it is useless.
Please note that any node that was created in the combiner/legalizer transformation already has the correct divergence because in CreateOperands the new node divergence is computed as OR over it's operands divergence bits.
Even if some transformation is managed to change the non-divergent use with the divergent one it is illegal.
The corner case in your example that turns variable into the constant can never lead to incorrect code. In the worst case we'll have a splat of zeroes that waste vector register. So the code is still correct but is not optimal.


https://reviews.llvm.org/D35267