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

[Roman Lebedev via Phabricator via llvm-commits]

lebedev.ri marked an inline comment as done.
lebedev.ri 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.
----------------
spatel wrote:
> 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?
Asserts are great.


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

https://reviews.llvm.org/D62191