[PATCH] D87607: [clang][aarch64] Support implicit casts between GNU and SVE vectors

[Eli Friedman via Phabricator via cfe-commits]

efriedma added inline comments.


================
Comment at: clang/lib/AST/ASTContext.cpp:8522
+          return VT->getElementType().getCanonicalType() ==
+                 getBuiltinVectorTypeInfo(BT).ElementType;
       }
----------------
c-rhodes wrote:
> efriedma wrote:
> > We allow casting SVE fixed-width vectors only if the element type is identical, but we allow casting to GNU fixed-width vectors if the vector width is the same?  That seems sort of loose; could allow weird casts, particularly when bool vectors are involved.
> > We allow casting SVE fixed-width vectors only if the element type is identical, but we allow casting to GNU fixed-width vectors if the vector width is the same? That seems sort of loose; could allow weird casts, particularly when bool vectors are involved.
> 
> Fixed-length SVE vectors are created with the attribute and the vector width must match `-msve-vector-bits`, which should guarantee consistent vector widths for a given input. This is checking the vector width is the same and the element type matches when casting between GNU and SVE vectors, although since neither of those types are created by the attribute it's necessary to check the vector width to validate `__ARM_FEATURE_SVE_BITS==N`.
> 
> Predicates are treated differently in the ACLE and it doesn't mention supporting casts between GNU bool vectors and SVE VLS/VLA predicates. I'm also not sure the GNU vector_size extension is defined for bool elements yet, but I did notice Simon Moll is working on this (D81083).
I think you need to check both the size and the element type here?


================
Comment at: clang/test/Sema/attr-arm-sve-vector-bits.c:237
+// Test implicit conversion between SVE and GNU vector is invalid when
+// __ARM_FEATURE_SVE_BITS != N
+#if defined(__ARM_FEATURE_SVE_BITS) && __ARM_FEATURE_SVE_BITS == 512
----------------
This test isn't checking what it says it is; int4 and svint64_t have different element types.


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

https://reviews.llvm.org/D87607