[PATCH] D115755: [InstSimplify] Fold logic And to Zero

[Mehrnoosh Heidarpour via Phabricator via llvm-commits]

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


================
Comment at: llvm/lib/Analysis/InstructionSimplify.cpp:2180
+    return Constant::getNullValue(Op0->getType());
+  // ((A | B) ^ B ) & ((A | B) ^ A) --> 0
+  if (match(Op0, m_c_Xor(m_c_Or(m_Value(X), m_Value(Y)), m_Deferred(Y))) &&
----------------
rampitec wrote:
> MehrHeidar wrote:
> > rampitec wrote:
> > > You could use 'm_CombineOr' for the LHS to select either deferred X or deferred Y. Then you do not need second expression.
> > I replaced the comments and changed the usage of A/B with X/Y.
> > Also, I tried to use the idea for usage of `m_CombineOr` to remove the second expression.
> Second match shall not use m_CombineOr. Now you would incorrectly match `((X | Y) ^ X ) & ((X | Y) ^ X)`. Speaking of which it deserves a negative test.
If I don't put the `m_CombineOr` on the second match, we won't be able to catch all commutes. I added a negative test now to see it won't incorrectly match your example.

I am not so confident with the current code too and prefer my first version of this patch. What do you think @rampitec ?



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115755