[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