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

[Mehrnoosh Heidarpour via Phabricator via llvm-commits]

MehrHeidar added a comment.

In D115755#3204690 <https://reviews.llvm.org/D115755#3204690>, @spatel wrote:

> In D115755#3204659 <https://reviews.llvm.org/D115755#3204659>, @spatel wrote:
>
>> The code seems more complicated than necessary (and the tests less thorough than necessary). 
>> We know that the 'or' instruction is a shared op, so let's match that first and then match it again as a specific value? 
>> If I'm seeing it correctly, that means there are 8 potential commutes:
>>
>>   BinaryOperator *Or;
>>   if (match(Op0, m_c_Xor(m_Value(X), m_BinOp(Or))))
>>     if (match(Or, m_c_Or(m_Specific(X), m_Value(Y))) &&
>>         match(Op1, m_c_Xor(m_Specific(Or), m_Specific(Y))))
>>       return Constant::getNullValue(Op0->getType());
>
> On 2nd thought, that might not be complicated enough. :)
> I think this could match a "wrong" binop first (for example Y is an add instruction) and then fail to match the pattern (so we could use even more tests...).
> We probably need to use `m_CombineAnd` to get this to accurately match Op0 and capture the 'or' in one shot:
>
>   BinaryOperator *Or;
>   if (match(Op0, m_c_Xor(m_Value(X),
>                          m_CombineAnd(m_BinOp(Or),
>                                       m_c_Or(m_Deferred(X), m_Value(Y))))) &&
>       match(Op1, m_c_Xor(m_Specific(Or), m_Specific(Y))))
>     return Constant::getNullValue(Op0->getType());



In D115755#3204690 <https://reviews.llvm.org/D115755#3204690>, @spatel wrote:

> In D115755#3204659 <https://reviews.llvm.org/D115755#3204659>, @spatel wrote:
>
>> The code seems more complicated than necessary (and the tests less thorough than necessary). 
>> We know that the 'or' instruction is a shared op, so let's match that first and then match it again as a specific value? 
>> If I'm seeing it correctly, that means there are 8 potential commutes:
>>
>>   BinaryOperator *Or;
>>   if (match(Op0, m_c_Xor(m_Value(X), m_BinOp(Or))))
>>     if (match(Or, m_c_Or(m_Specific(X), m_Value(Y))) &&
>>         match(Op1, m_c_Xor(m_Specific(Or), m_Specific(Y))))
>>       return Constant::getNullValue(Op0->getType());
>
> On 2nd thought, that might not be complicated enough. :)
> I think this could match a "wrong" binop first (for example Y is an add instruction) and then fail to match the pattern (so we could use even more tests...).
> We probably need to use `m_CombineAnd` to get this to accurately match Op0 and capture the 'or' in one shot:
>
>   BinaryOperator *Or;
>   if (match(Op0, m_c_Xor(m_Value(X),
>                          m_CombineAnd(m_BinOp(Or),
>                                       m_c_Or(m_Deferred(X), m_Value(Y))))) &&
>       match(Op1, m_c_Xor(m_Specific(Or), m_Specific(Y))))
>     return Constant::getNullValue(Op0->getType());

Thank you!
But, this one is not complicated enough to catch this pattern : )   -->`(Y ^ (Y | X) ) & ((X | Y) ^ X) --> 0`

  %or1 = or i32 %y, %x
   %or2 = or i32 %x, %y
   %xor1 = xor i32 %y, %or1
   %xor2 = xor i32 %or2, %x
   %and = and i32 %xor1, %xor2
   ret i32 %and




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115755