[llvm-commits] [llvm] r120028 - in /llvm/trunk: lib/Target/README.txt lib/Transforms/InstCombine/InstCombineShifts.cpp test/Transforms/InstCombine/shift.ll

[Benjamin Kramer]

On 23.11.2010, at 20:56, Frits van Bommel wrote:

> On Tue, Nov 23, 2010 at 7:52 PM, Benjamin Kramer
> <benny.kra at googlemail.com> wrote:
>> Author: d0k
>> Date: Tue Nov 23 12:52:42 2010
>> New Revision: 120028
>> 
>> URL: http://llvm.org/viewvc/llvm-project?rev=120028&view=rev
>> Log:
>> InstCombine: Reduce "X shift (A srem B)" to "X shift (A urem B)" iff B is positive.
>> 
>> This allows to transform the rem in "1 << ((int)x % 8);" to an and.
> 
> Is this still worth it if the srem has multiple uses?
> If the srem isn't optimized out completely you'd end up doing extra work here.
> 
>> --- llvm/trunk/lib/Transforms/InstCombine/InstCombineShifts.cpp (original)
>> +++ llvm/trunk/lib/Transforms/InstCombine/InstCombineShifts.cpp Tue Nov 23 12:52:42 2010
>> @@ -53,6 +53,21 @@
>>   if (ConstantInt *CUI = dyn_cast<ConstantInt>(Op1))
>>     if (Instruction *Res = FoldShiftByConstant(Op0, CUI, I))
>>       return Res;
>> +
>> +  // X shift (A srem B) -> X shift (A urem B) iff B is positive.
>> +  // Because shifts by negative values are undefined.
>> +  if (BinaryOperator *BO = dyn_cast<BinaryOperator>(Op1))
>> +    if (BO->getOpcode() == Instruction::SRem && BO->getType()->isIntegerTy()) {
> 
> Maybe add a "&& BO->hasOneUse()" test here?

Yup, fixed in r120049. Thanks for the review.