[PATCH] D91262: [AArch64][SVE] Allow C-style casts between fixed-size and scalable vectors

[Joe Ellis via Phabricator via cfe-commits]

joechrisellis added inline comments.


================
Comment at: clang/lib/Sema/SemaCast.cpp:2217-2218
 
-  // Allow reinterpret_casts between vectors of the same size and
-  // between vectors and integers of the same size.
   bool destIsVector = DestType->isVectorType();
----------------
c-rhodes wrote:
> nit: not sure we need to change this
I changed this because the code changes mean that the original comment is incomplete. I.e., we now allow for reinterpret casts VLAT <-> VLST, which is not captured by the comment above.


================
Comment at: clang/lib/Sema/SemaCast.cpp:2763
+  // SVE bitcast. We can bitcast between SVE VLATs and VLSTs, and vice-versa.
+  if (SrcType->isVectorType() || DestType->isVectorType())
+    if (Self.isValidSveBitcast(SrcType, DestType)) {
----------------
peterwaller-arm wrote:
> c-rhodes wrote:
> > I think braces are recommended on the outer if as well, see: https://llvm.org/docs/CodingStandards.html#don-t-use-braces-on-simple-single-statement-bodies-of-if-else-loop-statements
> > 
> > Although I suppose it could also be written as:
> > 
> > ```
> > if ((SrcType->isVectorType() || DestType->isVectorType()) &&
> >     Self.isValidSveBitcast(SrcType, DestType)) {
> >   Kind = CK_BitCast;
> >   return;
> > }```
> I don't understand why this is an || rather than an &&, please can you clarify? I would have expected they must both be vectors.
The sizeless types defined by the ACLE are represented in Clang as `BuiltinType`s rather than `VectorType`s. The sized types are represented as `VectorTypes`. The only possibility for a valid C-style cast is when we're casting VLAT <-> VLST, which is if one of the two is a `VectorType`. :) 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91262