[PATCH] D32530: [SVE][IR] Scalable Vector IR Type

[Graham Hunter via Phabricator via llvm-commits]

huntergr added inline comments.


================
Comment at: include/llvm/IR/DerivedTypes.h:498
+    uint64_t MinimumEltCnt = getNumElements();
+    assert(MinimumEltCnt <= UINT_MAX && "Too many elements in vector");
+    return { (unsigned)MinimumEltCnt, Scalable };
----------------
rkruppe wrote:
> Nit: This restriction on the range of NumElements is very reasonable, but we should make it a proper invariant of the type and enforce it in `VectorType::get` rather than post-hoc in some accessors.
VectorType::get already has this restriction, at least on most platforms -- the argument to it is 'unsigned', which is usually 32 bits. The ElementCount struct also uses 'unsigned'. It may be worth changing it to an explicit uint32_t. 

I think VectorType originally stored the number of elements as a 32 bit field so was consistent with the interface, but at some point the different SequentialType variants were changed to unify with a single 64 bit size field in the parent class.

I will create an updated patch to check that we don't overflow when using methods like getDoubleElementsVectorType; good catch.


https://reviews.llvm.org/D32530