[PATCH] D76050: [LoopPeel] Remove incorrect assertion in countToEliminateCompares

[Max Kazantsev via Phabricator via llvm-commits]

mkazantsev created this revision.
mkazantsev added reviewers: lebedev.ri, fhahn, sanjoy, fedor.sergeev.
Herald added subscribers: llvm-commits, javed.absar, zzheng, hiraditya.
Herald added a project: LLVM.
mkazantsev added a comment.
mkazantsev edited the summary of this revision.
mkazantsev edited the summary of this revision.
mkazantsev edited the summary of this revision.

@lebedev.ri I am not sure what your logic implied here; maybe instead of removal of assert this check should be a part of `if` for profitability reasons.


fedor.sergeev added a comment.

> @lebedev.ri I am not sure what your logic implied here; maybe instead of removal of assert this check should be a part of if for profitability reasons.

In review where this assert was introduced in one of the previous versions of it there was a check instead of assert (see line 261+:    https://reviews.llvm.org/D69617?id=227953)
I believe doing this check makes sense.


This patch removes the incorrect check that if SCEV cannot prove
`isKnownPredicate(A != B)`, then it should be able to prove
`isKnownPredicate(A == B)`. 
Both these fact may be not provable. It is shown in the provided test:

Could not prove: `{-294,+,-2}<%bb1> !=  0`
Asserting: `{-294,+,-2}<%bb1> == 0`

Obviously, this SCEV is not equal to zero, but 0 is in its range so we cannot
also prove that it is not zero.

So this assert is just wrong and must be removed. Another part of the assert is also removed
because it duplicates exit condition of `while` just few lines above.


https://reviews.llvm.org/D76050

Files:
  llvm/lib/Transforms/Utils/LoopUnrollPeel.cpp
  llvm/test/Transforms/LoopUnroll/wrong_assert_in_peeling.ll


Index: llvm/test/Transforms/LoopUnroll/wrong_assert_in_peeling.ll
===================================================================
--- /dev/null
+++ llvm/test/Transforms/LoopUnroll/wrong_assert_in_peeling.ll
@@ -0,0 +1,49 @@
+; RUN: opt -S < %s -loop-unroll | FileCheck %s
+; RUN: opt -S < %s -passes=unroll | FileCheck %s
+target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128-ni:1-p2:32:8:8:32-ni:2"
+target triple = "x86_64-unknown-linux-gnu"
+
+define i32 @test() {
+; CHECK-LABEL: test
+bb:
+  br label %bb1
+
+bb1:                                              ; preds = %bb13, %bb
+  %tmp = phi i32 [ -147, %bb ], [ %tmp14, %bb13 ]
+  br label %bb2
+
+bb2:                                              ; preds = %bb12, %bb1
+  %tmp3 = phi i32 [ undef, %bb1 ], [ %tmp4, %bb12 ]
+  %tmp4 = add nsw i32 %tmp3, %tmp
+  br label %bb5
+
+bb5:                                              ; preds = %bb2
+  %tmp6 = icmp eq i32 undef, 33
+  br i1 %tmp6, label %bb7, label %bb15
+
+bb7:                                              ; preds = %bb5
+  %tmp8 = sub nsw i32 %tmp3, undef
+  %tmp9 = icmp eq i32 %tmp8, 0
+  br i1 %tmp9, label %bb10, label %bb10
+
+bb10:                                             ; preds = %bb7, %bb7
+  %tmp11 = icmp eq i8 undef, 0
+  br i1 %tmp11, label %bb12, label %bb17
+
+bb12:                                             ; preds = %bb10
+  br i1 false, label %bb13, label %bb2
+
+bb13:                                             ; preds = %bb12
+  %tmp14 = add nsw i32 %tmp, -1
+  br label %bb1
+
+bb15:                                             ; preds = %bb5
+  %tmp16 = call i32 (...) @llvm.experimental.deoptimize.i32(i32 17) [ "deopt"() ]
+  ret i32 %tmp16
+
+bb17:                                             ; preds = %bb10
+  %tmp18 = call i32 (...) @llvm.experimental.deoptimize.i32(i32 6) [ "deopt"() ]
+  ret i32 %tmp18
+}
+
+declare i32 @llvm.experimental.deoptimize.i32(...)
Index: llvm/lib/Transforms/Utils/LoopUnrollPeel.cpp
===================================================================
--- llvm/lib/Transforms/Utils/LoopUnrollPeel.cpp
+++ llvm/lib/Transforms/Utils/LoopUnrollPeel.cpp
@@ -263,9 +263,6 @@
     if (ICmpInst::isEquality(Pred) &&
         !SE.isKnownPredicate(ICmpInst::getInversePredicate(Pred), NextIterVal,
                              RightSCEV)) {
-      assert(!SE.isKnownPredicate(Pred, IterVal, RightSCEV) &&
-             SE.isKnownPredicate(Pred, NextIterVal, RightSCEV) &&
-             "Expected Pred to go from known to unknown.");
       if (!CanPeelOneMoreIteration())
         continue; // Need to peel one more iteration, but can't. Give up.
       PeelOneMoreIteration(); // Great!


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D76050.249852.patch
Type: text/x-patch
Size: 2713 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200312/e1d225b4/attachment.bin>