[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: include/llvm/CodeGen/SelectionDAGNodes.h:665-666
 
+  bool isDivergent() const { return SDNodeBits.IsDivergent; }
+  void setIsDivergent(bool b) { SDNodeBits.IsDivergent = b; }
+
----------------
arsenm wrote:
> I have a general concern about this. The way this is used is going to not fit with how SelectionDAG APIs work, and is going to be very invasive. An SDNode is supposed to be immutable and some level of CSE is done by getNode. You can't have an API that involves setting a bit on a newly created node. Anything setting this needs to be done in getNode.
> 
> Are divergent and non-divergent nodes CSEable? These need to be handled somewhere to prevent them from folding.
> 
> You seem to only specially handle loads, but we have a lot of cases where we have combine issues from not knowing whether it's going to be selected to SALU or VALU instructions. If we have to somehow propagate this on every place a node is produced, that is a massive undertaking. I don't think that at this point it's worth trying to do such a level of work on SelectionDAG with GlobalISel on the way. Only handling loads I thought we could do just from the MemOperand.
I agree with you in general... I also don't like to explicitly propagate divergence flag in each place in combining or/and legalizing.
The problem is that getNode is not the only point where new SDNode may be created. For example getLoad and getExtLoad bypass getNode and create LoadSDNode explicitly. As for the handling divergence in CSE map... I maybe do not understand your point? If the node is CSEed we don't care is it divergent or not.


https://reviews.llvm.org/D35267