[PATCH] [InstCombine/InstSimplify] Move ashr optimization from Instcombine to Instsimplify

[Rahul Jain]

Hi Nick,

Thanks for your inputs.

I think this test case is in accordance with the new simplification we just
taught it.

Assuming just 8 bits for explaination:

Anding or with 1 will give us:
Either 00000000    or    00000001     for the variable o  depending on the
LSB of x.
Adding -1 i.e 11111111 will give us:
Either 11111111    or    00000000 respectively   for the variable n

this is exactly what we need in n, all zeros or all ones!


I tried a simpler test case like:

define i32 @test1(i32 %x) {
  %n = or i32 %x, -1
  %t = ashr i32 %n, 17
  ret i32 %t
}

But in this case the instruction t here will get constant folded before
only.

What do you say??

Thanks,
Rahul







On Thu, Jun 19, 2014 at 9:10 AM, Nick Lewycky <nlewycky at google.com> wrote:

> On 18 June 2014 06:19, suyog <suyog.sarda at samsung.com> wrote:
>
>>
>> Hi Nick,
>>
>> Added supporting testcase.
>>
>> Please if you could commit it for me if it looks
>> good to you.
>>
>
> Not yet, sorry. It needs a test that ensures that "opt -instsimplify" *is*
> doing the new simplification we just taught it. You can keep the test you
> wrote too if you like. Also, using "grep" is old-style, please use
> FileCheck. For example:
>
> define i32 @test1(i32 %x) {
> ; CHECK-LABEL: @test1
> ; CHECK-NOT: ashr
>   %o = and i32 %x, 1
>   %n = add i32 %o, -1
>   %t = ashr i32 %n, 17
>   ret i32 %t
> }
>
> Nick
>
> Thanks,
>> Suyog
>>
>> http://reviews.llvm.org/D4102
>>
>> Files:
>>   lib/Analysis/InstructionSimplify.cpp
>>   lib/Transforms/InstCombine/InstCombineShifts.cpp
>>   test/Transforms/InstSimplify/ashr-nop.ll
>>
>> Index: lib/Analysis/InstructionSimplify.cpp
>> ===================================================================
>> --- lib/Analysis/InstructionSimplify.cpp
>> +++ lib/Analysis/InstructionSimplify.cpp
>> @@ -1436,6 +1436,11 @@
>>        cast<OverflowingBinaryOperator>(Op0)->hasNoSignedWrap())
>>      return X;
>>
>> +  // Arithmetic shifting an all-sign-bit value is a no-op.
>> +  unsigned NumSignBits = ComputeNumSignBits(Op0);
>> +  if (NumSignBits == Op0->getType()->getScalarSizeInBits())
>> +    return Op0;
>> +
>>    return nullptr;
>>  }
>>
>> Index: lib/Transforms/InstCombine/InstCombineShifts.cpp
>> ===================================================================
>> --- lib/Transforms/InstCombine/InstCombineShifts.cpp
>> +++ lib/Transforms/InstCombine/InstCombineShifts.cpp
>> @@ -820,10 +820,5 @@
>>
>>  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;
>>  }
>> Index: test/Transforms/InstSimplify/ashr-nop.ll
>> ===================================================================
>> --- test/Transforms/InstSimplify/ashr-nop.ll
>> +++ test/Transforms/InstSimplify/ashr-nop.ll
>> @@ -0,0 +1,8 @@
>> +; RUN: opt < %s -instsimplify -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
>> +}
>>
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140623/4012e13f/attachment.html>