[llvm] r204912 - InstCombine: merge constants in both operands of icmp.

Benjamin Kramer benny.kra at gmail.com
Thu Mar 27 12:30:44 PDT 2014


On 27.03.2014, at 20:23, Erik Verbruggen <erikjv at me.com> wrote:

> Actually, I'm wondering if it's better to revert my patch (and yours). The "nightly" test-suite pointed out a regression. If you have a tight for loop, like: 
>    for (i = 0; i <= 39; i++)
> 	doSomethingHere;
> 
> then before the patch it would be:
>    %indvars.iv.next = add nuw nsw i64 %indvars.iv, 1
>    %exitcond = icmp eq i64 %indvars.iv.next, 40
> 
> while with the patch it's:
>    %indvars.iv.next = add nuw nsw i64 %indvars.iv, 1
>    %exitcond = icmp eq i64 %indvars.iv, 39
> 
> That keeps the "old" %indvars.iv alive. This causes some performance tests to fail, like MultiSource/Benchmarks/MiBench/telecomm-gsm. 
> 
> I Cc-ed d0k because he did the review, so what do you guys think?

A simple fix would be to check if the add has just one use. I was hoping that this kind of workaround wouldn't be necessary anymore but it looks like we still need it in this case :|

- Ben

> 
> -- Erik.
> 
> 
> On 27 Mar 2014, at 18:56, Reid Kleckner <rnk at google.com> wrote:
> 
>> This transform was broken for unsigned comparisons.  I disabled it in r204948, which should fix the problem.  See the commit message for the counterexample.
>> 
>> 
>> On Thu, Mar 27, 2014 at 4:16 AM, Erik Verbruggen <erikjv at me.com> wrote:
>> Author: erikjv
>> Date: Thu Mar 27 06:16:05 2014
>> New Revision: 204912
>> 
>> URL: http://llvm.org/viewvc/llvm-project?rev=204912&view=rev
>> Log:
>> InstCombine: merge constants in both operands of icmp.
>> 
>> Transform:
>>     icmp X+Cst2, Cst
>> into:
>>     icmp X, Cst-Cst2
>> when Cst-Cst2 does not overflow, and the add has nsw.
>> 
>> Modified:
>>     llvm/trunk/lib/Target/README.txt
>>     llvm/trunk/lib/Transforms/InstCombine/InstCombineCompares.cpp
>>     llvm/trunk/test/Transforms/InstCombine/icmp.ll
>> 
>> Modified: llvm/trunk/lib/Target/README.txt
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/README.txt?rev=204912&r1=204911&r2=204912&view=diff
>> ==============================================================================
>> --- llvm/trunk/lib/Target/README.txt (original)
>> +++ llvm/trunk/lib/Target/README.txt Thu Mar 27 06:16:05 2014
>> @@ -930,18 +930,6 @@ optimized with "clang -emit-llvm-bc | op
>> 
>>  //===---------------------------------------------------------------------===//
>> 
>> -int g(int x) { return (x - 10) < 0; }
>> -Should combine to "x <= 9" (the sub has nsw).  Currently not
>> -optimized with "clang -emit-llvm-bc | opt -std-compile-opts".
>> -
>> -//===---------------------------------------------------------------------===//
>> -
>> -int g(int x) { return (x + 10) < 0; }
>> -Should combine to "x < -10" (the add has nsw).  Currently not
>> -optimized with "clang -emit-llvm-bc | opt -std-compile-opts".
>> -
>> -//===---------------------------------------------------------------------===//
>> -
>>  int f(int i, int j) { return i < j + 1; }
>>  int g(int i, int j) { return j > i - 1; }
>>  Should combine to "i <= j" (the add/sub has nsw).  Currently not
>> 
>> Modified: llvm/trunk/lib/Transforms/InstCombine/InstCombineCompares.cpp
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/InstCombine/InstCombineCompares.cpp?rev=204912&r1=204911&r2=204912&view=diff
>> ==============================================================================
>> --- llvm/trunk/lib/Transforms/InstCombine/InstCombineCompares.cpp (original)
>> +++ llvm/trunk/lib/Transforms/InstCombine/InstCombineCompares.cpp Thu Mar 27 06:16:05 2014
>> @@ -3008,6 +3008,19 @@ Instruction *InstCombiner::visitICmpInst
>>      // icmp X, X+Cst
>>      if (match(Op1, m_Add(m_Value(X), m_ConstantInt(Cst))) && Op0 == X)
>>        return FoldICmpAddOpCst(I, X, Cst, I.getSwappedPredicate());
>> +
>> +    ConstantInt *Cst2;
>> +    if (match(Op1, m_ConstantInt(Cst)) &&
>> +        match(Op0, m_Add(m_Value(X), m_ConstantInt(Cst2))) &&
>> +        cast<BinaryOperator>(Op0)->hasNoSignedWrap()) {
>> +      // icmp X+Cst2, Cst --> icmp X, Cst-Cst2
>> +      // iff Cst-Cst2 does not overflow
>> +      bool Overflow;
>> +      APInt NewCst = Cst->getValue().ssub_ov(Cst2->getValue(), Overflow);
>> +      if (!Overflow)
>> +        return new ICmpInst(I.getPredicate(), X,
>> +                            ConstantInt::get(Cst->getType(), NewCst));
>> +    }
>>    }
>>    return Changed ? &I : 0;
>>  }
>> 
>> Modified: llvm/trunk/test/Transforms/InstCombine/icmp.ll
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/InstCombine/icmp.ll?rev=204912&r1=204911&r2=204912&view=diff
>> ==============================================================================
>> --- llvm/trunk/test/Transforms/InstCombine/icmp.ll (original)
>> +++ llvm/trunk/test/Transforms/InstCombine/icmp.ll Thu Mar 27 06:16:05 2014
>> @@ -1356,3 +1356,56 @@ define i1 @icmp_ashr_ashr_ne(i32 %a, i32
>>   %z = icmp ne i32 %x, %y
>>   ret i1 %z
>>  }
>> +
>> +; CHECK-LABEL: icmp_add_const_const1
>> +; CHECK: %cmp = icmp slt i32 %x, -10
>> +; CHECK-NOT: %add = add nsw i32 %x, 10
>> +define i32 @icmp_add_const_const1(i32 %x) nounwind ssp uwtable {
>> +entry:
>> +  %add = add nsw i32 %x, 10
>> +  %cmp = icmp slt i32 %add, 0
>> +  %conv = zext i1 %cmp to i32
>> +  ret i32 %conv
>> +}
>> +
>> +; CHECK-LABEL: icmp_add_const_const2
>> +; CHECK: %cmp = icmp slt i32 %x, -10
>> +; CHECK-NOT: %add = add nsw i32 %x, 10
>> +define i32 @icmp_add_const_const2(i32 %x) nounwind ssp uwtable {
>> +entry:
>> +  %add = add nsw i32 10, %x
>> +  %cmp = icmp sgt i32 0, %add
>> +  %conv = zext i1 %cmp to i32
>> +  ret i32 %conv
>> +}
>> +
>> +; CHECK-LABEL: icmp_add_const_const3
>> +; CHECK: %cmp = icmp slt i32 %x, 20
>> +; CHECK-NOT: %sub = add nsw i32 %x, -10
>> +define i32 @icmp_add_const_const3(i32 %x) nounwind ssp uwtable {
>> +entry:
>> +  %add = add nsw i32 -10, %x
>> +  %cmp = icmp sgt i32 10, %add
>> +  %conv = zext i1 %cmp to i32
>> +  ret i32 %conv
>> +}
>> +
>> +; CHECK-LABEL: icmp_add_const_intmin
>> +; CHECK: %cmp = icmp ne i32 %x, 2147483638
>> +define i32 @icmp_add_const_intmin(i32 %x) nounwind ssp uwtable {
>> +entry:
>> +  %add = add nsw i32 %x, 10
>> +  %cmp = icmp sgt i32 %add, -2147483648
>> +  %conv = zext i1 %cmp to i32
>> +  ret i32 %conv
>> +}
>> +
>> +; CHECK-LABEL: icmp_add_const_intmax
>> +; CHECK: %cmp = icmp ne i32 %x, 2147483637
>> +define i32 @icmp_add_const_intmax(i32 %x) nounwind ssp uwtable {
>> +entry:
>> +  %add = add nsw i32 %x, 10
>> +  %cmp = icmp slt i32 %add, 2147483647
>> +  %conv = zext i1 %cmp to i32
>> +  ret i32 %conv
>> +}
>> 
>> 
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>> 
> 





More information about the llvm-commits mailing list