[AArch64] Extend SVE diagnostics. #94976
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The SVE diagnostics were guarded by a FD->hasBody() check that prevented the diagnostic from being emitted for code that still triggered the backend crashes that the errors were meant to avoid, because FD->hasBody() returns false for a function that Clang is currently processing. This is not done for the equivalent RISC-V code, and is not needed for AArch64 either, so remove it. Errors were also emitted in the wrong location, errors were emitted at the called function's location, rather than at the caller's, which meant that just removing the FD->hasBody() check resulted in incomprehensible errors. Change this as well. The aarch64-mangle-sve-vectors.cpp test was using -target-feature wrong which was exposed as a result of these changes. Different target features need to be passed in as different -target-feature options. aarch64-targetattr-arch.c has a test_errors() function that needs to be split in two. Now that svundef_s8() is diagnosed for its use of svint8_t, the "needs target feature sve" diagnostic is no longer emitted, but this affects all calls in the same function. To ensure we still check this for its __crc32cd call, move that into a separate function. Fixes llvm#94766.
llvmbot
added
clang
|
@llvm/pr-subscribers-clang Author: Harald van Dijk (hvdijk) ChangesThe SVE diagnostics were guarded by a FD->hasBody() check that prevented the diagnostic from being emitted for code that still triggered the backend crashes that the errors were meant to avoid, because FD->hasBody() returns false for a function that Clang is currently processing. This is not done for the equivalent RISC-V code, and is not needed for AArch64 either, so remove it. Errors were also emitted in the wrong location, errors were emitted at the called function's location, rather than at the caller's, which meant that just removing the FD->hasBody() check resulted in incomprehensible errors. Change this as well. The aarch64-mangle-sve-vectors.cpp test was using -target-feature wrong which was exposed as a result of these changes. Different target features need to be passed in as different -target-feature options. aarch64-targetattr-arch.c has a test_errors() function that needs to be split in two. Now that svundef_s8() is diagnosed for its use of svint8_t, the "needs target feature sve" diagnostic is no longer emitted, but this affects all calls in the same function. To ensure we still check this for its __crc32cd call, move that into a separate function. Fixes #94766. Full diff: https://github.com/llvm/llvm-project/pull/94976.diff 6 Files Affected:
diff --git a/clang/lib/Sema/Sema.cpp b/clang/lib/Sema/Sema.cpp
index a612dcd4b4d03..907a05a5d1b49 100644
--- a/clang/lib/Sema/Sema.cpp
+++ b/clang/lib/Sema/Sema.cpp
@@ -2093,16 +2093,15 @@ void Sema::checkTypeSupport(QualType Ty, SourceLocation Loc, ValueDecl *D) {
}
// Don't allow SVE types in functions without a SVE target.
- if (Ty->isSVESizelessBuiltinType() && FD && FD->hasBody()) {
+ if (Ty->isSVESizelessBuiltinType() && FD) {
llvm::StringMap<bool> CallerFeatureMap;
Context.getFunctionFeatureMap(CallerFeatureMap, FD);
if (!Builtin::evaluateRequiredTargetFeatures("sve", CallerFeatureMap)) {
if (!Builtin::evaluateRequiredTargetFeatures("sme", CallerFeatureMap))
- Diag(D->getLocation(), diag::err_sve_vector_in_non_sve_target) << Ty;
+ Diag(Loc, diag::err_sve_vector_in_non_sve_target) << Ty;
else if (!IsArmStreamingFunction(FD,
/*IncludeLocallyStreaming=*/true)) {
- Diag(D->getLocation(), diag::err_sve_vector_in_non_streaming_function)
- << Ty;
+ Diag(Loc, diag::err_sve_vector_in_non_streaming_function) << Ty;
}
}
}
diff --git a/clang/test/CodeGen/aarch64-targetattr-arch.c b/clang/test/CodeGen/aarch64-targetattr-arch.c
index ed731d0378625..5de73d6027845 100644
--- a/clang/test/CodeGen/aarch64-targetattr-arch.c
+++ b/clang/test/CodeGen/aarch64-targetattr-arch.c
@@ -29,14 +29,18 @@ float16_t test_fp16_on_v9(float16_t x, float16_t y)
return vabdh_f16(x, y);
}
-void test_errors()
+void test_error1()
{
#ifdef HAS8
// expected-error@+2{{always_inline function '__crc32cd' requires target feature 'crc'}}
#endif
__crc32cd(1, 1);
+}
+
+void test_error2()
+{
#if defined(HAS8) || defined(HAS81)
-// expected-error@+2{{'svundef_s8' needs target feature sve}}
+// expected-error@+2{{SVE vector type 'svint8_t' (aka '__SVInt8_t') cannot be used in a target without sve}}
#endif
svundef_s8();
}
diff --git a/clang/test/CodeGen/aarch64_neon_sve_bridge_intrinsics/target.c b/clang/test/CodeGen/aarch64_neon_sve_bridge_intrinsics/target.c
index a08c452fdc7fe..bc5f01e7ce0ff 100644
--- a/clang/test/CodeGen/aarch64_neon_sve_bridge_intrinsics/target.c
+++ b/clang/test/CodeGen/aarch64_neon_sve_bridge_intrinsics/target.c
@@ -18,15 +18,23 @@ void target_svebf16(svbfloat16_t t, bfloat16x8_t m) {
}
void base(int8x16_t n, bfloat16x8_t m) {
- // expected-error@+1 {{'svundef_s8' needs target feature sve}}
- svset_neonq_s8(svundef_s8(), n); // expected-error {{'svset_neonq_s8' needs target feature sve}}
- // expected-error@+1 {{'svundef_s8' needs target feature sve}}
- svget_neonq_s8(svundef_s8()); // expected-error {{'svget_neonq_s8' needs target feature sve}}
- svdup_neonq_s8(n); // expected-error {{'svdup_neonq_s8' needs target feature sve}}
+ // expected-error@+3 {{SVE vector type 'svint8_t' (aka '__SVInt8_t') cannot be used in a target without sve}}
+ // expected-error@+2 {{SVE vector type 'svint8_t' (aka '__SVInt8_t') cannot be used in a target without sve}}
+ // expected-error@+1 {{SVE vector type 'svint8_t' (aka '__SVInt8_t') cannot be used in a target without sve}}
+ svset_neonq_s8(svundef_s8(), n);
+ // expected-error@+2 {{SVE vector type 'svint8_t' (aka '__SVInt8_t') cannot be used in a target without sve}}
+ // expected-error@+1 {{SVE vector type 'svint8_t' (aka '__SVInt8_t') cannot be used in a target without sve}}
+ svget_neonq_s8(svundef_s8());
+ // expected-error@+1 {{SVE vector type 'svint8_t' (aka '__SVInt8_t') cannot be used in a target without sve}}
+ svdup_neonq_s8(n);
- // expected-error@+1 {{'svundef_bf16' needs target feature sve}}
- svset_neonq_bf16(svundef_bf16(), m); // expected-error {{'svset_neonq_bf16' needs target feature sve,bf16}}
- // expected-error@+1 {{'svundef_bf16' needs target feature sve}}
- svget_neonq_bf16(svundef_bf16()); // expected-error {{'svget_neonq_bf16' needs target feature sve,bf16}}
- svdup_neonq_bf16(m); // expected-error {{'svdup_neonq_bf16' needs target feature sve,bf16}}
+ // expected-error@+3 {{SVE vector type 'svbfloat16_t' (aka '__SVBfloat16_t') cannot be used in a target without sve}}
+ // expected-error@+2 {{SVE vector type 'svbfloat16_t' (aka '__SVBfloat16_t') cannot be used in a target without sve}}
+ // expected-error@+1 {{SVE vector type 'svbfloat16_t' (aka '__SVBfloat16_t') cannot be used in a target without sve}}
+ svset_neonq_bf16(svundef_bf16(), m);
+ // expected-error@+2 {{SVE vector type 'svbfloat16_t' (aka '__SVBfloat16_t') cannot be used in a target without sve}}
+ // expected-error@+1 {{SVE vector type 'svbfloat16_t' (aka '__SVBfloat16_t') cannot be used in a target without sve}}
+ svget_neonq_bf16(svundef_bf16());
+ // expected-error@+1 {{SVE vector type 'svbfloat16_t' (aka '__SVBfloat16_t') cannot be used in a target without sve}}
+ svdup_neonq_bf16(m);
}
diff --git a/clang/test/CodeGenCXX/aarch64-mangle-sve-vectors.cpp b/clang/test/CodeGenCXX/aarch64-mangle-sve-vectors.cpp
index f87a254f5ae72..752b2beca3881 100644
--- a/clang/test/CodeGenCXX/aarch64-mangle-sve-vectors.cpp
+++ b/clang/test/CodeGenCXX/aarch64-mangle-sve-vectors.cpp
@@ -1,8 +1,8 @@
// NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py UTC_ARGS: --version 3
// RUN: %clang_cc1 -fclang-abi-compat=latest -triple aarch64-none-linux-gnu %s -emit-llvm -o - \
-// RUN: -target-feature +sve,+bf16 | FileCheck %s
+// RUN: -target-feature +sve -target-feature +bf16 | FileCheck %s
// RUN: %clang_cc1 -fclang-abi-compat=latest -triple aarch64-none-linux-gnu %s -emit-llvm -o - \
-// RUN: -target-feature +sve,+bf16 -fclang-abi-compat=17 | FileCheck %s --check-prefix=COMPAT_17
+// RUN: -target-feature +sve -target-feature +bf16 -fclang-abi-compat=17 | FileCheck %s --check-prefix=COMPAT_17
void f(__SVInt8_t, __SVInt8_t);
void f(__SVInt16_t, __SVInt16_t);
diff --git a/clang/test/Sema/aarch64-sme2-sve2p1-diagnostics.c b/clang/test/Sema/aarch64-sme2-sve2p1-diagnostics.c
index 2012221b48041..97cdd6d9fb3a8 100644
--- a/clang/test/Sema/aarch64-sme2-sve2p1-diagnostics.c
+++ b/clang/test/Sema/aarch64-sme2-sve2p1-diagnostics.c
@@ -10,6 +10,9 @@
// expected-error@+2 {{SVE vector type 'svuint8x2_t' (aka '__clang_svuint8x2_t') cannot be used in a non-streaming function}}
__attribute__((target("+sme2")))
svuint8x2_t sme2_or_sve2p1_intrinsic_test_sme2_invalid(svcount_t png, const uint8_t *rn) {
+ // expected-error@+4 {{SVE vector type 'svcount_t' (aka '__SVCount_t') cannot be used in a non-streaming function}}
+ // expected-error@+3 {{SVE vector type 'svuint8x2_t' (aka '__clang_svuint8x2_t') cannot be used in a non-streaming function}}
+ // expected-error@+2 {{SVE vector type 'svcount_t' (aka '__SVCount_t') cannot be used in a non-streaming function}}
// expected-warning@+1 {{builtin call has undefined behaviour when called from a non-streaming function}}
return svldnt1_u8_x2(png, rn);
}
diff --git a/clang/test/Sema/arm-sve-target.cpp b/clang/test/Sema/arm-sve-target.cpp
index a753f772cc372..1567475e681da 100644
--- a/clang/test/Sema/arm-sve-target.cpp
+++ b/clang/test/Sema/arm-sve-target.cpp
@@ -23,7 +23,7 @@ void test_var_target3() {
__SVFloat32_t other_ret();
__SVFloat32_t test_ret() { // expected-error {{SVE vector type '__SVFloat32_t' cannot be used in a target without sve}}
- return other_ret();
+ return other_ret(); // expected-error {{SVE vector type '__SVFloat32_t' cannot be used in a target without sve}}
}
__attribute__((target("sve")))
|
sdesmalen-arm
approved these changes
Jun 14, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The SVE diagnostics were guarded by a FD->hasBody() check that prevented the diagnostic from being emitted for code that still triggered the backend crashes that the errors were meant to avoid, because FD->hasBody() returns false for a function that Clang is currently processing. This is not done for the equivalent RISC-V code, and is not needed for AArch64 either, so remove it.
Errors were also emitted in the wrong location, errors were emitted at the called function's location, rather than at the caller's, which meant that just removing the FD->hasBody() check resulted in incomprehensible errors. Change this as well.
The aarch64-mangle-sve-vectors.cpp test was using -target-feature wrong which was exposed as a result of these changes. Different target features need to be passed in as different -target-feature options.
aarch64-targetattr-arch.c has a test_errors() function that needs to be split in two. Now that svundef_s8() is diagnosed for its use of svint8_t, the "needs target feature sve" diagnostic is no longer emitted, but this affects all calls in the same function. To ensure we still check this for its __crc32cd call, move that into a separate function.
Fixes #94766.