[llvm-commits] [llvm] r162749 - in /llvm/trunk: lib/Transforms/InstCombine/InstCombineMulDivRem.cpp test/Transforms/InstCombine/2012-08-28-udiv_ashl.ll

[Benjamin Kramer]

On 28.08.2012, at 14:23, Nadav Rotem <nrotem at apple.com> wrote:

> Author: nadav
> Date: Tue Aug 28 07:23:22 2012
> New Revision: 162749
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=162749&view=rev
> Log:
> 
> Make sure that we don't call getZExtValue on values > 64 bits.
> Thanks Benjamin for noticing this.
> 
> 
> Modified:
>    llvm/trunk/lib/Transforms/InstCombine/InstCombineMulDivRem.cpp
>    llvm/trunk/test/Transforms/InstCombine/2012-08-28-udiv_ashl.ll
> 
> Modified: llvm/trunk/lib/Transforms/InstCombine/InstCombineMulDivRem.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/InstCombine/InstCombineMulDivRem.cpp?rev=162749&r1=162748&r2=162749&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Transforms/InstCombine/InstCombineMulDivRem.cpp (original)
> +++ llvm/trunk/lib/Transforms/InstCombine/InstCombineMulDivRem.cpp Tue Aug 28 07:23:22 2012
> @@ -462,11 +462,11 @@
>     }
>   }
> 
> -  // Udiv ((Lshl x, c1) , c2) ->  x / (C1 * 1<<C2);
> -  if (Constant *C = dyn_cast<Constant>(Op1)) {
> +  // Udiv ((Lshl x, C1) , C2) ->  x / (C2 * 1<<C1);
> +  if (ConstantInt *C2 = dyn_cast<ConstantInt>(Op1)) {
>     Value *X = 0, *C1 = 0;
> -    if (match(Op0, m_LShr(m_Value(X), m_Value(C1)))) {
> -      uint64_t NC = cast<ConstantInt>(C)->getZExtValue() *
> +    if (match(Op0, m_LShr(m_Value(X), m_Value(C1))) && C2->getBitWidth() < 65) {
> +      uint64_t NC = cast<ConstantInt>(C2)->getZExtValue() *
>                     (1<< cast<ConstantInt>(C1)->getZExtValue());
>       return BinaryOperator::CreateUDiv(X, ConstantInt::get(I.getType(), NC));
>     }

This is not what I meant, transforms in instcombine should be as general as possible and also deal with big integers. They may be uncommon on x86 and arm but sometimes occur even there due to tricks performed by SROA. Also this code didn't properly guard against shifts with a non-constant amount. I fixed those issues in r162743.

- Ben

> @@ -543,11 +543,11 @@
>                                           ConstantExpr::getNeg(RHS));
>   }
> 
> -  // Sdiv ((Ashl x, c1) , c2) ->  x / (C1 * 1<<C2);
> -  if (Constant *C = dyn_cast<Constant>(Op1)) {
> +  // Sdiv ((Ashl x, C1) , C2) ->  x / (C2 * 1<<C1);
> +  if (ConstantInt *C2 = dyn_cast<ConstantInt>(Op1)) {
>     Value *X = 0, *C1 = 0;
> -    if (match(Op0, m_AShr(m_Value(X), m_Value(C1)))) {
> -      uint64_t NC = cast<ConstantInt>(C)->getZExtValue() *
> +    if (match(Op0, m_AShr(m_Value(X), m_Value(C1))) && C2->getBitWidth() < 65) {
> +      uint64_t NC = cast<ConstantInt>(C2)->getZExtValue() *
>                     (1<< cast<ConstantInt>(C1)->getZExtValue());
>       return BinaryOperator::CreateSDiv(X, ConstantInt::get(I.getType(), NC));
>     }
> 
> Modified: llvm/trunk/test/Transforms/InstCombine/2012-08-28-udiv_ashl.ll
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/InstCombine/2012-08-28-udiv_ashl.ll?rev=162749&r1=162748&r2=162749&view=diff
> ==============================================================================
> --- llvm/trunk/test/Transforms/InstCombine/2012-08-28-udiv_ashl.ll (original)
> +++ llvm/trunk/test/Transforms/InstCombine/2012-08-28-udiv_ashl.ll Tue Aug 28 07:23:22 2012
> @@ -48,3 +48,10 @@
>   %div1 = sdiv i32 %div, 100
>   ret i32 %div1
> }
> +
> +define i80 @no_crash_i80(i80 %x) {
> +entry:
> +  %div = lshr i80 %x, 2
> +  %div1 = udiv i80 %div, 100
> +  ret i80 %div1
> +}
> 
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits