[PATCH] D77587: [SVE] Add new VectorType subclasses

[Eli Friedman via Phabricator via llvm-commits]

efriedma added a comment.

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?



================
Comment at: llvm/include/llvm-c/Core.h:165
+  LLVMFixedVectorTypeKind, /**< Fixed width SIMD vector type */
+  LLVMScalableVectorTypeKind /**< Scalable SIMD vector type */
 } LLVMTypeKind;
----------------
Adding LLVMFixedVectorTypeKind and LLVMScalableVectorTypeKind without removing LLVMVectorTypeKind does't make sense.


================
Comment at: llvm/include/llvm/IR/DerivedTypes.h:389
 
-/// Class to represent vector types.
+/// Base class of all SIMD vecctor types
 class VectorType : public Type {
----------------
"vector"


================
Comment at: llvm/include/llvm/IR/DerivedTypes.h:430
+  /// 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());
----------------
It seems confusing to overload get() this way.


================
Comment at: llvm/include/llvm/IR/Type.h:76
+    PointerTyID,       ///< 15: Pointers
+    VectorTyID,        ///< 16: SIMD 'packed' format, or other vector type
+    FixedVectorTyID,   ///< 17: Fixed width SIMD vector type
----------------
VectorTyID should be dead.


================
Comment at: llvm/lib/IR/Type.cpp:128
     const VectorType *VTy = cast<VectorType>(this);
-    return TypeSize(VTy->getBitWidth(), VTy->isScalable());
   }
----------------
I'm not sure killing off isScalable() is productive.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D77587/new/

https://reviews.llvm.org/D77587