[PATCH] D78601: [SVE] Do not store a bool for Scalable in VectorType

[Francesco Petrogalli via Phabricator via llvm-commits]

fpetrogalli added a comment.

Hi @ctetreau ,

thank you for working on this!

Nit: the name `ElementQuantity` seems to refer to some exotic property of a `VectorType`, while all it refers to it is the minimum number of lanes of the vector that are available in the vector. To give a direct meaning to the name of the field, I think you should rename it `MinNumberOfLanes`, or `KnownMinNumberOfLanes`, and rename `getElementQuantity` accordingly. Getting the minimum number of lanes is an operation that makes perfectly sense for both Scalable and Fixed Type vectors, so there should be no controversies in defining this method in VectorType and let scalable and fixed vector types inherit it?

Thank you,

Francesco



================
Comment at: llvm/include/llvm/IR/DerivedTypes.h:408
+  /// The element quantity of this vector. The meaning of this value depends
+  /// the type of vector
+  unsigned ElementQuantity;
----------------
I think it would be great to explain the different meaning here, for both FixedVectorType and ScalableVectorType.

Something like:

```
/// The element quantity of this vector. The meaning of this value depends
/// the type of vector:
/// For FixedVectorType = <m x ty>, ElementQuantity = m
/// Fox ScalableVectorType = <vscale x m x ty>, ElementQuantity = m 
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78601