[PATCH] Incorrect loop limit computation in LFTR

Sanjoy Das sanjoy at playingwithpointers.com
Fri Jan 23 01:06:18 PST 2015


Hi atrick, majnemer, hfinkel,

In `LinearFunctionTestReplace`, it seems like if we conclude that it is unsafe to insert a comparison because the induction variable is poison, we don't emit the overflowing addition, but emit the LFTR with an incorrect `IVCount` anyway.  I've added a test case `lftr-bad-ivcount.ll` that demonstrates the bug.

There are a handful of unit tests that need to be fixed with this change (so if you just apply this patch, `make check` will fail), but first I'd like to get a sanity check on whether this is a bug or if I've misunderstood something.

http://reviews.llvm.org/D7137

Files:
  lib/Transforms/Scalar/IndVarSimplify.cpp
  test/Transforms/IndVarSimplify/lftr-bad-ivcount.ll

Index: lib/Transforms/Scalar/IndVarSimplify.cpp
===================================================================
--- lib/Transforms/Scalar/IndVarSimplify.cpp
+++ lib/Transforms/Scalar/IndVarSimplify.cpp
@@ -1689,7 +1689,8 @@
 /// loop to be a canonical != comparison against the incremented loop induction
 /// variable.  This pass is able to rewrite the exit tests of any loop where the
 /// SCEV analysis can determine a loop-invariant trip count of the loop, which
-/// is actually a much broader range than just linear tests.
+/// is actually a much broader range than just linear tests.  Returns nullptr on
+/// failure.
 Value *IndVarSimplify::
 LinearFunctionTestReplace(Loop *L,
                           const SCEV *BackedgeTakenCount,
@@ -1740,16 +1741,17 @@
         SE->getAddExpr(SE->getZeroExtendExpr(IVInit, WideTy),
                        SE->getZeroExtendExpr(BackedgeTakenCount, WideTy)))
       WrappingFlags = ScalarEvolution::clearFlags(WrappingFlags, SCEV::FlagNUW);
-    if (!ScalarEvolution::maskFlags(IncrementedIndvarSCEV->getNoWrapFlags(),
-                                    WrappingFlags)) {
-      // Add one to the "backedge-taken" count to get the trip count.
-      // This addition may overflow, which is valid as long as the comparison is
-      // truncated to BackedgeTakenCount->getType().
-      IVCount =
-          SE->getAddExpr(BackedgeTakenCount,
-                         SE->getConstant(BackedgeTakenCount->getType(), 1));
-      CmpIndVar = IncrementedIndvar;
-    }
+    if (ScalarEvolution::maskFlags(IncrementedIndvarSCEV->getNoWrapFlags(),
+                                   WrappingFlags))
+      return nullptr;
+
+    // Add one to the "backedge-taken" count to get the trip count.  This
+    // addition may overflow, which is valid as long as the comparison is
+    // truncated to BackedgeTakenCount->getType().
+    IVCount =
+      SE->getAddExpr(BackedgeTakenCount,
+                     SE->getConstant(BackedgeTakenCount->getType(), 1));
+    CmpIndVar = IncrementedIndvar;
   }
 
   Value *ExitCnt = genLoopLimit(IndVar, IVCount, L, Rewriter, SE);
Index: test/Transforms/IndVarSimplify/lftr-bad-ivcount.ll
===================================================================
--- /dev/null
+++ test/Transforms/IndVarSimplify/lftr-bad-ivcount.ll
@@ -0,0 +1,21 @@
+; RUN: opt -S -indvars < %s | FileCheck %s
+
+define void @test1(i32 %data_len, i32 %sample) {
+; CHECK-LABEL: @test1(
+entry:
+; CHECK: [[IVCount:[^ ]+]] = sub i32 %data_len, %sample
+  %sub = sub i32 %data_len, %sample
+  %cmp4 = icmp eq i32 %data_len, %sample
+  br i1 %cmp4, label %for.end, label %for.body
+
+for.body:                                         ; preds = %entry, %for.body
+  %indvars.iv = phi i64 [ %indvars.iv.next, %for.body ], [ 0, %entry ]
+  %indvars.iv.next = add nuw i64 %indvars.iv, 1
+  %tr = trunc i64 %indvars.iv.next to i32
+  %cmp = icmp ult i32 %tr, %sub
+; CHECK: %cmp = icmp ult i32 %tr, [[IVCount]]
+  br i1 %cmp, label %for.body, label %for.end
+
+for.end:                                          ; preds = %for.body, %entry
+  ret void
+}

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D7137.18652.patch
Type: text/x-patch
Size: 3110 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150123/179312de/attachment.bin>


More information about the llvm-commits mailing list