[PATCH] D140851: [Patch 3/4]: Add cases for assume (X & Y != {0|Y})

[Noah Goldstein via Phabricator via llvm-commits]

goldstein.w.n marked an inline comment as done.
goldstein.w.n added inline comments.


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp:4832-4834
+  // Canonicalize:
+  // 1. A & B_Pow2 != B_Pow2 -> A & B_Pow2 == 0
+  // 1. A & B_Pow2 == B_Pow2 -> A & B_Pow2 != 0
----------------
spatel wrote:
> This should be split into an independent patch with several tests.
> 
> Double-check to make sure this is the same logic, but I think it'd be shorter and easier to read without the loop:
>   // If Op1 is a power-of-2 (has exactly one bit set):
>   // (Op1 & X) == Op1 --> (Op1 & X) != 0
>   // (Op1 & X) != Op1 --> (Op1 & X) == 0
>   if (match(Op0, m_c_And(m_Specific(Op1), m_Value())) &&
>       isKnownToBeAPowerOfTwo(Op1, false, 0, &I)) {
>     return new ICmpInst(CmpInst::getInversePredicate(Pred), Op0,
>                         ConstantInt::getNullValue(Op0->getType()));
>   }
>   // If Op0 is a power-of-2 (has exactly one bit set):
>   // Op0 == (Op0 & X) --> (Op0 & X) != 0
>   // Op0 != (Op0 & X) --> (Op0 & X) == 0
>   if (match(Op1, m_c_And(m_Specific(Op0), m_Value())) &&
>       isKnownToBeAPowerOfTwo(Op0, false, 0, &I)) {
>     return new ICmpInst(CmpInst::getInversePredicate(Pred), Op1,
>                         ConstantInt::getNullValue(Op0->getType()));
>   }
> 
> This should be split into an independent patch with several tests.
> 
Sure, if I split it off will I be able to just revise the parent/child status of the surrounding reviews to link it in or will I need to make 5-new ones?

> Double-check to make sure this is the same logic, but I think it'd be shorter and easier to read without the loop:
>   // If Op1 is a power-of-2 (has exactly one bit set):
>   // (Op1 & X) == Op1 --> (Op1 & X) != 0
>   // (Op1 & X) != Op1 --> (Op1 & X) == 0
>   if (match(Op0, m_c_And(m_Specific(Op1), m_Value())) &&
>       isKnownToBeAPowerOfTwo(Op1, false, 0, &I)) {
>     return new ICmpInst(CmpInst::getInversePredicate(Pred), Op0,
>                         ConstantInt::getNullValue(Op0->getType()));
>   }
>   // If Op0 is a power-of-2 (has exactly one bit set):
>   // Op0 == (Op0 & X) --> (Op0 & X) != 0
>   // Op0 != (Op0 & X) --> (Op0 & X) == 0
>   if (match(Op1, m_c_And(m_Specific(Op0), m_Value())) &&
>       isKnownToBeAPowerOfTwo(Op0, false, 0, &I)) {
>     return new ICmpInst(CmpInst::getInversePredicate(Pred), Op1,
>                         ConstantInt::getNullValue(Op0->getType()));
>   }
> 

Will take a look for next version.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140851