[llvm] r213231 - Move ashr optimization from InstCombineShift to InstSimplify.

[Hal Finkel]

----- Original Message -----
> From: "Suyog Sarda" <suyog.sarda at samsung.com>
> To: llvm-commits at cs.uiuc.edu
> Sent: Thursday, July 17, 2014 1:28:15 AM
> Subject: [llvm] r213231 - Move ashr optimization from InstCombineShift to	InstSimplify.
> 
> Author: suyog
> Date: Thu Jul 17 01:28:15 2014
> New Revision: 213231
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=213231&view=rev
> Log:
> Move ashr optimization from InstCombineShift to InstSimplify.
> Refactor code, no functionality change, test case moved from
> instcombine to instsimplify.
> 
> Differential Revision: http://reviews.llvm.org/D4102
>  
> 
> Added:
>     llvm/trunk/test/Transforms/InstSimplify/ashr-nop.ll
> Removed:
>     llvm/trunk/test/Transforms/InstCombine/ashr-nop.ll
> Modified:
>     llvm/trunk/lib/Analysis/InstructionSimplify.cpp
>     llvm/trunk/lib/Transforms/InstCombine/InstCombineShifts.cpp
> 
> Modified: llvm/trunk/lib/Analysis/InstructionSimplify.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Analysis/InstructionSimplify.cpp?rev=213231&r1=213230&r2=213231&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Analysis/InstructionSimplify.cpp (original)
> +++ llvm/trunk/lib/Analysis/InstructionSimplify.cpp Thu Jul 17
> 01:28:15 2014
> @@ -1346,6 +1346,11 @@ static Value *SimplifyAShrInst(Value *Op
>        cast<OverflowingBinaryOperator>(Op0)->hasNoSignedWrap())
>      return X;
>  
> +  // Arithmetic shifting an all-sign-bit value is a no-op.
> +  unsigned NumSignBits = ComputeNumSignBits(Op0);

This is not quite right. When this code was in InstCombine, you were calling the version of ComputeNumSignBits in InstCombine.h that automatically added the DataLayout* before calling into ValueTracking. Here, you're calling into ValueTracking directly without passing in the DataLayout*. I believe this should be:

  ComputeNumSignBits(Op0, Q.DL);

Please fix this.

Thanks,
Hal

> +  if (NumSignBits == Op0->getType()->getScalarSizeInBits())
> +    return Op0;
> +
>    return nullptr;
>  }
>  
> 
> Modified: llvm/trunk/lib/Transforms/InstCombine/InstCombineShifts.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/InstCombine/InstCombineShifts.cpp?rev=213231&r1=213230&r2=213231&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Transforms/InstCombine/InstCombineShifts.cpp
> (original)
> +++ llvm/trunk/lib/Transforms/InstCombine/InstCombineShifts.cpp Thu
> Jul 17 01:28:15 2014
> @@ -815,10 +815,5 @@ Instruction *InstCombiner::visitAShr(Bin
>                          APInt::getSignBit(I.getType()->getScalarSizeInBits())))
>      return BinaryOperator::CreateLShr(Op0, Op1);
>  
> -  // Arithmetic shifting an all-sign-bit value is a no-op.
> -  unsigned NumSignBits = ComputeNumSignBits(Op0);
> -  if (NumSignBits == Op0->getType()->getScalarSizeInBits())
> -    return ReplaceInstUsesWith(I, Op0);
> -
>    return nullptr;
>  }
> 
> Removed: llvm/trunk/test/Transforms/InstCombine/ashr-nop.ll
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/InstCombine/ashr-nop.ll?rev=213230&view=auto
> ==============================================================================
> --- llvm/trunk/test/Transforms/InstCombine/ashr-nop.ll (original)
> +++ llvm/trunk/test/Transforms/InstCombine/ashr-nop.ll (removed)
> @@ -1,8 +0,0 @@
> -; RUN: opt < %s -instcombine -S | not grep ashr
> -
> -define i32 @foo(i32 %x) {
> -  %o = and i32 %x, 1
> -  %n = add i32 %o, -1
> -  %t = ashr i32 %n, 17
> -  ret i32 %t
> -}
> 
> Added: llvm/trunk/test/Transforms/InstSimplify/ashr-nop.ll
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/InstSimplify/ashr-nop.ll?rev=213231&view=auto
> ==============================================================================
> --- llvm/trunk/test/Transforms/InstSimplify/ashr-nop.ll (added)
> +++ llvm/trunk/test/Transforms/InstSimplify/ashr-nop.ll Thu Jul 17
> 01:28:15 2014
> @@ -0,0 +1,10 @@
> +; RUN: opt < %s -instsimplify -S | FileCheck %s
> +
> +; CHECK-LABEL: @foo
> +; CHECK-NOT: ashr
> +define i32 @foo(i32 %x) {
> + %o = and i32 %x, 1
> + %n = add i32 %o, -1
> + %t = ashr i32 %n, 17
> + ret i32 %t
> +}
> 
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
> 

-- 
Hal Finkel
Assistant Computational Scientist
Leadership Computing Facility
Argonne National Laboratory