[SVE] Add new VectorType subclasses

[ctetreau]

Summary

Introduce new types for fixed width and scalable vectors.

Does not remove getNumElements yet so as to not break code during transition
period.

[ctetreau]

fix permissions

[ctetreau]

Add new VectorType::get overloads

[ctetreau]

Fix getPrimitiveSizeInBits

[ctetreau]

Fix non-exhaustive switch

[efriedma]

If I'm following correctly, this should apply on its own. Then you're expecting the following API changes:

1. getNumElements() will move from VectorType to FixedVectorType. Existing code will be changed to either cast to FixedVectorType, or switch to using getElementCount().
2. Maybe remove the default argument from VectorType::get()

Does that sound right?

llvm/include/llvm-c/Core.h

165

Adding LLVMFixedVectorTypeKind and LLVMScalableVectorTypeKind without removing LLVMVectorTypeKind does't make sense.

llvm/include/llvm/IR/DerivedTypes.h

389

"vector"

430

It seems confusing to overload get() this way.

llvm/include/llvm/IR/Type.h

76

VectorTyID should be dead.

llvm/lib/IR/Type.cpp

128

I'm not sure killing off isScalable() is productive.

[efriedma]

llvm/include/llvm/IR/Type.h

76

Oh, there's still a bunch of uses of VectorTyID. You probably need to take care of that in this patch.

[ctetreau]

llvm/include/llvm/IR/DerivedTypes.h

430

There's a bunch of code that is "I want a vector with the same shape as some other vector, but with a different element type"

Currently, there's a bunch of variants like:

auto *V2 = VectorType::Get(SomeTy, V1->getNumElements());
auto *V3 = VectorType::Get(SomeTy, V4->getElementType());

Really, for all variants of this operation, the first is potentially buggy if v1 is scalable, and it can always be replaced with the second and be correct (unless you are specifically trying to get a fixed width vector that has the same minimum number of elements as some potentially scalable vector, but that's a strange special case and I'd ask in code review that the author specifically pass false). But really, the quantity that you get out of the second argument is always noise. This change makes the following equivalent:

auto *V2 = VectorType::Get(SomeTy, V1->getElementType());
auto *V3 = VectorType::Get(SomeTy, V1);
assert(V2->getType() == V3->getType());

If V1 was scalable, then V2 is scalable, and vice versa. If you like, I can improve the documentation for this function, but I think it adds value.

llvm/include/llvm/IR/Type.h

76

Since this thing is exposed from the C api, it might break external code to remove it.

However, it is the design that it should never be possible to construct a base VectorType. Maybe it makes sense to delete it here, but leave the type in the C api?

llvm/lib/IR/Type.cpp

128

I'm still in the early stages of this, but if I have an instance of ScalableVectorType, then this is always true, and vice versa. It's kind of a pointless function. Usages of it tend to look like this:

if (isa<VectorType>(Ty) && cast<VectorType>(Ty)->isScalable())
   doStuff(Ty);

If it ends up being a ton of work, I'll leave it

[efriedma]

llvm/include/llvm/IR/Type.h

76

Whether or not we actually remove LLVMVectorTypeKind from the enum, it's a breaking change from the C API's perspective if vector types don't have the right ID. So we have two options: either make the breaking change to the C API, or don't expose the Scalable/Fixed split via the C API at all (and explicitly remap the internal IDs in LLVMGetTypeKind).

Either way, that isn't a reason to keep around the C++ VectorTyID; we don't expose that directly anyway.

[ctetreau]

llvm/include/llvm/IR/Type.h

76

Fair enough, I'll get rid of it

[ctetreau]

adddress code review issues

[ctetreau]

In D77587#1965604, @efriedma wrote:

If I'm following correctly, this should apply on its own. Then you're expecting the following API changes:

1. getNumElements() will move from VectorType to FixedVectorType. Existing code will be changed to either cast to FixedVectorType, or switch to using getElementCount().
2. Maybe remove the default argument from VectorType::get()

Does that sound right?

This is basically the plan:

1. Add the new types and functions
2. Fix up usages of getNumElements()
3. move getNumElements() and getMinNumElements() into the derived classes

[ctetreau]

Fix permissions

[ctetreau]

Move getMinNumElements to ScalableVectorType. There's no reason for it to be in base VectorType

[efriedma]

Looks right, generally.

llvm/include/llvm-c/Core.h

179

The C API is officially not ABI-stable anymore across versions, No reason to have a gap like this.

llvm/lib/ExecutionEngine/ExecutionEngine.cpp

1011 ↗

(On Diff #256129)

?

llvm/lib/Target/AMDGPU/AMDGPUPrintfRuntimeBinding.cpp

481 ↗

(On Diff #256129)

Please fix this to use isa<> while you're here.

[ctetreau]

llvm/lib/ExecutionEngine/ExecutionEngine.cpp

1011 ↗

(On Diff #256129)

clang-format-diff must have done this.

[sdesmalen]

Thanks for this patch @ctetreau! Overall looks great to me, just two little nits.

llvm/include/llvm/IR/DerivedTypes.h

434

Do you need the method on line 432 if you have this one (that takes a const VectorType *Other) ?

llvm/lib/ExecutionEngine/ExecutionEngine.cpp

1109 ↗

(On Diff #256129)

This code doesn't really work for scalable vectors. Assuming you don't want to change that in this patch, Is it worth putting a FIXME here?

[ctetreau]

llvm/include/llvm/IR/DerivedTypes.h

434

It shouldn't be needed, I'll remove it.

llvm/lib/ExecutionEngine/ExecutionEngine.cpp

1109 ↗

(On Diff #256129)

This will have to be changed when getNumElements() moves into FixedVectorType. I'll get it then.

Likely, the plan will be to just have the scalable vector branch assert. My overall strategy is to just assume all calls to getNumElements are correct, cast to FixedVectorType instead of VectorType, and just fix enough to make the tests pass.

There's a lot of code that calls getNumElements, and the overall goal is to force everybody to clean their own houses.

[ctetreau]

address code review issues

[ctetreau]

rebase

[ctetreau]

llvm/include/llvm/IR/DerivedTypes.h

430

In this example, "getElementType" should be "getElementCount", but I assume you get the idea...

[sdesmalen]

llvm/lib/Target/AMDGPU/AMDGPUPrintfRuntimeBinding.cpp

411 ↗

(On Diff #257138)

should this be !isa<FixedVectorType>(ArgType) (i.e. negated)?

[ctetreau]

address code review issues

[sdesmalen]

Thanks for all the changes, this patch LGTM!

[ctetreau]

Fixup canLosslesslyBitCastTo to use FixedVectorType again

[ctetreau]

rebase

[ctetreau]

remove unused variable

[ctetreau]

Catch stragglers in C api

[ctetreau]

split C api stuff into separate commit. Do bare minimum here

llvm/include/llvm-c/Core.h

typedef enum {
   LLVMIntegerTypeKind,     /**< Arbitrary bit width integers */
   LLVMFunctionTypeKind,    /**< Functions */
   LLVMStructTypeKind,      /**< Structures */
   LLVMArrayTypeKind,       /**< Arrays */
   LLVMPointerTypeKind,     /**< Pointers */
   LLVMVectorTypeKind,      /**< SIMD 'packed' format, or other vector type */
   LLVMMetadataTypeKind,    /**< Metadata */
   LLVMX86_MMXTypeKind,     /**< X86 MMX */
-  LLVMTokenTypeKind        /**< Tokens */
+  LLVMTokenTypeKind,       /**< Tokens */
+  LLVMFixedVectorTypeKind, /**< Fixed width SIMD vector type */
+  LLVMScalableVectorTypeKind /**< Scalable SIMD vector type */
 } LLVMTypeKind;
 
 typedef enum {
   LLVMExternalLinkage,    /**< Externally visible function */
   LLVMAvailableExternallyLinkage,
   LLVMLinkOnceAnyLinkage, /**< Keep one copy of function when linking (inline)*/
   LLVMLinkOnceODRLinkage, /**< Same, but only replaced by something
                             equivalent. */
   LLVMLinkOnceODRAutoHideLinkage, /**< Obsolete */
   LLVMWeakAnyLinkage,     /**< Keep one copy of function when linking (weak) */
   LLVMWeakODRLinkage,     /**< Same, but only replaced by something
                             equivalent. */
   LLVMAppendingLinkage,   /**< Special purpose, only applies to global arrays */
   LLVMInternalLinkage,    /**< Rename collisions when linking (static
                                functions) */
   LLVMPrivateLinkage,     /**< Like Internal, but omit from symbol table */
   LLVMDLLImportLinkage,   /**< Obsolete */
   LLVMDLLExportLinkage,   /**< Obsolete */
   LLVMExternalWeakLinkage,/**< ExternalWeak linkage description */
   LLVMGhostLinkage,       /**< Obsolete */
   LLVMCommonLinkage,      /**< Tentative definitions */
   LLVMLinkerPrivateLinkage, /**< Like Private, but linker removes. */

llvm/include/llvm/IR/DerivedTypes.h

static bool classof(const Type *T) {
     return T->getTypeID() == ArrayTyID;
   }
 };
 
 uint64_t Type::getArrayNumElements() const {
   return cast<ArrayType>(this)->getNumElements();
 }
 
-/// Class to represent vector types.
+/// Base class of all SIMD vecctor types
 class VectorType : public Type {
   /// A fully specified VectorType is of the form <vscale x n x Ty>. 'n' is the
   /// minimum number of elements of type Ty contained within the vector, and
   /// 'vscale x' indicates that the total element count is an integer multiple
   /// of 'n', where the multiple is either guaranteed to be one, or is
   /// statically unknown at compile time.
   ///
   /// If the multiple is known to be 1, then the extra term is discarded in
   /// textual IR:
   ///
   /// <4 x i32>          - a vector containing 4 i32s
   /// <vscale x 4 x i32> - a vector containing an unknown integer multiple
   ///                      of 4 i32s
 
   /// The element type of the vector.
   Type *ContainedType;
-  /// Minumum number of elements in the vector.
-  uint64_t NumElements;
 
-  VectorType(Type *ElType, unsigned NumEl, bool Scalable = false);
-  VectorType(Type *ElType, ElementCount EC);
+  /// The element count of this vector
+  ElementCount EC;
 
-  // If true, the total number of elements is an unknown multiple of the
-  // minimum 'NumElements'. Otherwise the total number of elements is exactly
-  // equal to 'NumElements'.
-  bool Scalable;
+protected:
+  VectorType(Type *ElType, ElementCount EC, Type::TypeID TID);
 
 public:
   VectorType(const VectorType &) = delete;
   VectorType &operator=(const VectorType &) = delete;
 
   /// For scalable vectors, this will return the minimum number of elements
   /// in the vector.
-  uint64_t getNumElements() const { return NumElements; }
+  uint64_t getNumElements() const { return EC.Min; }
   Type *getElementType() const { return ContainedType; }
 
   /// This static method is the primary way to construct an VectorType.
   static VectorType *get(Type *ElementType, ElementCount EC);
   static VectorType *get(Type *ElementType, unsigned NumElements,
                          bool Scalable = false) {
     return VectorType::get(ElementType, {NumElements, Scalable});
   }
 
+  /// Construct a VectorType that has the same shape as some other VectorType
+  static VectorType *get(Type *ElementType, VectorType *Other) {
+    return VectorType::get(ElementType, Other->getElementCount());
+  }
+
+  static VectorType *get(Type *ElementType, const VectorType *Other) {
+    return VectorType::get(ElementType, Other->getElementCount());
+  }
+
   /// This static method gets a VectorType with the same number of elements as
   /// the input type, and the element type is an integer type of the same width
   /// as the input element type.
   static VectorType *getInteger(VectorType *VTy) {
     unsigned EltBits = VTy->getElementType()->getPrimitiveSizeInBits();
     assert(EltBits && "Element size must be of a non-zero size");
     Type *EltTy = IntegerType::get(VTy->getContext(), EltBits);
     return VectorType::get(EltTy, VTy->getElementCount());
public:
   /// Return true if the specified type is valid as a element type.
   static bool isValidElementType(Type *ElemTy);
 
   /// Return an ElementCount instance to represent the (possibly scalable)
   /// number of elements in the vector.
   ElementCount getElementCount() const {
     uint64_t MinimumEltCnt = getNumElements();
     assert(MinimumEltCnt <= UINT_MAX && "Too many elements in vector");
-    return { (unsigned)MinimumEltCnt, Scalable };
+    return EC;
   }
 
   /// Returns whether or not this is a scalable vector (meaning the total
   /// element count is a multiple of the minimum).
-  bool isScalable() const {
-    return Scalable;
-  }
+  bool isScalable() const { return EC.Scalable; }
 
   /// Return the minimum number of bits in the Vector type.
   /// Returns zero when the vector is a vector of pointers.
   unsigned getBitWidth() const {
     return getNumElements() * getElementType()->getPrimitiveSizeInBits();
   }
 
   /// Methods for support type inquiry through isa, cast, and dyn_cast.
   static bool classof(const Type *T) {
-    return T->getTypeID() == VectorTyID;
+    switch (T->getTypeID()) {
+    case VectorTyID:
+    case FixedVectorTyID:
+    case ScalableVectorTyID:
+      return true;
+    default:
+      return false;
+    }
   }
 };
 
 bool Type::isVectorTy() const { return isa<VectorType>(this); }
 
+/// Class to represent fixed width SIMD vectors
+class FixedVectorType : public VectorType {
+protected:
+  FixedVectorType(Type *ElTy, unsigned NumElts)
+      : VectorType(ElTy, {NumElts, false}, FixedVectorTyID) {}
+
+public:
+  static FixedVectorType *get(Type *ElementType, unsigned NumElts);
+
+  static bool classof(const Type *T) {
+    return T->getTypeID() == FixedVectorTyID;
+  }
+};
+
+/// Class to represent scalable SIMD vectors
+class ScalableVectorType : public VectorType {
+protected:
+  ScalableVectorType(Type *ElTy, unsigned MinNumElts)
+      : VectorType(ElTy, {MinNumElts, true}, ScalableVectorTyID) {}
+
+public:
+  static ScalableVectorType *get(Type *ElementType, unsigned MinNumElts);
+
+  static bool classof(const Type *T) {
+    return T->getTypeID() == ScalableVectorTyID;
+  }
+};
+
 /// Class to represent pointers.
 class PointerType : public Type {
   explicit PointerType(Type *ElType, unsigned AddrSpace);
 
   Type *PointeeTy;
 
 public:
   PointerType(const PointerType &) = delete;

llvm/include/llvm/IR/Type.h

public:
   /// Definitions of all of the base types for the Type system.  Based on this
   /// value, you can cast to a class defined in DerivedTypes.h.
   /// Note: If you add an element to this, you need to add an element to the
   /// Type::getPrimitiveType function, or else things will break!
   /// Also update LLVMTypeKind and LLVMGetTypeKind () in the C binding.
   ///
   enum TypeID {
     // PrimitiveTypes - make sure LastPrimitiveTyID stays up to date.
-    VoidTyID = 0,    ///<  0: type with no size
-    HalfTyID,        ///<  1: 16-bit floating point type
-    FloatTyID,       ///<  2: 32-bit floating point type
-    DoubleTyID,      ///<  3: 64-bit floating point type
-    X86_FP80TyID,    ///<  4: 80-bit floating point type (X87)
-    FP128TyID,       ///<  5: 128-bit floating point type (112-bit mantissa)
-    PPC_FP128TyID,   ///<  6: 128-bit floating point type (two 64-bits, PowerPC)
-    LabelTyID,       ///<  7: Labels
-    MetadataTyID,    ///<  8: Metadata
-    X86_MMXTyID,     ///<  9: MMX vectors (64 bits, X86 specific)
-    TokenTyID,       ///< 10: Tokens
+    VoidTyID = 0,  ///<  0: type with no size
+    HalfTyID,      ///<  1: 16-bit floating point type
+    FloatTyID,     ///<  2: 32-bit floating point type
+    DoubleTyID,    ///<  3: 64-bit floating point type
+    X86_FP80TyID,  ///<  4: 80-bit floating point type (X87)
+    FP128TyID,     ///<  5: 128-bit floating point type (112-bit mantissa)
+    PPC_FP128TyID, ///<  6: 128-bit floating point type (two 64-bits, PowerPC)
+    LabelTyID,     ///<  7: Labels
+    MetadataTyID,  ///<  8: Metadata
+    X86_MMXTyID,   ///<  9: MMX vectors (64 bits, X86 specific)
+    TokenTyID,     ///< 10: Tokens
 
     // Derived types... see DerivedTypes.h file.
     // Make sure FirstDerivedTyID stays up to date!
-    IntegerTyID,     ///< 11: Arbitrary bit width integers
-    FunctionTyID,    ///< 12: Functions
-    StructTyID,      ///< 13: Structures
-    ArrayTyID,       ///< 14: Arrays
-    PointerTyID,     ///< 15: Pointers
-    VectorTyID       ///< 16: SIMD 'packed' format, or other vector type
+    IntegerTyID,       ///< 11: Arbitrary bit width integers
+    FunctionTyID,      ///< 12: Functions
+    StructTyID,        ///< 13: Structures
+    ArrayTyID,         ///< 14: Arrays
+    PointerTyID,       ///< 15: Pointers
+    VectorTyID,        ///< 16: SIMD 'packed' format, or other vector type
+    FixedVectorTyID,   ///< 17: Fixed width SIMD vector type
+    ScalableVectorTyID ///< 18: Scalable SIMD vector type
   };
 
 private:
   /// This refers to the LLVMContext in which this type was uniqued.
   LLVMContext &Context;
 
   TypeID   ID : 8;            // The current base type of this type.
   unsigned SubclassData : 24; // Space for subclasses to store data.

llvm/lib/IR/Core.cpp

LLVMTypeKind LLVMGetTypeKind(LLVMTypeRef Ty) {
   case Type::PointerTyID:
     return LLVMPointerTypeKind;
   case Type::VectorTyID:
     return LLVMVectorTypeKind;
   case Type::X86_MMXTyID:
     return LLVMX86_MMXTypeKind;
   case Type::TokenTyID:
     return LLVMTokenTypeKind;
+  case Type::FixedVectorTyID:
+    return LLVMFixedVectorTypeKind;
+  case Type::ScalableVectorTyID:
+    return LLVMScalableVectorTypeKind;
   }
   llvm_unreachable("Unhandled TypeID.");
 }
 
 LLVMBool LLVMTypeIsSized(LLVMTypeRef Ty)
 {
     return unwrap(Ty)->isSized();
 }

llvm/lib/IR/Type.cpp

TypeSize Type::getPrimitiveSizeInBits() const {
   case Type::FloatTyID: return TypeSize::Fixed(32);
   case Type::DoubleTyID: return TypeSize::Fixed(64);
   case Type::X86_FP80TyID: return TypeSize::Fixed(80);
   case Type::FP128TyID: return TypeSize::Fixed(128);
   case Type::PPC_FP128TyID: return TypeSize::Fixed(128);
   case Type::X86_MMXTyID: return TypeSize::Fixed(64);
   case Type::IntegerTyID:
     return TypeSize::Fixed(cast<IntegerType>(this)->getBitWidth());
-  case Type::VectorTyID: {
+  case Type::VectorTyID:
+  case Type::FixedVectorTyID:
+  case Type::ScalableVectorTyID: {
     const VectorType *VTy = cast<VectorType>(this);
-    return TypeSize(VTy->getBitWidth(), VTy->isScalable());
+    return TypeSize(VTy->getBitWidth(), isa<ScalableVectorType>(VTy));
   }
   default: return TypeSize::Fixed(0);
   }
 }
 
 unsigned Type::getScalarSizeInBits() const {
   // It is safe to assume that the scalar types have a fixed size.
   return getScalarType()->getPrimitiveSizeInBits().getFixedSize();
return !ElemTy->isVoidTy() && !ElemTy->isLabelTy() &&
          !ElemTy->isMetadataTy() && !ElemTy->isFunctionTy() &&
          !ElemTy->isTokenTy();
 }
 
 //===----------------------------------------------------------------------===//
 //                          VectorType Implementation
 //===----------------------------------------------------------------------===//
 
-VectorType::VectorType(Type *ElType, ElementCount EC)
-    : Type(ElType->getContext(), VectorTyID), ContainedType(ElType),
-      NumElements(EC.Min), Scalable(EC.Scalable) {
+VectorType::VectorType(Type *ElType, ElementCount EC, Type::TypeID TID)
+    : Type(ElType->getContext(), TID), ContainedType(ElType), EC(EC) {
   ContainedTys = &ContainedType;
   NumContainedTys = 1;
 }
 
 VectorType *VectorType::get(Type *ElementType, ElementCount EC) {
-  assert(EC.Min > 0 && "#Elements of a VectorType must be greater than 0");
+  if (EC.Scalable)
+    return ScalableVectorType::get(ElementType, EC.Min);
+  else
+    return FixedVectorType::get(ElementType, EC.Min);
+}
+
+bool VectorType::isValidElementType(Type *ElemTy) {
+  return ElemTy->isIntegerTy() || ElemTy->isFloatingPointTy() ||
+         ElemTy->isPointerTy();
+}
+
+//===----------------------------------------------------------------------===//
+//                        FixedVectorType Implementation
+//===----------------------------------------------------------------------===//
+
+FixedVectorType *FixedVectorType::get(Type *ElementType, unsigned NumElts) {
+  assert(NumElts > 0 && "#Elements of a VectorType must be greater than 0");
   assert(isValidElementType(ElementType) && "Element type of a VectorType must "
                                             "be an integer, floating point, or "
                                             "pointer type.");
 
+  ElementCount EC(NumElts, false);
+
   LLVMContextImpl *pImpl = ElementType->getContext().pImpl;
-  VectorType *&Entry = ElementType->getContext().pImpl
-                                 ->VectorTypes[std::make_pair(ElementType, EC)];
+  VectorType *&Entry = ElementType->getContext()
+                           .pImpl->VectorTypes[std::make_pair(ElementType, EC)];
+
   if (!Entry)
-    Entry = new (pImpl->Alloc) VectorType(ElementType, EC);
-  return Entry;
+    Entry = new (pImpl->Alloc) FixedVectorType(ElementType, NumElts);
+  return cast<FixedVectorType>(Entry);
 }
 
-bool VectorType::isValidElementType(Type *ElemTy) {
-  return ElemTy->isIntegerTy() || ElemTy->isFloatingPointTy() ||
-    ElemTy->isPointerTy();
+//===----------------------------------------------------------------------===//
+//                       ScalableVectorType Implementation
+//===----------------------------------------------------------------------===//
+
+ScalableVectorType *ScalableVectorType::get(Type *ElementType,
+                                            unsigned MinNumElts) {
+  assert(MinNumElts > 0 && "#Elements of a VectorType must be greater than 0");
+  assert(isValidElementType(ElementType) && "Element type of a VectorType must "
+                                            "be an integer, floating point, or "
+                                            "pointer type.");
+
+  ElementCount EC(MinNumElts, true);
+
+  LLVMContextImpl *pImpl = ElementType->getContext().pImpl;
+  VectorType *&Entry = ElementType->getContext()
+                           .pImpl->VectorTypes[std::make_pair(ElementType, EC)];
+
+  if (!Entry)
+    Entry = new (pImpl->Alloc) ScalableVectorType(ElementType, MinNumElts);
+  return cast<ScalableVectorType>(Entry);
 }
 
 //===----------------------------------------------------------------------===//
 //                         PointerType Implementation
 //===----------------------------------------------------------------------===//
 
 PointerType *PointerType::get(Type *EltTy, unsigned AddressSpace) {
   assert(EltTy && "Can't get a pointer to <null> type!");