[PATCH] D81083: [Clang] Allow "vector_size" applied to Booleans

[Richard Sandiford via Phabricator via cfe-commits]

rsandifo-arm added inline comments.


================
Comment at: clang/docs/LanguageExtensions.rst:473
+architectures.  The size parameter of a boolean vector type is the number of
+bits in the vector (for all non-bool vectors, the number refers to the number
+of bytes in the vector).
----------------
simoll wrote:
> rsandifo-arm wrote:
> > simoll wrote:
> > > lenary wrote:
> > > > It would be nice if this aside about non-bool vectors was more prominently displayed - it's something I hadn't realised before.
> > > Yep. that caught me by surprise too. I'll move that sentence to the paragraph about GCC vectors above.
> > Sorry for the extremely late comment.  Like @lenary I hadn't thought about this.  I'd assumed that the vector woiuld still be a multiple of 8 bits in size, but I agree that's probably too restrictive to be the only option available.
> > 
> > In that case, would it make sense to add a separate attribute instead?  I think it's too surprising to change the units of the existing attribute based on the element type.  Perhaps we should even make it take two parameters: the total number of elements, and the number of bits per element.  That might be more natural for some AVX and SVE combinations.  We wouldn't need to supporrt all combinations from the outset, it's just a question whether we should make the syntax general enough to support it in future.
> > 
> > Perhaps we could do both: support `vector_size` for `bool` using byte sizes (and not allowing subbyte vector lengths), and add a new, more general attribute that allows subbyte lengths and explicit subbyte element sizes.
> > In that case, would it make sense to add a separate attribute instead? I think it's too surprising to change the units of the existing attribute based on the element type. Perhaps we should even make it take two parameters: the total number of elements, and the number of bits per element. That might be more natural for some AVX and SVE combinations. We wouldn't need to supporrt all combinations from the outset, it's just a question whether we should make the syntax general enough to support it in future.
> 
> I guess adding a new attribute makes sense mid to long term. For now, i'd want something that just does the job... ie, what is proposed here. We should absolutely document the semantics of vector_size properly.. it already is counterintuitive (broken, if you ask me).
> 
> 
> > Perhaps we could do both: support vector_size for bool using byte sizes (and not allowing subbyte vector lengths), and add a new, more general attribute that allows subbyte lengths and explicit subbyte element sizes.
> 
> Disallowing subbyte bool vectors actually makes this more complicated because these types are produced implicitly by compares of (legal) vector types. Consider this:
> 
>   typedef int int3 __attribute__((vector_size(3 * sizeof(int))));
>   int3 A = ...;
>   int3 B = ...;
>   auto Z = A < B; // vector compare yielding a `bool vector_size(3)`-typed value.
> 
> 
> Regarding your proposal for a separate subbyte element size and subbyte length, that may or may not make sense but it is surely something that should be discussed more broadly with its own proposal.
> > Perhaps we could do both: support vector_size for bool using byte sizes (and not allowing subbyte vector lengths), and add a new, more general attribute that allows subbyte lengths and explicit subbyte element sizes.
> 
> Disallowing subbyte bool vectors actually makes this more complicated because these types are produced implicitly by compares of (legal) vector types. Consider this:
> 
>   typedef int int3 __attribute__((vector_size(3 * sizeof(int))));
>   int3 A = ...;
>   int3 B = ...;
>   auto Z = A < B; // vector compare yielding a `bool vector_size(3)`-typed value.

Yeah, I understand the need for some way of creating subbyte vectors.  I'm just not sure using the existing `vector_size` attribute would be the best choice for that case.  I.e. the objection wasn't based on “keeping things simple” but more “keeping things consistent“.

That doesn't mean that the above code should be invalid.  It just means that it wouldn't be possible to write the type of Z using the existing `vector_size` attribute.

(FWIW, `vector_size` was originally a GNUism and GCC stil requires vector sizes to be a power of 2, but I realise that isn't relevant here.  And the same principle applies with s/3/4 in the above example anyway.)

> Regarding your proposal for a separate subbyte element size and subbyte length, that may or may not make sense but it is surely something that should be discussed more broadly with its own proposal.

Yeah, I agree any new attribute would need to be discussed more widely.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81083