[llvm] r307249 - Revert "Revert "Revert "[IndVars] Canonicalize comparisons between non-negative values and indvars"""

Maxim Kazantsev via llvm-commits llvm-commits at lists.llvm.org
Thu Jul 6 23:33:05 PDT 2017


Hello,

Thanks for your help! I was still unable to reproduce the problem with saturated mul on my side, but apparently have found a bug in code and was able to construct a test where the problem shows up. The fixed version of the patch is on review as https://reviews.llvm.org/D35107 . Could you please be so kind and check whether the problem is still there with this patch? I'd be very grateful for that!

Have a nice day,
Max

From: NAKAMURA Takumi [mailto:geek4civic at gmail.com]
Sent: Thursday, July 6, 2017 7:27 PM
To: Diana Picus <diana.picus at linaro.org>; Maxim Kazantsev <max.kazantsev at azul.com>
Cc: llvm-commits <llvm-commits at lists.llvm.org>
Subject: Re: [llvm] r307249 - Revert "Revert "Revert "[IndVars] Canonicalize comparisons between non-negative values and indvars"""

FYI, I can reproduce it locally. http://bb.pgr.jp/builders/bootstrap-clang-libcxx-lld-i686-linux/builds/84
(It's not i686 but x86-64, due to issues...)

On Thu, Jul 6, 2017 at 8:55 PM Diana Picus via llvm-commits <llvm-commits at lists.llvm.org<mailto:llvm-commits at lists.llvm.org>> wrote:
Hi Max,

Thanks for reverting.

As I told you before, the patch also broke some of the test-suite
apps, not just the SaturatedMultiply test:
See e.g. http://lab.llvm.org:8011/builders/clang-ppc64be-linux-lnt/builds/6568

You should try to reproduce those failures and make sure they're fixed
before trying to commit again. If you don't have access to any
hardware on which these failures can be reproduced, you can send me
the updated patch and I can run a pre-commit for you on ARM.

Thanks,
Diana

On 6 July 2017 at 12:47, Max Kazantsev via llvm-commits
<llvm-commits at lists.llvm.org<mailto:llvm-commits at lists.llvm.org>> wrote:
> Author: mkazantsev
> Date: Thu Jul  6 03:47:13 2017
> New Revision: 307249
>
> URL: http://llvm.org/viewvc/llvm-project?rev=307249&view=rev
> Log:
> Revert "Revert "Revert "[IndVars] Canonicalize comparisons between non-negative values and indvars"""
>
> It appears that the problem is still there. Needs more analysis to understand why
> SaturatedMultiply test fails.
>
> Removed:
>     llvm/trunk/test/Transforms/IndVarSimplify/canonicalize-cmp.ll
> Modified:
>     llvm/trunk/lib/Transforms/Utils/SimplifyIndVar.cpp
>     llvm/trunk/test/Analysis/ScalarEvolution/guards.ll
>     llvm/trunk/test/Transforms/IndVarSimplify/eliminate-comparison.ll
>     llvm/trunk/test/Transforms/IndVarSimplify/widen-loop-comp.ll
>
> Modified: llvm/trunk/lib/Transforms/Utils/SimplifyIndVar.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Utils/SimplifyIndVar.cpp?rev=307249&r1=307248&r2=307249&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Transforms/Utils/SimplifyIndVar.cpp (original)
> +++ llvm/trunk/lib/Transforms/Utils/SimplifyIndVar.cpp Thu Jul  6 03:47:13 2017
> @@ -264,10 +264,6 @@ void SimplifyIndvar::eliminateIVComparis
>      ICmp->setPredicate(InvariantPredicate);
>      ICmp->setOperand(0, NewLHS);
>      ICmp->setOperand(1, NewRHS);
> -  } else if (ICmpInst::isSigned(Pred) &&
> -             SE->isKnownNonNegative(S) && SE->isKnownNonNegative(X)) {
> -    DEBUG(dbgs() << "INDVARS: Turn to unsigned comparison: " << *ICmp << '\n');
> -    ICmp->setPredicate(ICmpInst::getUnsignedPredicate(Pred));
>    } else
>      return;
>
>
> Modified: llvm/trunk/test/Analysis/ScalarEvolution/guards.ll
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Analysis/ScalarEvolution/guards.ll?rev=307249&r1=307248&r2=307249&view=diff
> ==============================================================================
> --- llvm/trunk/test/Analysis/ScalarEvolution/guards.ll (original)
> +++ llvm/trunk/test/Analysis/ScalarEvolution/guards.ll Thu Jul  6 03:47:13 2017
> @@ -19,7 +19,7 @@ entry:
>  loop:
>  ; CHECK: loop:
>  ; CHECK:  call void (i1, ...) @llvm.experimental.guard(i1 true) [ "deopt"() ]
> -; CHECK:  %iv.inc.cmp = icmp ult i32 %iv.inc, %len
> +; CHECK:  %iv.inc.cmp = icmp slt i32 %iv.inc, %len
>  ; CHECK:  call void (i1, ...) @llvm.experimental.guard(i1 %iv.inc.cmp) [ "deopt"() ]
>  ; CHECK: leave:
>
> @@ -41,7 +41,7 @@ leave:
>
>  define void @test_2(i32 %n, i32* %len_buf) {
>  ; CHECK-LABEL: @test_2(
> -; CHECK:  [[LEN_ZEXT:%[^ ]+]] = zext i32 %len to i64
> +; CHECK:  [[LEN_SEXT:%[^ ]+]] = sext i32 %len to i64
>  ; CHECK:  br label %loop
>
>  entry:
> @@ -52,7 +52,7 @@ loop:
>  ; CHECK: loop:
>  ; CHECK:  %indvars.iv = phi i64 [ %indvars.iv.next, %loop ], [ 0, %entry ]
>  ; CHECK:  %indvars.iv.next = add nuw nsw i64 %indvars.iv, 1
> -; CHECK:  %iv.inc.cmp = icmp ult i64 %indvars.iv.next, [[LEN_ZEXT]]
> +; CHECK:  %iv.inc.cmp = icmp slt i64 %indvars.iv.next, [[LEN_SEXT]]
>  ; CHECK:  call void (i1, ...) @llvm.experimental.guard(i1 %iv.inc.cmp) [ "deopt"() ]
>  ; CHECK: leave:
>
>
> Removed: llvm/trunk/test/Transforms/IndVarSimplify/canonicalize-cmp.ll
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/IndVarSimplify/canonicalize-cmp.ll?rev=307248&view=auto
> ==============================================================================
> --- llvm/trunk/test/Transforms/IndVarSimplify/canonicalize-cmp.ll (original)
> +++ llvm/trunk/test/Transforms/IndVarSimplify/canonicalize-cmp.ll (removed)
> @@ -1,52 +0,0 @@
> -; RUN: opt -S -indvars < %s | FileCheck %s
> -
> -; Check that we replace signed comparisons between non-negative values with
> -; unsigned comparisons if we can.
> -
> -target datalayout = "n8:16:32:64"
> -
> -define i32 @test_01(i32 %a, i32 %b, i32* %p) {
> -
> -; CHECK-LABEL: @test_01(
> -; CHECK-NOT:   icmp slt
> -; CHECK:       %cmp1 = icmp ult i32 %iv, 100
> -; CHECK:       %cmp2 = icmp ult i32 %iv, 100
> -; CHECK-NOT:   %cmp3
> -; CHECK:       %exitcond = icmp ne i32 %iv.next, 1000
> -
> -entry:
> -  br label %loop.entry
> -
> -loop.entry:
> -  %iv = phi i32 [ 0, %entry ], [ %iv.next, %loop.be<http://loop.be> ]
> -  %cmp1 = icmp slt i32 %iv, 100
> -  br i1 %cmp1, label %b1, label %b2
> -
> -b1:
> -  store i32 %iv, i32* %p
> -  br label %merge
> -
> -b2:
> -  store i32 %a, i32* %p
> -  br label %merge
> -
> -merge:
> -  %cmp2 = icmp ult i32 %iv, 100
> -  br i1 %cmp2, label %b3, label %b4
> -
> -b3:
> -  store i32 %iv, i32* %p
> -  br label %loop.be<http://loop.be>
> -
> -b4:
> -  store i32 %b, i32* %p
> -  br label %loop.be<http://loop.be>
> -
> -loop.be<http://loop.be>:
> -  %iv.next = add i32 %iv, 1
> -  %cmp3 = icmp slt i32 %iv.next, 1000
> -  br i1 %cmp3, label %loop.entry, label %exit
> -
> -exit:
> -  ret i32 %iv
> -}
>
> Modified: llvm/trunk/test/Transforms/IndVarSimplify/eliminate-comparison.ll
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/IndVarSimplify/eliminate-comparison.ll?rev=307249&r1=307248&r2=307249&view=diff
> ==============================================================================
> --- llvm/trunk/test/Transforms/IndVarSimplify/eliminate-comparison.ll (original)
> +++ llvm/trunk/test/Transforms/IndVarSimplify/eliminate-comparison.ll Thu Jul  6 03:47:13 2017
> @@ -111,7 +111,7 @@ return:
>  ; Indvars should not turn the second loop into an infinite one.
>
>  ; CHECK-LABEL: @func_11(
> -; CHECK: %tmp5 = icmp ult i32 %__key6.0, 10
> +; CHECK: %tmp5 = icmp slt i32 %__key6.0, 10
>  ; CHECK-NOT: br i1 true, label %noassert68, label %unrolledend
>
>  define i32 @func_11() nounwind uwtable {
> @@ -163,7 +163,7 @@ declare void @llvm.trap() noreturn nounw
>
>  ; In this case the second loop only has a single iteration, fold the header away
>  ; CHECK-LABEL: @func_12(
> -; CHECK: %tmp5 = icmp ult i32 %__key6.0, 10
> +; CHECK: %tmp5 = icmp slt i32 %__key6.0, 10
>  ; CHECK: br i1 true, label %noassert68, label %unrolledend
>  define i32 @func_12() nounwind uwtable {
>  entry:
>
> Modified: llvm/trunk/test/Transforms/IndVarSimplify/widen-loop-comp.ll
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/IndVarSimplify/widen-loop-comp.ll?rev=307249&r1=307248&r2=307249&view=diff
> ==============================================================================
> --- llvm/trunk/test/Transforms/IndVarSimplify/widen-loop-comp.ll (original)
> +++ llvm/trunk/test/Transforms/IndVarSimplify/widen-loop-comp.ll Thu Jul  6 03:47:13 2017
> @@ -64,7 +64,7 @@ for.end:
>  ; CHECK-LABEL: @test2
>  ; CHECK: for.body4.us<http://for.body4.us>
>  ; CHECK: %indvars.iv.next = add nuw nsw i64 %indvars.iv, 1
> -; CHECK: %cmp2.us<http://cmp2.us> = icmp ult i64
> +; CHECK: %cmp2.us<http://cmp2.us> = icmp slt i64
>  ; CHECK-NOT: %2 = trunc i64 %indvars.iv.next to i32
>  ; CHECK-NOT: %cmp2.us<http://cmp2.us> = icmp slt i32
>
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org<mailto:llvm-commits at lists.llvm.org>
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
_______________________________________________
llvm-commits mailing list
llvm-commits at lists.llvm.org<mailto:llvm-commits at lists.llvm.org>
http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170707/3b8a5a89/attachment.html>


More information about the llvm-commits mailing list