[llvm-commits] [llvm] r147391 - in /llvm/trunk: lib/Transforms/InstCombine/InstCombineShifts.cpp test/Transforms/InstCombine/shift.ll

[Nick Lewycky]

On 01/03/2012 11:51 PM, Duncan Sands wrote:
> Hi Nick,
>
>>>>>> @@ -603,9 +602,16 @@
>>>>>> // (X>>? C1)<< C2 --> X>>? (C1-C2)& (-1<< C2)
>>>>>> if (I.getOpcode() == Instruction::Shl&&
>>>>>> ShiftOp->getOpcode() != Instruction::Shl) {
>>>>>> - Value *Shift = Builder->CreateBinOp(ShiftOp->getOpcode(), X,
>>>>>> - ConstantInt::get(Ty, ShiftDiff));
>>>>>> -
>>>>>> + ConstantInt *ShiftDiffCst = ConstantInt::get(Ty, ShiftDiff);
>>>>>> + if (ShiftOp->isExact()) {
>>>>>> + // (X>>?exact C1)<< C2 --> X>>?exact (C1-C2)
>>>>>
>>>>> what happens if C2 is bigger than C1?
>>>>
>>>> We won't reach here. It extracts C1 and C2 into ShiftAmt1 and
>>>> ShiftAmt2 and does
>>>> if (ShiftAmt1 == ShiftAmt2) { ... } else if (ShiftAmt1 < ShiftAmt2) {
>>>> ... } else
>>>> { our transform here }.
>>>
>>> OK, thanks. However if C2 is bigger than C1 then, thanks to the exact
>>> flag, you
>>> can just turn it into a left shift (without an "and"). Likewise, for C1
>>> == C2
>>> the pair of shifts becomes a no-op. But maybe those are handled already?
>>
>> We handle the exact bit in the C1 == C2 case already. When C2 > C1,
>> you can't
>> use exact to remove the 'and' operation the shl is adding more zero
>> bits than
>> the shr guaranteed would be there, with or without exact.
>
> not sure what you are saying here. As far as I can see (X >>exact 1) <<
> 3 is
> the same thing as X << 2. The point of an exact shift is that it is
> invertible:
> (X >>exact N) << N equals X. Thus, writing (X >>exact 1) << 3 as
> ((X >>exact 1) << 1) << 2, you get that it simplifies to X << 2 by the
> above.
> No "and" required. Am I missing something?

Nope, you're right. (I confused myself, thinking that the 'and' was 
still necessary to clear the bottom bits, but that we should shrink the 
size of the and constant but not remove it. Of course, a shift-left 
always fills in zeros.)

Patch on the way. :)

Nick

>
> Ciao, Duncan.
>
> There may be something
>> more you can do with nuw/nsw on the 'shl', but I couldn't see anything
>> obvious.
>
>
>