[PATCH] D82182: [AArch64][SVE] Add bfloat16 support to perm and select intrinsics

[Cullen Rhodes via Phabricator via cfe-commits]

c-rhodes added inline comments.


================
Comment at: clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_sel.c:2
 // REQUIRES: aarch64-registered-target
-// RUN: %clang_cc1 -D__ARM_FEATURE_SVE -triple aarch64-none-linux-gnu -target-feature +sve -fallow-half-arguments-and-returns -S -O1 -Werror -Wall -emit-llvm -o - %s | FileCheck %s
-// RUN: %clang_cc1 -D__ARM_FEATURE_SVE -DSVE_OVERLOADED_FORMS -triple aarch64-none-linux-gnu -target-feature +sve -fallow-half-arguments-and-returns -S -O1 -Werror -Wall -emit-llvm -o - %s | FileCheck %s
-// RUN: %clang_cc1 -D__ARM_FEATURE_SVE -triple aarch64-none-linux-gnu -target-feature +sve -fallow-half-arguments-and-returns -S -O1 -Werror -Wall -o - %s >/dev/null 2>%t
+// RUN: %clang_cc1 -D__ARM_FEATURE_SVE -D__ARM_FEATURE_BF16_SCALAR_ARITHMETIC -D__ARM_FEATURE_SVE_BF16 -triple aarch64-none-linux-gnu -target-feature +sve -target-feature +bf16 -fallow-half-arguments-and-returns -S -O1 -Werror -Wall -emit-llvm -o - %s | FileCheck %s
+// RUN: %clang_cc1 -D__ARM_FEATURE_SVE -D__ARM_FEATURE_BF16_SCALAR_ARITHMETIC -D__ARM_FEATURE_SVE_BF16 -DSVE_OVERLOADED_FORMS -triple aarch64-none-linux-gnu -target-feature +sve -target-feature +bf16 -fallow-half-arguments-and-returns -S -O1 -Werror -Wall -emit-llvm -o - %s | FileCheck %s
----------------
sdesmalen wrote:
> Can you move the clang bfloat tests to separate files and add a RUN line similar to what we've done for the sve2 tests (to check that we get a diagnostic if +bf16 is not specified) ?
I can move the bfloat tests to separate files but I'm not sure about the RUN line, if `+bf16` is omitted we get the following:

```/home/culrho01/llvm-project/build/bin/clang -cc1 -internal-isystem /home/culrho01/llvm-project/build/lib/clang/11.0.0/include -nostdsysteminc -D__ARM_FEATURE_SVE -D__ARM_FEATURE_BF16_SCALAR_ARITHMETIC -D__ARM_FEATURE_SVE_BF16 -triple aarch64-none-linux-gnu -target-feature +sve -fallow-half-arguments-and-returns -fsyntax-only -verify /home/culrho01/llvm-project/clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_rev-bfloat.c
error: no expected directives found: consider use of 'expected-no-diagnostics'
error: 'error' diagnostics seen but not expected:
  File /home/culrho01/llvm-project/build/lib/clang/11.0.0/include/arm_bf16.h Line 14: __bf16 is not supported on this target
  File /home/culrho01/llvm-project/build/lib/clang/11.0.0/include/arm_sve.h Line 52: __bf16 is not supported on this target
3 errors generated.```

Whereas I think the desired behaviour we want to test as we do for sve2 is checking the intrinsics are guarded with the right feature flag, which at the moment would be omitting `-D__ARM_FEATURE_SVE_BF16` from the RUN line, until `+bf16` implies `-D__ARM_FEATURE_SVE_BF16` anyway, which is when the ACLE is fully complete. Should I do that? I guess we'd want to update this RUN line to omit `+bf16` when it implies the feature macro


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82182