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

[Alexander via Phabricator via llvm-commits]

alex-t added inline comments.


================
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];
----------------
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


https://reviews.llvm.org/D35267