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

[Duncan Sands]

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?

Ciao, Duncan.

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