[llvm-commits] [llvm] r162743 - in /llvm/trunk: lib/Transforms/InstCombine/InstCombineMulDivRem.cpp test/Transforms/InstCombine/2012-08-28-udiv_ashl.ll test/Transforms/InstCombine/udiv-simplify-bug-1.ll

[Dirk Steinke]

Hi Nadav.

On 08/28/2012 12:01 PM, Nadav Rotem wrote:
> Author: nadav
> Date: Tue Aug 28 05:01:43 2012
> New Revision: 162743
>
> URL: http://llvm.org/viewvc/llvm-project?rev=162743&view=rev
> Log:
>
> Teach InstCombine to canonicalize  [SU]div+[AL]shl patterns.
>
> For example:
>    %1 = lshr i32 %x, 2
>    %2 = udiv i32 %1, 100
>
> rdar://12182093
>
> [snip]
>
> +  // Sdiv ((Ashl x, c1) , c2) ->   x / (C1 * 1<<C2);
> +  if (Constant *C = dyn_cast<Constant>(Op1)) {
> +    Value *X = 0, *C1 = 0;
> +    if (match(Op0, m_AShr(m_Value(X), m_Value(C1)))) {
> +      uint64_t NC = cast<ConstantInt>(C)->getZExtValue() *
> +                    (1<<  cast<ConstantInt>(C1)->getZExtValue());
> +      return BinaryOperator::CreateSDiv(X, ConstantInt::get(I.getType(), NC));
> +    }
> +  }
> +
 > [snip]

The transformation
   // Sdiv ((Ashr x, c1) , c2) ->   x / (C2 * 1<<C1);
does not sound right to me.
E.g. (-7 >> 2) / 2 = -2 / 2 = -1; -7 / (2 * 1<<2) = -7 / 8 = 0.
It might work if the low bits of x are all zero though.

If I'm missing something, please feel free to correct me.

On a side note: the description comment of the Udiv transform should use 
Lshr, not Lshl. Similar for Sdiv.

Bye
Dirk