[PATCH] D65802: [DAGCombiner] Rebuild (setcc x, y, ==) from (xor (xor x, y), 1)

[Roger Ferrer Ibanez via Phabricator via llvm-commits]

rogfer01 marked 3 inline comments as done.
rogfer01 added inline comments.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp:13496-13499
+  // Transform (brcond (xor x, y)) -> (brcond (setcc, x, y, ne))
+  // Transform (brcond (xor (xor x, y), -1)) -> (brcond (setcc, x, y, eq))
+  if (N.getOpcode() == ISD::XOR && N->hasOneUse() &&
+      (*N->use_begin())->getOpcode() == ISD::BRCOND) {
----------------
lebedev.ri wrote:
> rogfer01 wrote:
> > lebedev.ri wrote:
> > > Why is this one-use restriction being introduced here?
> > There are two callers of this function `visitSETCC`, which passes a node generated by `SimplifySetCC`, and `visitBRCOND` which does indeed check that the passed node has just one use.
> > 
> > I'm less sure about the the node coming out of `SimplifySetCC` so it might make sense to err on the safe side here?
> `(*N->use_begin())->getOpcode() == ISD::BRCOND)` should go, or make it an assert.
> 
> I'm not really sure what's on the other unsafe side, against what is this protecting?
> 
@lebedev.ri @spatel I removed the "one use" check but I'm under the impression that we still want the assert, do we?


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

https://reviews.llvm.org/D65802