Handle undef vectors consistently in pattern matching

[nikic]

Summary

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.

[lebedev.ri]

include/llvm/IR/PatternMatch.h

230 ↗

(On Diff #174369)

So now instead of checking that any (zero or more) non-undef elements match,
we check that all (one or more) non-undef elements match?

In other words, if we had a vector of undefs, we used to accept it, and will no longer?
I think this needs unit tests. ({int, fp} x {scalar, vector})

test/Transforms/InstSimplify/shr-scalar-vector-consistency.ll

6 ↗

(On Diff #174369)

Please precommit the test and rebase.

[nikic]

Add unit tests, rebase over committed baseline instsimplify test.

[nikic]

include/llvm/IR/PatternMatch.h

230 ↗

(On Diff #174369)

Yes, your understanding is exactly right. Previously all cst_pred_ty/cstfp_pred_ty matchers returned true for a vector of undefs, now they will return false.

I've now added some unit tests for behavior of m_Undef and m_Zero/m_AnyZeroFP applied to scalars and vectors of undef, zero and mixed zero/undef.

[lebedev.ri]

Looks ok.

[spatel]

LGTM - have you requested commit privileges? You should be able to get that based on approved patches here + D54531 + D54532.

llvm/trunk/include/llvm/IR/PatternMatch.h

template <typename ITy> bool match(ITy *V) {
     if (V->getType()->isVectorTy()) {
       if (const auto *C = dyn_cast<Constant>(V)) {
         if (const auto *CI = dyn_cast_or_null<ConstantInt>(C->getSplatValue()))
           return this->isValue(CI->getValue());
 
         // 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)
             return false;
           if (isa<UndefValue>(Elt))
             continue;
           auto *CI = dyn_cast<ConstantInt>(Elt);
           if (!CI || !this->isValue(CI->getValue()))
             return false;
+          HasNonUndefElements = true;
         }
-        return true;
+        return HasNonUndefElements;
       }
     }
     return false;
   }
 };
 
 /// This helper class is used to match scalar and vector constants that
 /// satisfy a specified predicate, and bind them to an APInt.
template <typename ITy> bool match(ITy *V) {
     if (V->getType()->isVectorTy()) {
       if (const auto *C = dyn_cast<Constant>(V)) {
         if (const auto *CF = dyn_cast_or_null<ConstantFP>(C->getSplatValue()))
           return this->isValue(CF->getValueAPF());
 
         // 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)
             return false;
           if (isa<UndefValue>(Elt))
             continue;
           auto *CF = dyn_cast<ConstantFP>(Elt);
           if (!CF || !this->isValue(CF->getValueAPF()))
             return false;
+          HasNonUndefElements = true;
         }
-        return true;
+        return HasNonUndefElements;
       }
     }
     return false;
   }
 };
 
 ///////////////////////////////////////////////////////////////////////////////
 //

llvm/trunk/test/Transforms/InstSimplify/shr-scalar-vector-consistency.ll

 ;
   %c = sext i1 %b to i32
   %d = ashr i32 undef, %c
   ret i32 %d
 }
 
 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
   ret <2 x i32> %d
 }
 

llvm/trunk/test/Transforms/Reassociate/fp-expr.ll

 ; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
 ; RUN: opt -S -reassociate < %s | FileCheck %s
 
 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> <float -0.000000e+00, float -0.000000e+00, float -0.000000e+00, float -0.000000e+00>, [[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
 ;
   %t1 = tail call <4 x float> @blam()
   %t23 = fsub fast <4 x float> undef, %t1
   %t24 = fadd fast <4 x float> %t23, undef
   tail call void @wombat(<4 x float> %t24)
   ret void

llvm/trunk/unittests/IR/PatternMatch.cpp

EXPECT_TRUE(match(
       m_ShuffleVector(m_InsertElement(m_Undef(), m_SpecificInt(2), m_Zero()),
                       m_Undef(), m_Zero())));
   EXPECT_TRUE(match(
       SP2, m_ShuffleVector(m_InsertElement(m_Undef(), m_Value(A), m_Zero()),
                            m_Undef(), m_Zero())));
   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<Constant *, 4> 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<Constant *, 4> 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 <typename T> struct MutableConstTest : PatternMatchTest { };
 
 typedef ::testing::Types<std::tuple<Value*, Instruction*>,
                          std::tuple<const Value*, const Instruction *>>
     MutableConstTestTypes;
 TYPED_TEST_CASE(MutableConstTest, MutableConstTestTypes);
 
 TYPED_TEST(MutableConstTest, ICmp) {