[PATCH] D85794: [WIP][llvm][LV] Replace `unsigned VF` with `ElementCount VF` [NFCI]

[Vineet Kumar via Phabricator via llvm-commits]

vkmr added inline comments.


================
Comment at: llvm/include/llvm/Support/TypeSize.h:87
+  /// One or more elements.
+  bool isVector() const {
+    assert(!(Min == 0 && Scalable) &&
----------------
ctetreau wrote:
> Constraining the constructor doesn't help with the case where a valid `ElementCount` is constructed, and then `Min` set to zero later on.
> 
> Why exactly is `{true, 0}` forbidden? `forall N, N * 0 = 0` after all.
@ctetreau I see your point about constraining the constructor for scalable vectors; `ElementCount` is a counter and 0 is a valid count. Also, however one defines a "vector with 0 elements", it would be the same for fixed and scalar vectors. So, IF the constructor allows constructing  fixed vectors of 0 length, it should also allow scalable vectors of 0 length.

The perspective behind not allowing to have `Min = 0` (for both fixed and scalable vectors) is that `ElementCount`'s only purpose is to count the number of elements in a vector, and a vector with no elements is illegal vector. But that discussion would be out of scope of this patch.

The updated condition for `isVector()` still makes sense though.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85794