[PATCH] D62191: [SelectionDAG] define binops as a superset of commutative binops

[Sanjay Patel via Phabricator via llvm-commits]

spatel marked an inline comment as done.
spatel added inline comments.


================
Comment at: llvm/include/llvm/CodeGen/TargetLowering.h:2199
   /// Returns true if the opcode is a commutative binary operation.
   virtual bool isCommutativeBinOp(unsigned Opcode) const {
     // FIXME: This should get its info from the td file.
----------------
lebedev.ri wrote:
> Actually, while this patch in itself looks good, this function looks a bit scary.
> FP ops are commutative, but not associative..
> 
Yes, that is scary. From what I can tell, we only call this via DAGCombiner::reassociateOps() starting from add/mul/and/or/xor. Add an assert on the value type to prevent misuse?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D62191/new/

https://reviews.llvm.org/D62191