[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:2222-2227
+    // Allow bitcasting if either the source or destination is a scalable
+    // vector.
+    if (SrcType->isSizelessBuiltinType() || DestType->isSizelessBuiltinType()) {
+      Kind = CK_BitCast;
+      return TC_Success;
+    }
----------------
c-rhodes wrote:
> This is a bit ambiguous, it'll allow bitcasting between things like a fixed predicate and any sizeless type which I don't think makes sense. I think it'll also allow bitcasting between any scalable and GNU vectors regardless of vector length, e.g.:
> 
> ```
> typedef int32_t int32x4_t __attribute__((vector_size(16)));
> 
> void foo() {
> svint32_t s32;
> int32x4_t g32;
> g32 = (int32x4_t)s32;
> s32 = (svint32_t)g32;
> }
> ```
> 
> the above should only work when `-msve-vector-bits=128` but will currently be allowed for any N.
Ah, you're dead right. I think the next diff should fix this, but if there are any more inconsistencies/ambiguities I'll fix them too. :) 


================
Comment at: clang/test/Sema/aarch64-sve-explicit-casts.cpp:1-36
+// RUN: %clang_cc1 -triple aarch64-none-linux-gnu -target-feature +sve -msve-vector-bits=512 -flax-vector-conversions=none -fallow-half-arguments-and-returns -ffreestanding -fsyntax-only -verify %s
+
+#include <arm_sve.h>
+
+// SVE types cannot be C-style casted to one another.
+// "To avoid any ambiguity [between the two operations], the ACLE does not allow C-style casting from one
+// vector type to another." ~ACLE Section 3.4 (Vector Types)
----------------
c-rhodes wrote:
> `clang/test/SemaCXX/sizeless-1.cpp` already contains a test checking C-style casts between distinct sizeless types aren't supported, I'm not sure if adding this is necessary.
ACK -- removed. :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91262