<div dir="ltr">On Tue, Jul 9, 2013 at 10:28 AM, David Majnemer <span dir="ltr"><<a href="mailto:david.majnemer@gmail.com" target="_blank">david.majnemer@gmail.com</a>></span> wrote:<br><div class="gmail_extra"><div class="gmail_quote">
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div class=""><div class="h5">On Jul 9, 2013, at 5:34 AM, Jay Foad <<a href="mailto:jay.foad@gmail.com">jay.foad@gmail.com</a>> wrote:<br>
<br>
> On 30 June 2013 00:44, David Majnemer <<a href="mailto:david.majnemer@gmail.com">david.majnemer@gmail.com</a>> wrote:<br>
>> Author: majnemer<br>
>> Date: Sat Jun 29 18:44:53 2013<br>
>> New Revision: 185272<br>
>><br>
>> URL: <a href="http://llvm.org/viewvc/llvm-project?rev=185272&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=185272&view=rev</a><br>
>> Log:<br>
>> ValueTracking: Teach isKnownToBeAPowerOfTwo about (ADD X, (XOR X, Y)) where X is a power of two<br>
>><br>
>> This allows us to simplify urem instructions involving the add+xor to<br>
>> turn into simpler math.<br>
>><br>
>> Modified:<br>
>> llvm/trunk/lib/Analysis/ValueTracking.cpp<br>
>> llvm/trunk/test/Transforms/InstCombine/rem.ll<br>
>><br>
>> Modified: llvm/trunk/lib/Analysis/ValueTracking.cpp<br>
>> URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Analysis/ValueTracking.cpp?rev=185272&r1=185271&r2=185272&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Analysis/ValueTracking.cpp?rev=185272&r1=185271&r2=185272&view=diff</a><br>
>> ==============================================================================<br>
>> --- llvm/trunk/lib/Analysis/ValueTracking.cpp (original)<br>
>> +++ llvm/trunk/lib/Analysis/ValueTracking.cpp Sat Jun 29 18:44:53 2013<br>
>> @@ -855,16 +855,24 @@ bool llvm::isKnownToBeAPowerOfTwo(Value<br>
>> return false;<br>
>> }<br>
>><br>
>> - // Adding a power of two to the same power of two is a power of two or zero.<br>
>> - if (OrZero && match(V, m_Add(m_Value(X), m_Value(Y)))) {<br>
>> - if (match(X, m_And(m_Value(), m_Specific(Y)))) {<br>
>> - if (isKnownToBeAPowerOfTwo(Y, /*OrZero*/true, Depth))<br>
>> - return true;<br>
>> - } else if (match(Y, m_And(m_Value(), m_Specific(X)))) {<br>
>> - if (isKnownToBeAPowerOfTwo(X, /*OrZero*/true, Depth))<br>
>> - return true;<br>
>> - }<br>
>> - }<br>
>> + if (match(V, m_Add(m_Value(X), m_Value(Y))))<br>
>> + if (OverflowingBinaryOperator *VOBO = cast<OverflowingBinaryOperator>(V))<br>
>> + if (OrZero || VOBO->hasNoUnsignedWrap() || VOBO->hasNoSignedWrap()) {<br>
>> + // Adding a power of two to the same power of two is a power of two or<br>
>> + // zero.<br>
>> + if (BinaryOperator *XBO = dyn_cast<BinaryOperator>(X))<br>
>> + if (XBO->getOpcode() == Instruction::And ||<br>
>> + XBO->getOpcode() == Instruction::Xor)<br>
>> + if (XBO->getOperand(0) == Y || XBO->getOperand(1) == Y)<br>
>> + if (isKnownToBeAPowerOfTwo(Y, /*OrZero*/true, Depth))<br>
>> + return true;<br>
>> + if (BinaryOperator *YBO = dyn_cast<BinaryOperator>(Y))<br>
>> + if (YBO->getOpcode() == Instruction::And ||<br>
>> + YBO->getOpcode() == Instruction::Xor)<br>
>> + if (YBO->getOperand(0) == X || YBO->getOperand(1) == X)<br>
>> + if (isKnownToBeAPowerOfTwo(X, /*OrYero*/true, Depth))<br>
>> + return true;<br>
>> + }<br>
>><br>
>> // An exact divide or right shift can only shift off zero bits, so the result<br>
>> // is a power of two only if the first operand is a power of two and not<br>
><br>
> The original code said that if X is a power of two or zero, then<br>
> X+(X&Y) is a power of two or zero. OK.<br>
><br>
> The new code says that if X is a power of two or zero, then X+(X&Y)<br>
> and X+(X^Y) are both powers of two (not zero). (I'm assuming the + is<br>
> a normal C addition of signed integers, so it hasNoSignedWrap().)<br>
><br>
> This is clearly wrong if X is zero; 0+(0&99) is not a power of two.<br>
<br>
</div></div>It is not a power of two but it is zero. If X is zero then X+(X&Y) is zero.<br></blockquote><div><br></div><div style>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.</div>
<div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<div class="im"><br>
><br>
> It's also clearly wrong if Y has bits set that are not set in X;<br>
> 4+(4^99) is not a power of two.<br>
><br>
> Or have I misread the code?<br>
<br>
</div>This looks like it is wrong. I'll fix it.<br>
<br>
><br>
> Jay.<br>
</blockquote></div><br></div></div>