From f5ead29b781fea13bdac8bf154a6a600b2adfbb4 Mon Sep 17 00:00:00 2001 From: Sanjay Patel Date: Tue, 20 Nov 2018 16:08:19 +0000 Subject: [PATCH] [PatternMatch] Handle undef vectors consistently This patch fixes the issue noticed in D54532. The problem is that cst_pred_ty-based matchers like m_Zero() currently do not match scalar undefs (as expected), but *do* match vector undefs. This may lead to optimization inconsistencies in rare cases. There is only one existing test for which output changes, reverting the change from D53205. The reason here is that vector fsub undef, %x is no longer matched as an m_FNeg(). While I think that the new output is technically worse than the previous one, it is consistent with scalar, and I don't think it's really important either way (generally that undef should have been folded away prior to reassociation.) I've also added another test case for this issue based on InstructionSimplify. It took some effort to find that one, as in most cases undef folds are either checked first -- and in the cases where they aren't it usually happens to not make a difference in the end. This is the only case I was able to come up with. Prior to this patch the test case simplified to undef in the scalar case, but zeroinitializer in the vector case. Patch by: @nikic (Nikita Popov) Differential Revision: https://reviews.llvm.org/D54631 llvm-svn: 347318 --- llvm/include/llvm/IR/PatternMatch.h | 8 ++- .../shr-scalar-vector-consistency.ll | 2 +- llvm/test/Transforms/Reassociate/fp-expr.ll | 4 +- llvm/unittests/IR/PatternMatch.cpp | 56 +++++++++++++++++++ 4 files changed, 65 insertions(+), 5 deletions(-) diff --git a/llvm/include/llvm/IR/PatternMatch.h b/llvm/include/llvm/IR/PatternMatch.h index dd30072ce571..e1e7c7268b5d 100644 --- a/llvm/include/llvm/IR/PatternMatch.h +++ b/llvm/include/llvm/IR/PatternMatch.h @@ -215,6 +215,7 @@ template struct cst_pred_ty : public Predicate { // Non-splat vector constant: check each element for a match. unsigned NumElts = V->getType()->getVectorNumElements(); assert(NumElts != 0 && "Constant vector with no elements?"); + bool HasNonUndefElements = false; for (unsigned i = 0; i != NumElts; ++i) { Constant *Elt = C->getAggregateElement(i); if (!Elt) @@ -224,8 +225,9 @@ template struct cst_pred_ty : public Predicate { auto *CI = dyn_cast(Elt); if (!CI || !this->isValue(CI->getValue())) return false; + HasNonUndefElements = true; } - return true; + return HasNonUndefElements; } } return false; @@ -272,6 +274,7 @@ template struct cstfp_pred_ty : public Predicate { // Non-splat vector constant: check each element for a match. unsigned NumElts = V->getType()->getVectorNumElements(); assert(NumElts != 0 && "Constant vector with no elements?"); + bool HasNonUndefElements = false; for (unsigned i = 0; i != NumElts; ++i) { Constant *Elt = C->getAggregateElement(i); if (!Elt) @@ -281,8 +284,9 @@ template struct cstfp_pred_ty : public Predicate { auto *CF = dyn_cast(Elt); if (!CF || !this->isValue(CF->getValueAPF())) return false; + HasNonUndefElements = true; } - return true; + return HasNonUndefElements; } } return false; diff --git a/llvm/test/Transforms/InstSimplify/shr-scalar-vector-consistency.ll b/llvm/test/Transforms/InstSimplify/shr-scalar-vector-consistency.ll index f1bba37ff991..90725a153a2f 100644 --- a/llvm/test/Transforms/InstSimplify/shr-scalar-vector-consistency.ll +++ b/llvm/test/Transforms/InstSimplify/shr-scalar-vector-consistency.ll @@ -16,7 +16,7 @@ define i32 @test_scalar(i32 %a, i1 %b) { define <2 x i32> @test_vector(<2 x i32> %a, <2 x i1> %b) { ; CHECK-LABEL: @test_vector( -; CHECK-NEXT: ret <2 x i32> zeroinitializer +; CHECK-NEXT: ret <2 x i32> undef ; %c = sext <2 x i1> %b to <2 x i32> %d = ashr <2 x i32> undef, %c diff --git a/llvm/test/Transforms/Reassociate/fp-expr.ll b/llvm/test/Transforms/Reassociate/fp-expr.ll index dcbf835ba544..e616c52f28e6 100644 --- a/llvm/test/Transforms/Reassociate/fp-expr.ll +++ b/llvm/test/Transforms/Reassociate/fp-expr.ll @@ -4,8 +4,8 @@ define void @test1() { ; CHECK-LABEL: @test1( ; CHECK-NEXT: [[T1:%.*]] = tail call <4 x float> @blam() -; CHECK-NEXT: [[T23:%.*]] = fsub fast <4 x float> undef, [[T1]] -; CHECK-NEXT: [[T24:%.*]] = fadd fast <4 x float> [[T23]], undef +; CHECK-NEXT: [[T1_NEG:%.*]] = fsub fast <4 x float> , [[T1]] +; CHECK-NEXT: [[T24:%.*]] = fadd fast <4 x float> [[T1_NEG]], undef ; CHECK-NEXT: tail call void @wombat(<4 x float> [[T24]]) ; CHECK-NEXT: ret void ; diff --git a/llvm/unittests/IR/PatternMatch.cpp b/llvm/unittests/IR/PatternMatch.cpp index 6b5686d53c66..976e42d0dbba 100644 --- a/llvm/unittests/IR/PatternMatch.cpp +++ b/llvm/unittests/IR/PatternMatch.cpp @@ -534,6 +534,62 @@ TEST_F(PatternMatchTest, VectorOps) { EXPECT_TRUE(A == Val); } +TEST_F(PatternMatchTest, VectorUndefInt) { + Type *ScalarTy = IRB.getInt8Ty(); + Type *VectorTy = VectorType::get(ScalarTy, 4); + Constant *ScalarUndef = UndefValue::get(ScalarTy); + Constant *VectorUndef = UndefValue::get(VectorTy); + Constant *ScalarZero = Constant::getNullValue(ScalarTy); + Constant *VectorZero = Constant::getNullValue(VectorTy); + + SmallVector Elems; + Elems.push_back(ScalarUndef); + Elems.push_back(ScalarZero); + Elems.push_back(ScalarUndef); + Elems.push_back(ScalarZero); + Constant *VectorZeroUndef = ConstantVector::get(Elems); + + EXPECT_TRUE(match(ScalarUndef, m_Undef())); + EXPECT_TRUE(match(VectorUndef, m_Undef())); + EXPECT_FALSE(match(ScalarZero, m_Undef())); + EXPECT_FALSE(match(VectorZero, m_Undef())); + EXPECT_FALSE(match(VectorZeroUndef, m_Undef())); + + EXPECT_FALSE(match(ScalarUndef, m_Zero())); + EXPECT_FALSE(match(VectorUndef, m_Zero())); + EXPECT_TRUE(match(ScalarZero, m_Zero())); + EXPECT_TRUE(match(VectorZero, m_Zero())); + EXPECT_TRUE(match(VectorZeroUndef, m_Zero())); +} + +TEST_F(PatternMatchTest, VectorUndefFloat) { + Type *ScalarTy = IRB.getFloatTy(); + Type *VectorTy = VectorType::get(ScalarTy, 4); + Constant *ScalarUndef = UndefValue::get(ScalarTy); + Constant *VectorUndef = UndefValue::get(VectorTy); + Constant *ScalarZero = Constant::getNullValue(ScalarTy); + Constant *VectorZero = Constant::getNullValue(VectorTy); + + SmallVector Elems; + Elems.push_back(ScalarUndef); + Elems.push_back(ScalarZero); + Elems.push_back(ScalarUndef); + Elems.push_back(ScalarZero); + Constant *VectorZeroUndef = ConstantVector::get(Elems); + + EXPECT_TRUE(match(ScalarUndef, m_Undef())); + EXPECT_TRUE(match(VectorUndef, m_Undef())); + EXPECT_FALSE(match(ScalarZero, m_Undef())); + EXPECT_FALSE(match(VectorZero, m_Undef())); + EXPECT_FALSE(match(VectorZeroUndef, m_Undef())); + + EXPECT_FALSE(match(ScalarUndef, m_AnyZeroFP())); + EXPECT_FALSE(match(VectorUndef, m_AnyZeroFP())); + EXPECT_TRUE(match(ScalarZero, m_AnyZeroFP())); + EXPECT_TRUE(match(VectorZero, m_AnyZeroFP())); + EXPECT_TRUE(match(VectorZeroUndef, m_AnyZeroFP())); +} + template struct MutableConstTest : PatternMatchTest { }; typedef ::testing::Types,