From a22282be54b309ce7ab0e6bf8595893384971646 Mon Sep 17 00:00:00 2001 From: Sanjay Patel Date: Tue, 29 Oct 2019 10:39:59 -0400 Subject: [PATCH] [InstCombine] make icmp vector canonicalization safe for constant with undef elements This is a fix for: https://bugs.llvm.org/show_bug.cgi?id=43730 ...and as shown there, we have existing test cases that show potential miscompiles. We could just bail out for vector constants that contain any undef elements, or we can do as shown here: allow the transform, but replace the undefs with a safe value. For most of the tests shown, this results in a full splat constant (no undefs) which is probably a win for further IR analysis because we conservatively don't match undefs in most cases. Codegen can probably recover these kinds of undef lanes via demanded elements analysis if that's profitable. Differential Revision: https://reviews.llvm.org/D69519 --- llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp | 12 ++++++++++++ ...nicalize-constant-low-bit-mask-and-icmp-eq-to-icmp-ule.ll | 2 +- ...icalize-constant-low-bit-mask-and-icmp-sge-to-icmp-sle.ll | 2 +- ...icalize-constant-low-bit-mask-and-icmp-sle-to-icmp-sle.ll | 2 +- ...icalize-constant-low-bit-mask-and-icmp-uge-to-icmp-ule.ll | 2 +- ...icalize-constant-low-bit-mask-and-icmp-ule-to-icmp-ule.ll | 2 +- llvm/test/Transforms/InstCombine/icmp-vec.ll | 4 ++-- .../InstCombine/reuse-constant-from-select-in-icmp.ll | 4 ++-- 8 files changed, 21 insertions(+), 9 deletions(-) diff --git a/llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp b/llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp index a9f64fe..5fb3ec8 100644 --- a/llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp +++ b/llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp @@ -5167,6 +5167,7 @@ llvm::getFlippedStrictnessPredicateAndConstant(CmpInst::Predicate Pred, return WillIncrement ? !C->isMaxValue(IsSigned) : !C->isMinValue(IsSigned); }; + Constant *SafeReplacementConstant = nullptr; if (auto *CI = dyn_cast(C)) { // Bail out if the constant can't be safely incremented/decremented. if (!ConstantIsOk(CI)) @@ -5186,12 +5187,23 @@ llvm::getFlippedStrictnessPredicateAndConstant(CmpInst::Predicate Pred, auto *CI = dyn_cast(Elt); if (!CI || !ConstantIsOk(CI)) return llvm::None; + + if (!SafeReplacementConstant) + SafeReplacementConstant = CI; } } else { // ConstantExpr? return llvm::None; } + // It may not be safe to change a compare predicate in the presence of + // undefined elements, so replace those elements with the first safe constant + // that we found. + if (C->containsUndefElement()) { + assert(SafeReplacementConstant && "Replacement constant not set"); + C = Constant::replaceUndefsWith(C, SafeReplacementConstant); + } + CmpInst::Predicate NewPred = CmpInst::getFlippedStrictnessPredicate(Pred); // Increment or decrement the constant. diff --git a/llvm/test/Transforms/InstCombine/canonicalize-constant-low-bit-mask-and-icmp-eq-to-icmp-ule.ll b/llvm/test/Transforms/InstCombine/canonicalize-constant-low-bit-mask-and-icmp-eq-to-icmp-ule.ll index bb304d7..6a3564e 100644 --- a/llvm/test/Transforms/InstCombine/canonicalize-constant-low-bit-mask-and-icmp-eq-to-icmp-ule.ll +++ b/llvm/test/Transforms/InstCombine/canonicalize-constant-low-bit-mask-and-icmp-eq-to-icmp-ule.ll @@ -82,7 +82,7 @@ define <2 x i1> @p2_vec_nonsplat_edgecase1(<2 x i8> %x) { define <3 x i1> @p3_vec_splat_undef(<3 x i8> %x) { ; CHECK-LABEL: @p3_vec_splat_undef( -; CHECK-NEXT: [[TMP1:%.*]] = icmp ult <3 x i8> [[X:%.*]], +; CHECK-NEXT: [[TMP1:%.*]] = icmp ult <3 x i8> [[X:%.*]], ; CHECK-NEXT: ret <3 x i1> [[TMP1]] ; %tmp0 = and <3 x i8> %x, diff --git a/llvm/test/Transforms/InstCombine/canonicalize-constant-low-bit-mask-and-icmp-sge-to-icmp-sle.ll b/llvm/test/Transforms/InstCombine/canonicalize-constant-low-bit-mask-and-icmp-sge-to-icmp-sle.ll index 3ed42e4..3bfb48f 100644 --- a/llvm/test/Transforms/InstCombine/canonicalize-constant-low-bit-mask-and-icmp-sge-to-icmp-sle.ll +++ b/llvm/test/Transforms/InstCombine/canonicalize-constant-low-bit-mask-and-icmp-sge-to-icmp-sle.ll @@ -61,7 +61,7 @@ define <2 x i1> @p2_vec_nonsplat_edgecase(<2 x i8> %x) { define <3 x i1> @p3_vec_splat_undef(<3 x i8> %x) { ; CHECK-LABEL: @p3_vec_splat_undef( -; CHECK-NEXT: [[TMP1:%.*]] = icmp slt <3 x i8> [[X:%.*]], +; CHECK-NEXT: [[TMP1:%.*]] = icmp slt <3 x i8> [[X:%.*]], ; CHECK-NEXT: ret <3 x i1> [[TMP1]] ; %tmp0 = and <3 x i8> %x, diff --git a/llvm/test/Transforms/InstCombine/canonicalize-constant-low-bit-mask-and-icmp-sle-to-icmp-sle.ll b/llvm/test/Transforms/InstCombine/canonicalize-constant-low-bit-mask-and-icmp-sle-to-icmp-sle.ll index 64be961..eec8bdc 100644 --- a/llvm/test/Transforms/InstCombine/canonicalize-constant-low-bit-mask-and-icmp-sle-to-icmp-sle.ll +++ b/llvm/test/Transforms/InstCombine/canonicalize-constant-low-bit-mask-and-icmp-sle-to-icmp-sle.ll @@ -76,7 +76,7 @@ define <2 x i1> @p2_vec_nonsplat_edgecase() { define <3 x i1> @p3_vec_splat_undef() { ; CHECK-LABEL: @p3_vec_splat_undef( ; CHECK-NEXT: [[X:%.*]] = call <3 x i8> @gen3x8() -; CHECK-NEXT: [[TMP1:%.*]] = icmp slt <3 x i8> [[X]], +; CHECK-NEXT: [[TMP1:%.*]] = icmp slt <3 x i8> [[X]], ; CHECK-NEXT: ret <3 x i1> [[TMP1]] ; %x = call <3 x i8> @gen3x8() diff --git a/llvm/test/Transforms/InstCombine/canonicalize-constant-low-bit-mask-and-icmp-uge-to-icmp-ule.ll b/llvm/test/Transforms/InstCombine/canonicalize-constant-low-bit-mask-and-icmp-uge-to-icmp-ule.ll index 18325ed..1ae84cf 100644 --- a/llvm/test/Transforms/InstCombine/canonicalize-constant-low-bit-mask-and-icmp-uge-to-icmp-ule.ll +++ b/llvm/test/Transforms/InstCombine/canonicalize-constant-low-bit-mask-and-icmp-uge-to-icmp-ule.ll @@ -82,7 +82,7 @@ define <2 x i1> @p2_vec_nonsplat_edgecase1(<2 x i8> %x) { define <3 x i1> @p3_vec_splat_undef(<3 x i8> %x) { ; CHECK-LABEL: @p3_vec_splat_undef( -; CHECK-NEXT: [[TMP1:%.*]] = icmp ult <3 x i8> [[X:%.*]], +; CHECK-NEXT: [[TMP1:%.*]] = icmp ult <3 x i8> [[X:%.*]], ; CHECK-NEXT: ret <3 x i1> [[TMP1]] ; %tmp0 = and <3 x i8> %x, diff --git a/llvm/test/Transforms/InstCombine/canonicalize-constant-low-bit-mask-and-icmp-ule-to-icmp-ule.ll b/llvm/test/Transforms/InstCombine/canonicalize-constant-low-bit-mask-and-icmp-ule-to-icmp-ule.ll index 6f26720..2d2bfce 100644 --- a/llvm/test/Transforms/InstCombine/canonicalize-constant-low-bit-mask-and-icmp-ule-to-icmp-ule.ll +++ b/llvm/test/Transforms/InstCombine/canonicalize-constant-low-bit-mask-and-icmp-ule-to-icmp-ule.ll @@ -99,7 +99,7 @@ define <2 x i1> @p2_vec_nonsplat_edgecase1() { define <3 x i1> @p3_vec_splat_undef() { ; CHECK-LABEL: @p3_vec_splat_undef( ; CHECK-NEXT: [[X:%.*]] = call <3 x i8> @gen3x8() -; CHECK-NEXT: [[TMP1:%.*]] = icmp ult <3 x i8> [[X]], +; CHECK-NEXT: [[TMP1:%.*]] = icmp ult <3 x i8> [[X]], ; CHECK-NEXT: ret <3 x i1> [[TMP1]] ; %x = call <3 x i8> @gen3x8() diff --git a/llvm/test/Transforms/InstCombine/icmp-vec.ll b/llvm/test/Transforms/InstCombine/icmp-vec.ll index da80450..8f4cfdb 100644 --- a/llvm/test/Transforms/InstCombine/icmp-vec.ll +++ b/llvm/test/Transforms/InstCombine/icmp-vec.ll @@ -181,7 +181,7 @@ define <2 x i1> @PR27756_1(<2 x i8> %a) { define <3 x i1> @PR27756_2(<3 x i8> %a) { ; CHECK-LABEL: @PR27756_2( -; CHECK-NEXT: [[CMP:%.*]] = icmp slt <3 x i8> [[A:%.*]], +; CHECK-NEXT: [[CMP:%.*]] = icmp slt <3 x i8> [[A:%.*]], ; CHECK-NEXT: ret <3 x i1> [[CMP]] ; %cmp = icmp sle <3 x i8> %a, @@ -190,7 +190,7 @@ define <3 x i1> @PR27756_2(<3 x i8> %a) { define <3 x i1> @PR27756_3(<3 x i8> %a) { ; CHECK-LABEL: @PR27756_3( -; CHECK-NEXT: [[CMP:%.*]] = icmp sgt <3 x i8> [[A:%.*]], +; CHECK-NEXT: [[CMP:%.*]] = icmp sgt <3 x i8> [[A:%.*]], ; CHECK-NEXT: ret <3 x i1> [[CMP]] ; %cmp = icmp sge <3 x i8> %a, diff --git a/llvm/test/Transforms/InstCombine/reuse-constant-from-select-in-icmp.ll b/llvm/test/Transforms/InstCombine/reuse-constant-from-select-in-icmp.ll index 057ae86..6eb2146 100644 --- a/llvm/test/Transforms/InstCombine/reuse-constant-from-select-in-icmp.ll +++ b/llvm/test/Transforms/InstCombine/reuse-constant-from-select-in-icmp.ll @@ -106,7 +106,7 @@ define <2 x i32> @p7_vec_splat_sgt(<2 x i32> %x, <2 x i32> %y) { define <2 x i32> @p8_vec_nonsplat_undef0(<2 x i32> %x, <2 x i32> %y) { ; CHECK-LABEL: @p8_vec_nonsplat_undef0( -; CHECK-NEXT: [[T_INV:%.*]] = icmp ugt <2 x i32> [[X:%.*]], +; CHECK-NEXT: [[T_INV:%.*]] = icmp ugt <2 x i32> [[X:%.*]], ; CHECK-NEXT: [[R:%.*]] = select <2 x i1> [[T_INV]], <2 x i32> , <2 x i32> [[Y:%.*]] ; CHECK-NEXT: ret <2 x i32> [[R]] ; @@ -126,7 +126,7 @@ define <2 x i32> @p9_vec_nonsplat_undef1(<2 x i32> %x, <2 x i32> %y) { } define <2 x i32> @p10_vec_nonsplat_undef2(<2 x i32> %x, <2 x i32> %y) { ; CHECK-LABEL: @p10_vec_nonsplat_undef2( -; CHECK-NEXT: [[T_INV:%.*]] = icmp ugt <2 x i32> [[X:%.*]], +; CHECK-NEXT: [[T_INV:%.*]] = icmp ugt <2 x i32> [[X:%.*]], ; CHECK-NEXT: [[R:%.*]] = select <2 x i1> [[T_INV]], <2 x i32> , <2 x i32> [[Y:%.*]] ; CHECK-NEXT: ret <2 x i32> [[R]] ; -- 2.7.4