[PATCH] D124850: [Sema][SVE2] Move/simplify Sema testing for SVE2 ACLE builtins

[Sander de Smalen via Phabricator via cfe-commits]

sdesmalen added a comment.

Thanks for working on this @RosieSumpter!



================
Comment at: clang/test/Sema/aarch64-sve2-intrinsics/acle_sve2.cpp:1
+// RUN: %clang_cc1 -triple aarch64-none-linux-gnu -target-feature +sve -target-feature +bf16 -fallow-half-arguments-and-returns -fsyntax-only -verify -verify-ignore-unexpected=error,note %s
+// RUN: %clang_cc1 -DSVE_OVERLOADED_FORMS -triple aarch64-none-linux-gnu -target-feature +sve -target-feature +bf16 -fallow-half-arguments-and-returns -fsyntax-only -verify=overload -verify-ignore-unexpected=error,note %s
----------------
(see my commen later on as well) I'd remove this target feature and move those BF16 tests to a different file.


================
Comment at: clang/test/Sema/aarch64-sve2-intrinsics/acle_sve2.cpp:13
+
+void test_s8(svbool_t pg, int8_t op1, uint64_t op2, const int8_t *op3, uint8_t op4)
+{
----------------
It would be nice if these operands have slightly more descriptive names. Additionally, you may be able to allocate these as globals so that they don't need to be passed as operands to the test function, e.g.

 svbool_t pg;
 float f32;
 double f64;
 int32_t i32;
 int64_t i64;
 float *f32_ptr;
 double *f64_ptr;
 



================
Comment at: clang/test/Sema/aarch64-sve2-intrinsics/acle_sve2.cpp:4992
+
+void test_bf16(const bfloat16_t *op1)
+{
----------------
I think you'll need to move these to a separate test file where the RUN line has only `+sve2` (and not `+bf16`)
We'll probably want to do the same for the SVE bf16 tests


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124850