[llvm] r185272 - ValueTracking: Teach isKnownToBeAPowerOfTwo about (ADD X, (XOR X, Y)) where X is a power of two

[David Majnemer]

On Tue, Jul 9, 2013 at 10:28 AM, David Majnemer <david.majnemer at gmail.com>wrote:

> On Jul 9, 2013, at 5:34 AM, Jay Foad <jay.foad at gmail.com> wrote:
>
> > On 30 June 2013 00:44, David Majnemer <david.majnemer at gmail.com> wrote:
> >> Author: majnemer
> >> Date: Sat Jun 29 18:44:53 2013
> >> New Revision: 185272
> >>
> >> URL: http://llvm.org/viewvc/llvm-project?rev=185272&view=rev
> >> Log:
> >> ValueTracking: Teach isKnownToBeAPowerOfTwo about (ADD X, (XOR X, Y))
> where X is a power of two
> >>
> >> This allows us to simplify urem instructions involving the add+xor to
> >> turn into simpler math.
> >>
> >> Modified:
> >>    llvm/trunk/lib/Analysis/ValueTracking.cpp
> >>    llvm/trunk/test/Transforms/InstCombine/rem.ll
> >>
> >> Modified: llvm/trunk/lib/Analysis/ValueTracking.cpp
> >> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Analysis/ValueTracking.cpp?rev=185272&r1=185271&r2=185272&view=diff
> >>
> ==============================================================================
> >> --- llvm/trunk/lib/Analysis/ValueTracking.cpp (original)
> >> +++ llvm/trunk/lib/Analysis/ValueTracking.cpp Sat Jun 29 18:44:53 2013
> >> @@ -855,16 +855,24 @@ bool llvm::isKnownToBeAPowerOfTwo(Value
> >>     return false;
> >>   }
> >>
> >> -  // Adding a power of two to the same power of two is a power of two
> or zero.
> >> -  if (OrZero && match(V, m_Add(m_Value(X), m_Value(Y)))) {
> >> -    if (match(X, m_And(m_Value(), m_Specific(Y)))) {
> >> -      if (isKnownToBeAPowerOfTwo(Y, /*OrZero*/true, Depth))
> >> -        return true;
> >> -    } else if (match(Y, m_And(m_Value(), m_Specific(X)))) {
> >> -      if (isKnownToBeAPowerOfTwo(X, /*OrZero*/true, Depth))
> >> -        return true;
> >> -    }
> >> -  }
> >> +  if (match(V, m_Add(m_Value(X), m_Value(Y))))
> >> +    if (OverflowingBinaryOperator *VOBO =
> cast<OverflowingBinaryOperator>(V))
> >> +      if (OrZero || VOBO->hasNoUnsignedWrap() ||
> VOBO->hasNoSignedWrap()) {
> >> +        // Adding a power of two to the same power of two is a power
> of two or
> >> +        // zero.
> >> +        if (BinaryOperator *XBO = dyn_cast<BinaryOperator>(X))
> >> +          if (XBO->getOpcode() == Instruction::And ||
> >> +              XBO->getOpcode() == Instruction::Xor)
> >> +            if (XBO->getOperand(0) == Y || XBO->getOperand(1) == Y)
> >> +              if (isKnownToBeAPowerOfTwo(Y, /*OrZero*/true, Depth))
> >> +                return true;
> >> +        if (BinaryOperator *YBO = dyn_cast<BinaryOperator>(Y))
> >> +          if (YBO->getOpcode() == Instruction::And ||
> >> +              YBO->getOpcode() == Instruction::Xor)
> >> +            if (YBO->getOperand(0) == X || YBO->getOperand(1) == X)
> >> +              if (isKnownToBeAPowerOfTwo(X, /*OrYero*/true, Depth))
> >> +                return true;
> >> +      }
> >>
> >>   // An exact divide or right shift can only shift off zero bits, so
> the result
> >>   // is a power of two only if the first operand is a power of two and
> not
> >
> > The original code said that if X is a power of two or zero, then
> > X+(X&Y) is a power of two or zero. OK.
> >
> > The new code says that if X is a power of two or zero, then X+(X&Y)
> > and X+(X^Y) are both powers of two (not zero). (I'm assuming the + is
> > a normal C addition of signed integers, so it hasNoSignedWrap().)
> >
> > This is clearly wrong if X is zero; 0+(0&99) is not a power of two.
>
> It is not a power of two but it is zero. If X is zero then X+(X&Y) is zero.
>

Ah, I think I see what you are saying. We can only allow this if the caller
is OK with getting zero, we should pass the OrZero flag down into the
recursive step.


>
> >
> > It's also clearly wrong if Y has bits set that are not set in X;
> > 4+(4^99) is not a power of two.
> >
> > Or have I misread the code?
>
> This looks like it is wrong. I'll fix it.
>
> >
> > Jay.
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130709/24554355/attachment.html>