[SVE] add derived vector get(Type *, ElementCount) and get(Type *, VectorType)

[ctetreau]

Summary

These new getters in the derived vector types provide consistency with
all the other derived get functions. Prior to this change, if a
developer were to write:

auto *VTy = FixedVectorType::get(SomeTy, SomeVTy->getElementCount())

Not only is the type of VTy VectorType, which is inconsistent with how
all the other getters work when called through a derived vector type
get() function, but it might even actually be an instance of
ScalableVectorType, which is almost certainly not what the caller
expects.

This patch adds get functions that ensure that all base VectorType are
hidden when called via a derived vector type. When these functions are
called, if the result would not have matched the requested type, nullptr
is returned. This enables patterns like:

if (auto *SVTy = ScalableVectorType::get(SomeTy, SomeVTy)) {
// stuff

... where if SomeVTy is a fixed width vector, the branch will not be
taken.

[efriedma]

Prior to this change, if a developer were to write:

auto *VTy = FixedVectorType::get(SomeTy, SomeVTy->getElementCount())

This is just a compile error on master. (You could write "using VectorType::get" in the class to force overloading, but definitions in derived classes hide definitions from the base class by default.)

---

Could you give an example where you plan to use this? I don't expect it's common for code to take an arbitrary vector type, and try to produce a specific vector type based on it.

[ctetreau]

In D82419#2110388, @efriedma wrote:

Prior to this change, if a developer were to write:

auto *VTy = FixedVectorType::get(SomeTy, SomeVTy->getElementCount())

This is just a compile error on master. (You could write "using VectorType::get" in the class to force overloading, but definitions in derived classes hide definitions from the base class by default.)

---

Could you give an example where you plan to use this? I don't expect it's common for code to take an arbitrary vector type, and try to produce a specific vector type based on it.

This case came up in a code review comment on D82218. I had made a change like return FixedVectorType::get(Ty, cast<FixedVectorType>(VTy)->getNumElements()); and the reviewer asked why I was casting when I could just do return FixedVectorType::get(Ty, VTy->getElementCount());. My assumption was that it would just resolve to VectorType::get(Ty, VTy->getElementCount()). My concern was that one could call FixedVectorType::get and get an instance of ScalableVectorType. While this is what would have happened if get were a free function, clearly this is not what happens with it being a static function.

There does still exist a valid use case for these new functions. It can simplify code like:

FixedVectorType *NewFVTy = nullptr;
if (auto *FVTy = dyn_cast<FixedVectorType>(VTy)) {
   NewFVTy = FixedVectorType::get(SomeTy, FVTy);
}

... into something like:

FixedVectorType *NewFVTy = FixedVectorType::get(SomeTy, VTy)

My main concern was the perceived type mismatch due to overload resolution. However, since that can't happen, I'm not 100% sure these new functions carry their own weight.

llvm/include/llvm/IR/DerivedTypes.h

 class FixedVectorType : public VectorType {
 protected:
   FixedVectorType(Type *ElTy, unsigned NumElts)
       : VectorType(ElTy, NumElts, FixedVectorTyID) {}
 
 public:
   static FixedVectorType *get(Type *ElementType, unsigned NumElts);
 
+  static FixedVectorType *get(Type *ElementType, ElementCount EC) {
+    if (EC.Scalable)
+      return nullptr;
+
+    return get(ElementType, EC.Min);
+  }
+
+  static FixedVectorType *get(Type *ElementType, const VectorType *VTy) {
+    return get(ElementType, VTy->getElementCount());
+  }
+
   static FixedVectorType *get(Type *ElementType, const FixedVectorType *FVTy) {
     return get(ElementType, FVTy->getNumElements());
   }
 
   static FixedVectorType *getInteger(FixedVectorType *VTy) {
     return cast<FixedVectorType>(VectorType::getInteger(VTy));
   }
 

 class ScalableVectorType : public VectorType {
 protected:
   ScalableVectorType(Type *ElTy, unsigned MinNumElts)
       : VectorType(ElTy, MinNumElts, ScalableVectorTyID) {}
 
 public:
   static ScalableVectorType *get(Type *ElementType, unsigned MinNumElts);
 
+  static ScalableVectorType *get(Type *ElementType, ElementCount EC) {
+    if (!EC.Scalable)
+      return nullptr;
+
+    return get(ElementType, EC.Min);
+  }
+
+  static ScalableVectorType *get(Type *ElementType, const VectorType *VTy) {
+    return get(ElementType, VTy->getElementCount());
+  }
+
   static ScalableVectorType *get(Type *ElementType,
                                  const ScalableVectorType *SVTy) {
     return get(ElementType, SVTy->getMinNumElements());
   }
 
   static ScalableVectorType *getInteger(ScalableVectorType *VTy) {
     return cast<ScalableVectorType>(VectorType::getInteger(VTy));
   }

llvm/unittests/IR/VectorTypesTest.cpp

TEST(VectorTypesTest, CrossComparisons) {
   // If we are only checking the minimum, then they are the same size.
   EXPECT_EQ(V4Int32Ty->getPrimitiveSizeInBits().getKnownMinSize(),
             ScV4Int32Ty->getPrimitiveSizeInBits().getKnownMinSize());
 
   // We can't use ordering comparisons (<,<=,>,>=) between scalable and
   // non-scalable vector sizes.
 }
 
+TEST(VectorTypesTest, DerivedGettersHidingBaseGetters) {
+  // The derived vector types implement get(Type *, ElementCount) and
+  // get(Type*, VectorType*) that hide the base class versions and return
+  // instances of the derived vector type or null.
+
+  LLVMContext Ctx;
+
+  Type *Ty = Type::getInt32Ty(Ctx);
+
+  ElementCount FV4 = {4, false};
+  ElementCount SV4 = {4, true};
+
+  auto *BFV4Ty = VectorType::get(Ty, FV4);
+  auto *BSV4Ty = VectorType::get(Ty, SV4);
+
+  EXPECT_NE(nullptr, BFV4Ty);
+  EXPECT_NE(nullptr, BSV4Ty);
+
+  // Test ElementCount getters
+  auto *FV4Ty = FixedVectorType::get(Ty, FV4);
+  auto *FV4TyNaught = FixedVectorType::get(Ty, SV4);
+  auto *SV4Ty = ScalableVectorType::get(Ty, SV4);
+  auto *SV4TyNaught = ScalableVectorType::get(Ty, FV4);
+
+  EXPECT_EQ(BFV4Ty, FV4Ty);
+  EXPECT_EQ(nullptr, FV4TyNaught);
+  EXPECT_EQ(BSV4Ty, SV4Ty);
+  EXPECT_EQ(nullptr, SV4TyNaught);
+
+  // Test VectorType getters
+  FV4Ty = FixedVectorType::get(Ty, BFV4Ty);
+  FV4TyNaught = FixedVectorType::get(Ty, BSV4Ty);
+  SV4Ty = ScalableVectorType::get(Ty, BSV4Ty);
+  SV4TyNaught = ScalableVectorType::get(Ty, BFV4Ty);
+
+  EXPECT_EQ(BFV4Ty, FV4Ty);
+  EXPECT_EQ(nullptr, FV4TyNaught);
+  EXPECT_EQ(BSV4Ty, SV4Ty);
+  EXPECT_EQ(nullptr, SV4TyNaught);
+}
+
 } // end anonymous namespace