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

[Eli Friedman via Phabricator via llvm-commits]

efriedma added a comment.

In https://reviews.llvm.org/D35267#962949, @alex-t wrote:

> > This is only true if your original computation is correct, and if DAGCombine/Legalization doesn't create any nodes which are naturally divergent.  Neither of those are safe assumptions, I think.  DAGCombine and legalization will transform loads and stores, which could end up creating a naturally divergent node.
>
> So, my question is: could you imagine even theoretical sensible transformation that convert the graph in such a way that uniform node will get divergent income?


No, but that isn't the point.  The problem is that you could replace a naturally divergent node with an equivalent naturally divergent node, but the new node doesn't have the divergent bit set (since the bit only gets set in DAGCombine for nodes with divergent operands, and naturally divergent nodes might not have divergent operands).  Thinking about it a bit more, I guess regular load/store operations are a bad example; if a load produced multiple values given a uniform address, it would be a data race.  But I think atomic memory operations could run into this issue?  (Consider, for example, the code in DAGTypeLegalizer::PromoteIntRes_Atomic1.)

In https://reviews.llvm.org/D35267#962949, @alex-t wrote:

> > And some divergent nodes will never be passed to SelectionDAGBuilder::setValue when you build the DAG, due to the way SelectionDAGBuilder handles values with illegal types.  But I'm not sure that's a complete list of the issues with the current version, and there's no practical way to check without a verifier.
>
> Even if it creates new DAG pattern it returns it's root that (because of CreateOperands) has correct divergence that will be passed to setValue. Or I did not understand what you meant?


That's not what I meant.

Say you have a call to a divergent function which returns an i64, but i64 isn't legal on your target (so the function effectively returns two values of type i32).  We create the call, a couple CopyFromReg nodes, and then a MERGE_VALUES to merge the value.  Then you set the MERGE_VALUES to be divergent... but that isn't really helpful: legalization for MERGE_VALUES erases the node, so the "divergent" bit goes away.


https://reviews.llvm.org/D35267