[llvm] r366829 - [IndVars] Fix a subtle bug in optimizeLoopExits

[Philip Reames via llvm-commits]

Author: reames
Date: Tue Jul 23 10:45:11 2019
New Revision: 366829

URL: http://llvm.org/viewvc/llvm-project?rev=366829&view=rev
Log:
[IndVars] Fix a subtle bug in optimizeLoopExits

The original code failed to account for the fact that one exit can have a pointer exit count without all of them having pointer exit counts.  This could cause two separate bugs:
1) We might exit the loop early, and leave optimizations undone.  This is what triggered the assertion failure in the reported test case.
2) We might optimize one exit, then exit without indicating a change.  This could result in an analysis invalidaton bug if no other transform is done by the rest of indvars.

Note that the pointer exit counts are a really fragile concept.  They show up only when we have a pointer IV w/o a datalayout to provide their size.  It's really questionable to me whether the complexity implied is worth it.


Added:
    llvm/trunk/test/Transforms/IndVarSimplify/eliminate-exit-no-dl.ll
Modified:
    llvm/trunk/lib/Transforms/Scalar/IndVarSimplify.cpp

Modified: llvm/trunk/lib/Transforms/Scalar/IndVarSimplify.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/IndVarSimplify.cpp?rev=366829&r1=366828&r2=366829&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/Scalar/IndVarSimplify.cpp (original)
+++ llvm/trunk/lib/Transforms/Scalar/IndVarSimplify.cpp Tue Jul 23 10:45:11 2019
@@ -2688,10 +2688,12 @@ bool IndVarSimplify::optimizeLoopExits(L
       continue;
     }
 
-    // If we end up with a pointer exit count, bail.
+    // If we end up with a pointer exit count, bail.  Note that we can end up
+    // with a pointer exit count for one exiting block, and not for another in
+    // the same loop.
     if (!ExitCount->getType()->isIntegerTy() ||
         !MaxExitCount->getType()->isIntegerTy())
-      return false;
+      continue;
     
     Type *WiderType =
       SE->getWiderType(MaxExitCount->getType(), ExitCount->getType());

Added: llvm/trunk/test/Transforms/IndVarSimplify/eliminate-exit-no-dl.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/IndVarSimplify/eliminate-exit-no-dl.ll?rev=366829&view=auto
==============================================================================
--- llvm/trunk/test/Transforms/IndVarSimplify/eliminate-exit-no-dl.ll (added)
+++ llvm/trunk/test/Transforms/IndVarSimplify/eliminate-exit-no-dl.ll Tue Jul 23 10:45:11 2019
@@ -0,0 +1,45 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
+; RUN: opt -indvars -S < %s | FileCheck %s
+
+; Check the case where one exit has a pointer EC, and the other doesn't.
+; Note that this test case is really really fragile.  Removing any
+; instruction in the below causes the result to differ.  Note that the lack
+; of a data layout (with pointer size info) is critical to getting a pointer
+; EC returned by SCEV.
+
+ at global = external global [0 x i8], align 1
+
+define void @foo() {
+; CHECK-LABEL: @foo(
+; CHECK-NEXT:  bb:
+; CHECK-NEXT:    br label [[BB3:%.*]]
+; CHECK:       bb3:
+; CHECK-NEXT:    [[TMP:%.*]] = phi i8* [ [[TMP4:%.*]], [[BB7:%.*]] ], [ getelementptr inbounds ([0 x i8], [0 x i8]* @global, i64 0, i64 2), [[BB:%.*]] ]
+; CHECK-NEXT:    [[TMP4]] = getelementptr inbounds i8, i8* [[TMP]], i64 -1
+; CHECK-NEXT:    [[TMP6:%.*]] = load i8, i8* [[TMP4]], align 1
+; CHECK-NEXT:    [[TMP5:%.*]] = icmp ugt i8* [[TMP4]], getelementptr inbounds ([0 x i8], [0 x i8]* @global, i64 0, i64 500)
+; CHECK-NEXT:    br i1 [[TMP5]], label [[BB7]], label [[BB11:%.*]]
+; CHECK:       bb7:
+; CHECK-NEXT:    [[TMP8:%.*]] = zext i8 [[TMP6]] to i64
+; CHECK-NEXT:    br i1 true, label [[BB11]], label [[BB3]]
+; CHECK:       bb11:
+; CHECK-NEXT:    ret void
+;
+bb:
+  br label %bb3
+
+bb3:                                              ; preds = %bb7, %bb2
+  %tmp = phi i8* [ %tmp4, %bb7 ], [ getelementptr inbounds ([0 x i8], [0 x i8]* @global, i64 0, i64 2), %bb ]
+  %tmp4 = getelementptr inbounds i8, i8* %tmp, i64 -1
+  %tmp6 = load i8, i8* %tmp4, align 1
+  %tmp5 = icmp ugt i8* %tmp4, getelementptr inbounds ([0 x i8], [0 x i8]* @global, i64 0, i64 500)
+  br i1 %tmp5, label %bb7, label %bb11
+
+bb7:                                              ; preds = %bb3
+  %tmp8 = zext i8 %tmp6 to i64
+  %tmp10 = icmp eq i16 0, 0
+  br i1 %tmp10, label %bb11, label %bb3
+
+bb11:                                             ; preds = %bb7, %bb3
+  ret void
+}