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

[Mehrnoosh Heidarpour via Phabricator via llvm-commits]

MehrHeidar marked 5 inline comments 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:
> 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.


================
Comment at: llvm/test/Transforms/InstSimplify/and.ll:168
   %or = or i71 %x, %y
   %xor1 = xor i71 %x, %or
   %xor2 = xor i71 %y, %or
----------------
rampitec wrote:
> spatel wrote:
> > rampitec wrote:
> > > 'or' is more complex than the argument, the xor will be commuted.
> > > See InstCombiner::getComplexity() for the details. Search for "thwart complexity-based canonicalization" in the llvm/test/Transforms/InstCombine directory for test coverage that works around it.
> > This is instsimplify, so we don't have to worry about the pass itself altering the input (this is shown in the baseline CHECK lines).
> Ok, thanks.
I think the test coverage looks fine, based on @spatel  comment,



> Quoted Text


Right?


================
Comment at: llvm/test/Transforms/InstSimplify/and.ll:168
   %or = or i71 %x, %y
   %xor1 = xor i71 %x, %or
   %xor2 = xor i71 %y, %or
----------------
MehrHeidar wrote:
> rampitec wrote:
> > spatel wrote:
> > > rampitec wrote:
> > > > 'or' is more complex than the argument, the xor will be commuted.
> > > > See InstCombiner::getComplexity() for the details. Search for "thwart complexity-based canonicalization" in the llvm/test/Transforms/InstCombine directory for test coverage that works around it.
> > > This is instsimplify, so we don't have to worry about the pass itself altering the input (this is shown in the baseline CHECK lines).
> > Ok, thanks.
> I think the test coverage looks fine, based on @spatel  comment,
> 
> 
> 
> > Quoted Text
> 
> 
> Right?
I think according to the comment the test coverage is fine.
> This is instsimplify, so we don't have to worry about the pass itself altering the input (this is shown in the baseline CHECK lines).

Right?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115755