[PATCH] D88233: [clang][aarch64] Address various fixed-length SVE vector operations

[Francesco Petrogalli via Phabricator via cfe-commits]

fpetrogalli added a comment.

Hi @c-rhodes, Peter asked me to take a look at this. LGTM, I only have minor stuff.

In the commit message:

> Arm C Language Extensions (ACLE, version 00bet5, section 3.7.3.3) for SVE [1].

It seems that a reference to [1] is missing:  https://developer.arm.com/documentation/100987/latest
Also, please note that the current version is 00bet6. Nothing seem to have changed from 00bet5 to 00bet6 in terms of this patch, but I think it is worth keeping it up to date with the specs numbering until it is merged into master.

Please fix the commit message before submitting.

Other than that, I only have a couple of nits that I'll let you decide what to do.

Thanks!

Francesco



================
Comment at: clang/test/Sema/attr-arm-sve-vector-bits.c:137
+
+  sel = c ? gs8 : fs8; // expected-error {{cannot combine GNU and SVE vectors in expression, result is ambiguous}}
+  sel = c ? fs8 : gs8; // expected-error {{cannot combine GNU and SVE vectors in expression, result is ambiguous}}
----------------
Nit: I spent some time trying to verify that this was the right test. It would be grate if each test had some reference about the item and page stating the rule that the test is testing. Too bad that the items in the spec don't have a number  that can easily referred to...


================
Comment at: clang/test/Sema/attr-arm-sve-vector-bits.c:141-158
+  ss8 = ss8 + fs8; // expected-error {{cannot combine fixed-length and sizeless SVE vectors in expression, result is ambiguous}}
+  ss8 = ss8 + gs8; // expected-error {{cannot combine GNU and SVE vectors in expression, result is ambiguous}}
+
+  fs8 = fs8 + ss8; // expected-error {{cannot combine fixed-length and sizeless SVE vectors in expression, result is ambiguous}}
+  fs8 = fs8 + gs8; // expected-error {{cannot combine GNU and SVE vectors in expression, result is ambiguous}}
+
+  gs8 = gs8 + ss8; // expected-error {{cannot combine GNU and SVE vectors in expression, result is ambiguous}}
----------------
Nit: should you test more binary operators other than just `+`, like you have done for the vector initialization tests?


================
Comment at: clang/test/Sema/attr-arm-sve-vector-bits.c:300
+
+#if defined(__ARM_FEATURE_SVE_BITS) && __ARM_FEATURE_SVE_BITS == 256
+
----------------
Nit: the `define(...)` seems redundant. Can this be `#if  __ARM_FEATURE_SVE_BITS == 256`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88233