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

[Stanislav Mekhanoshin via Phabricator via llvm-commits]

rampitec added a comment.

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

> In https://reviews.llvm.org/D35267#949394, @alex-t wrote:
>
> > In https://reviews.llvm.org/D35267#948666, @rampitec wrote:
> >
> > > There actually can be problem with folding the node if we patch it after creation. At least this needs to be checked.
> >
> >
> > That's true. The problem is that in SelectionDAG::getNode (where the SCEMap insertion is) we have no Value and no chance to check it's divergence.
> >  And this is correct:  SelectionDAG is for selection and we should not expose the IR Values to it.
> >
> > The only way I see is to pass the Divergence parameter to getNode from all the SelectionDAGBuilder visitors. This will be correct but requires to change** each of 109 visitors** and getNode().
>
>
> In fact we have no chance to have 2 SDNodes that differ by the Divergence flag only.
>  Please note that the selection operates per block. SelectionDAGBuilder construct the DAG for one block at a time.
>  Then it selects and emits the code. Then all the data including CSE map get cleared.
>  FoldingSetNodeID creates the hash including node and it's operands.
>  Thus we hit the hash only if there is same node with same operands.
>  Form the data dependency point it must have same divergence. So literally it is same node and setting same value of divergence flag makes no harm.
>  The only case when we could have 2 nodes that differ by the divergence only is if both have same operands but one is control-dependent of the divergent branch.
>  That immediately means that 2 nodes belong to different basic blocks and hence cannot be folded.


Thank you, that makes sense. I have no objections then, since creation of a node with a proper flags would result in a really massive patch changing all constructors and visitors. E.g. all node visitors would then need to gain a knowledge about a non node specific property. Post patching seems better to me.


https://reviews.llvm.org/D35267