[PATCH] D36873: [IRCE] Fix buggy behavior in Clamp
Max Kazantsev via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Fri Aug 18 05:39:59 PDT 2017
mkazantsev created this revision.
Clamp function was too optimistic when choosing signed or unsigned min/max function for calculations.
In fact, `!IsSignedPredicate` guarantees us that `Smallest` and `Greatest` can be compared safely using unsigned
predicates, but we did not check this for `S` which can in theory be negative.
This patch makes Clamp use signed min/max for cases when it fails to prove `S` being non-negative,
and it adds a test where such situation may lead to incorrect conditions calculation.
https://reviews.llvm.org/D36873
Files:
lib/Transforms/Scalar/InductiveRangeCheckElimination.cpp
test/Transforms/IRCE/clamp.ll
Index: test/Transforms/IRCE/clamp.ll
===================================================================
--- /dev/null
+++ test/Transforms/IRCE/clamp.ll
@@ -0,0 +1,61 @@
+; RUN: opt -verify-loop-info -irce-print-changed-loops -irce -S < %s 2>&1 | FileCheck %s
+
+; The test demonstrates that incorrect behavior of Clamp may lead to incorrect
+; calculation of post-loop exit condition.
+
+; CHECK: irce: in function test: constrained Loop at depth 1 containing: %guarded531<header><exiting>,%guarded539<exiting>,%not_zero79<latch><exiting>
+
+define void @test() {
+bci_0:
+ %indvars.iv.next467 = add nuw nsw i64 2, 1
+ %length.i167 = load i32, i32 addrspace(1)* undef, align 8
+ %tmp21 = zext i32 %length.i167 to i64
+ %tmp34 = load atomic i32, i32 addrspace(1)* undef unordered, align 4
+ %tmp35 = add i32 %tmp34, -9581
+ %tmp36 = icmp ugt i32 %length.i167, 1
+ br i1 %tmp36, label %guarded531.lr.ph, label %bci_323
+
+bci_323: ; preds = %guarded539, %guarded531, %not_zero79, %bci_0
+ ret void
+
+guarded531.lr.ph: ; preds = %bci_0
+; CHECK: guarded531.lr.ph:
+; CHECK-NEXT: %length_gep.i146 = getelementptr inbounds i8, i8 addrspace(1)* undef, i64 8
+; CHECK-NEXT: %length_gep_typed.i147 = bitcast i8 addrspace(1)* undef to i32 addrspace(1)*
+; CHECK-NEXT: %tmp43 = icmp ult i64 %indvars.iv.next467, %tmp21
+; CHECK-NEXT: br i1 false, label %guarded531.preheader, label %main.pseudo.exit
+
+ %length_gep.i146 = getelementptr inbounds i8, i8 addrspace(1)* undef, i64 8
+ %length_gep_typed.i147 = bitcast i8 addrspace(1)* undef to i32 addrspace(1)*
+ %tmp43 = icmp ult i64 %indvars.iv.next467, %tmp21
+ br label %guarded531
+
+not_zero79: ; preds = %guarded539
+; CHECK: not_zero79:
+; CHECK: %tmp56 = icmp ult i64 %indvars.iv.next, %tmp21
+; CHECK-NEXT: [[COND:%[^ ]+]] = icmp ult i64 %indvars.iv.next, 1
+; CHECK-NEXT: br i1 [[COND]], label %guarded531, label %main.exit.selector
+
+ %tmp51 = trunc i64 %indvars.iv.next to i32
+ %tmp53 = mul i32 %tmp51, %tmp51
+ %tmp54 = add i32 %tmp53, -9582
+ %tmp55 = add i32 %tmp54, %tmp62
+ %tmp56 = icmp ult i64 %indvars.iv.next, %tmp21
+ br i1 %tmp56, label %guarded531, label %bci_323
+
+guarded531: ; preds = %not_zero79, %guarded531.lr.ph
+ %tmp62 = phi i32 [ 1, %guarded531.lr.ph ], [ %tmp55, %not_zero79 ]
+ %indvars.iv750 = phi i64 [ 1, %guarded531.lr.ph ], [ %indvars.iv.next, %not_zero79 ]
+ %length.i148 = load i32, i32 addrspace(1)* %length_gep_typed.i147, align 8
+ %tmp68 = zext i32 %length.i148 to i64
+ %tmp97 = icmp ult i64 2, %tmp68
+ %or.cond = and i1 %tmp43, %tmp97
+ %tmp99 = icmp ult i64 %indvars.iv750, %tmp21
+ %or.cond1 = and i1 %or.cond, %tmp99
+ br i1 %or.cond1, label %guarded539, label %bci_323
+
+guarded539: ; preds = %guarded531
+ %indvars.iv.next = add nuw nsw i64 %indvars.iv750, 3
+ %tmp107 = icmp ult i64 %indvars.iv.next, 2
+ br i1 %tmp107, label %not_zero79, label %bci_323
+}
Index: lib/Transforms/Scalar/InductiveRangeCheckElimination.cpp
===================================================================
--- lib/Transforms/Scalar/InductiveRangeCheckElimination.cpp
+++ lib/Transforms/Scalar/InductiveRangeCheckElimination.cpp
@@ -1085,7 +1085,8 @@
}
auto Clamp = [this, Smallest, Greatest, IsSignedPredicate](const SCEV *S) {
- return IsSignedPredicate
+ bool MaybeNegativeValues = IsSignedPredicate || !SE.isKnownNonNegative(S);
+ return MaybeNegativeValues
? SE.getSMaxExpr(Smallest, SE.getSMinExpr(Greatest, S))
: SE.getUMaxExpr(Smallest, SE.getUMinExpr(Greatest, S));
};
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D36873.111657.patch
Type: text/x-patch
Size: 3759 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170818/eb4bbf9c/attachment.bin>
More information about the llvm-commits
mailing list