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

[Frits van Bommel]

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?

> +      // Make sure the divisor's sign bit is zero.
> +      APInt Mask = APInt::getSignBit(BO->getType()->getPrimitiveSizeInBits());
> +      if (MaskedValueIsZero(BO->getOperand(1), Mask)) {
> +        Value *URem = Builder->CreateURem(BO->getOperand(0), BO->getOperand(1),
> +                                          BO->getName());
> +        I.setOperand(1, URem);
> +        return &I;
> +      }
> +    }
> +
>   return 0;
>  }