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

Eli Friedman via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Dec 8 13:24:32 PST 2017


efriedma added a comment.

> Any DAG transformation that change divergent pattern to not-divergent or vice versa is illegal.

Transforming "x*0 -> 0" is illegal if x is divergent?  That seems surprising.



================
Comment at: lib/CodeGen/SelectionDAG/SelectionDAGBuilder.h:688
   void setValue(const Value *V, SDValue NewN) {
+    NewN.getNode()->SDNodeBits.IsDivergent = DA ? DA->isDivergent(V) : 0;
     SDValue &N = NodeMap[V];
----------------
alex-t wrote:
> efriedma wrote:
> > You're mutating the node after it's been inserted into the CSEMap, which is generally bad.  Also, it's not clear this is the node you need to set the "divergent" bit on (NewN could be something which will be eliminated by DAGCombine, like a BITCAST or MERGE_VALUES).  Can we do this some other way which is more obviously correct?
> You are right.
> BTW this is not the only place mutating SDNode that has already been created.
> Look in SelectionDAG.cpp lines: 4719-4722
> 
>     if (SDNode *E = FindNodeOrInsertPos(ID, DL, IP)) {
>       E->intersectFlagsWith(Flags);
>       return SDValue(E, 0);
>     }
> 
> SDnode that found in the map is mutated and then returned w/o any memoization of the mutation
I'm more concerned about whether you're attaching the "divergent" bit to the right node.  The mutation is probably mostly harmless; as you note, other places mess with flags.


https://reviews.llvm.org/D35267





More information about the llvm-commits mailing list