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



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to