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

[Sanjay Patel via Phabricator via llvm-commits]

spatel 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
----------------
goldstein.w.n wrote:
> 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.
It's independent of anything else, so there's no need to update the other patches up for review? Just pull out this hunk and regenerate test diffs if needed. 

I just added some tests here:
ad14cef1d530


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140851