[SVE] Make getNumElements assert if the vector is scalable

[ctetreau]

Summary

VectorType inherits getNumElements from SequentialType. For a fixed

length vector, this returns the number of vector lanes. Previously, for
scalable vectors, this has returned the n for <vscale x n x ty>.
VectorType has another function getElementCount that returns a pair of
(bool, uint64_t) where the bool is true if the vector is scalable, and
min is the same as the return value of getNumElements. This form is
preferred because it provides the caller a way of knowing if the vector
is scalable.

There exists a large amount of code that calls getNumElements and

iterates over the returned value. This code is correct for fixed length
vectors, but is always a bug for scalable vectors since the number of
vector lanes in a scalable vector is unknown at compile time. To
mitigate bugs that are caused by this situation, we are changing the
behavior of getNumElements to assert that if the SequentialType instance
is a vector, that the vector is not scalable. This should be a minimally
intrusive change since very little code has been written that uses
getNumElements correctly.

[ctetreau]

fix commit message formatting

[efriedma]

https://reviews.llvm.org/B47829 says there are regression test failures; we can't merge until those are fixed.

I think this makes sense.

llvm/lib/IR/Type.cpp

584

Please make this inline.

[ctetreau]

I'm working through the test failures right now. It may be a little bit before I get a chance to update this one.

llvm/lib/IR/Type.cpp

584

I'll move it to the header

llvm/include/llvm/IR/DerivedTypes.h

SequentialType(TypeID TID, Type *ElType, uint64_t NumElements)
     ContainedTys = &ContainedType;
     NumContainedTys = 1;
   }
 
 public:
   SequentialType(const SequentialType &) = delete;
   SequentialType &operator=(const SequentialType &) = delete;
 
-  /// For scalable vectors, this will return the minimum number of elements
-  /// in the vector.
-  uint64_t getNumElements() const { return NumElements; }
+  /// For scalable vectors this value is undefined
+  uint64_t getNumElements() const;
   Type *getElementType() const { return ContainedType; }
 
   /// Methods for support type inquiry through isa, cast, and dyn_cast.
   static bool classof(const Type *T) {
     return T->getTypeID() == ArrayTyID || T->getTypeID() == VectorTyID;
   }
 };
 
class VectorType : public SequentialType {
   ///
   /// 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
 
-  VectorType(Type *ElType, unsigned NumEl, bool Scalable = false);
   VectorType(Type *ElType, ElementCount EC);
 
-  // If true, the total number of elements is an unknown multiple of the
-  // minimum 'NumElements' from SequentialType. Otherwise the total number
-  // of elements is exactly equal to 'NumElements'.
-  bool Scalable;
+  // If EC.Scalable is true, the total number of elements is an unknown multiple
+  // of the EC.Min. Otherwise the total number of elements is exactly equal to
+  // SequentialType::getNumElements().
+  ElementCount EC;
 
 public:
   VectorType(const VectorType &) = delete;
   VectorType &operator=(const VectorType &) = delete;
 
   /// 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,
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 {
+#ifndef NDEBUG
+    if (!EC.Scalable) {
       uint64_t MinimumEltCnt = getNumElements();
       assert(MinimumEltCnt <= UINT_MAX && "Too many elements in vector");
-    return { (unsigned)MinimumEltCnt, Scalable };
+      assert(MinimumEltCnt == EC.Min &&
+             "Mismatch between stored element count and getNumElements()");
+    }
+#endif
+    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;
+    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();
   }
 

llvm/lib/IR/Type.cpp

 bool CompositeType::indexValid(unsigned Idx) const {
   if (auto *STy = dyn_cast<StructType>(this))
     return Idx < STy->getNumElements();
   // Sequential types can be indexed by any integer.
   return true;
 }
 
 //===----------------------------------------------------------------------===//
+//                       SequentialType Implementation
+//===----------------------------------------------------------------------===//
+
+uint64_t SequentialType::getNumElements() const {
+#ifndef NDEBUG
+  if (auto *VTy = dyn_cast<VectorType>(this)) {
+    assert(!VTy->isScalable() &&
+           "The length of a scalable vector is unknown at compile time");
+  }
+#endif
+
+  return NumElements;
+}
+
+//===----------------------------------------------------------------------===//
 //                           ArrayType Implementation
 //===----------------------------------------------------------------------===//
 
 ArrayType::ArrayType(Type *ElType, uint64_t NumEl)
   : SequentialType(ArrayTyID, ElType, NumEl) {}
 
 ArrayType *ArrayType::get(Type *ElementType, uint64_t NumElements) {
   assert(isValidElementType(ElementType) && "Invalid type for array element!");
return !ElemTy->isVoidTy() && !ElemTy->isLabelTy() &&
          !ElemTy->isTokenTy();
 }
 
 //===----------------------------------------------------------------------===//
 //                          VectorType Implementation
 //===----------------------------------------------------------------------===//
 
 VectorType::VectorType(Type *ElType, ElementCount EC)
-  : SequentialType(VectorTyID, ElType, EC.Min), Scalable(EC.Scalable) {}
+  : SequentialType(VectorTyID, ElType, EC.Min), EC(EC) {}
 
 VectorType *VectorType::get(Type *ElementType, ElementCount EC) {
   assert(EC.Min > 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.");
 
   LLVMContextImpl *pImpl = ElementType->getContext().pImpl;