Skip to content

[ARM][Driver] Ensure NEON is enabled and disabled correctly #137595

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Apr 29, 2025
Merged

[ARM][Driver] Ensure NEON is enabled and disabled correctly #137595

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
2400745
[ARM][Driver] Ensure NEON is enabled and Disabled correctly
Stylie777 Apr 25, 2025
19f6d04
Formatting fixes
Stylie777 Apr 28, 2025
ab34913
Updates after review comments.
Stylie777 Apr 28, 2025
2bb9077
Change disabling features to use .insert rather than a for loop
Stylie777 Apr 28, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions clang/docs/ReleaseNotes.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -601,6 +601,8 @@ Arm and AArch64 Support
- The ``+nosimd`` attribute is now fully supported for ARM. Previously, this had no effect when being used with
ARM targets, however this will now disable NEON instructions being generated. The ``simd`` option is
also now printed when the ``--print-supported-extensions`` option is used.
- When a feature that depends on NEON (``simd``) is used, NEON is now automatically enabled.
- When NEON is disabled (``+nosimd``), all features that depend on NEON will now be disabled.

- Support for __ptrauth type qualifier has been added.

Expand Down
47 changes: 41 additions & 6 deletions clang/lib/Driver/ToolChains/Arch/ARM.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -781,6 +781,22 @@ llvm::ARM::FPUKind arm::getARMTargetFeatures(const Driver &D,
if (FPUKind == llvm::ARM::FK_FPV5_D16 || FPUKind == llvm::ARM::FK_FPV5_SP_D16)
Features.push_back("-mve.fp");

// If SIMD has been disabled and the selected FPU supports NEON, then features
// that rely on NEON instructions should also be disabled.
bool HasSimd = false;
const auto ItSimd =
llvm::find_if(llvm::reverse(Features),
[](const StringRef F) { return F.contains("neon"); });
const bool FPUSupportsNeon = (llvm::ARM::FPUNames[FPUKind].NeonSupport ==
llvm::ARM::NeonSupportLevel::Neon) ||
(llvm::ARM::FPUNames[FPUKind].NeonSupport ==
llvm::ARM::NeonSupportLevel::Crypto);
if (ItSimd != Features.rend())
HasSimd = ItSimd->starts_with("+");
if (!HasSimd && FPUSupportsNeon)
Features.insert(Features.end(),
{"-sha2", "-aes", "-crypto", "-dotprod", "-bf16", "-imm8"});

// For Arch >= ARMv8.0 && A or R profile: crypto = sha2 + aes
// Rather than replace within the feature vector, determine whether each
// algorithm is enabled and append this to the end of the vector.
Expand All @@ -791,6 +807,9 @@ llvm::ARM::FPUKind arm::getARMTargetFeatures(const Driver &D,
// FIXME: this needs reimplementation after the TargetParser rewrite
bool HasSHA2 = false;
bool HasAES = false;
bool HasBF16 = false;
bool HasDotprod = false;
bool HasI8MM = false;
const auto ItCrypto =
llvm::find_if(llvm::reverse(Features), [](const StringRef F) {
return F.contains("crypto");
Expand All @@ -803,12 +822,25 @@ llvm::ARM::FPUKind arm::getARMTargetFeatures(const Driver &D,
llvm::find_if(llvm::reverse(Features), [](const StringRef F) {
return F.contains("crypto") || F.contains("aes");
});
const bool FoundSHA2 = ItSHA2 != Features.rend();
const bool FoundAES = ItAES != Features.rend();
if (FoundSHA2)
HasSHA2 = ItSHA2->take_front() == "+";
if (FoundAES)
HasAES = ItAES->take_front() == "+";
const auto ItBF16 =
llvm::find_if(llvm::reverse(Features),
[](const StringRef F) { return F.contains("bf16"); });
const auto ItDotprod =
llvm::find_if(llvm::reverse(Features),
[](const StringRef F) { return F.contains("dotprod"); });
const auto ItI8MM =
llvm::find_if(llvm::reverse(Features),
[](const StringRef F) { return F.contains("i8mm"); });
if (ItSHA2 != Features.rend())
HasSHA2 = ItSHA2->starts_with("+");
if (ItAES != Features.rend())
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

starts_with

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

HasAES = ItAES->starts_with("+");
if (ItBF16 != Features.rend())
HasBF16 = ItBF16->starts_with("+");
if (ItDotprod != Features.rend())
HasDotprod = ItDotprod->starts_with("+");
if (ItI8MM != Features.rend())
HasI8MM = ItI8MM->starts_with("+");
if (ItCrypto != Features.rend()) {
if (HasSHA2 && HasAES)
Features.push_back("+crypto");
Expand All @@ -823,6 +855,9 @@ llvm::ARM::FPUKind arm::getARMTargetFeatures(const Driver &D,
else
Features.push_back("-aes");
}
// If any of these features are enabled, NEON should also be enabled.
if (HasAES || HasSHA2 || HasBF16 || HasDotprod || HasI8MM)
Features.push_back("+neon");

if (HasSHA2 || HasAES) {
StringRef ArchSuffix = arm::getLLVMArchSuffixForARM(
Expand Down
10 changes: 9 additions & 1 deletion clang/test/Driver/arm-features.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@
// Check +crypto for M and R profiles:
//
// RUN: %clang -target arm-arm-none-eabi -march=armv8-r+crypto -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-CRYPTO-R %s
// CHECK-CRYPTO-R: "-cc1"{{.*}} "-target-cpu" "generic"{{.*}} "-target-feature" "+sha2" "-target-feature" "+aes"
// CHECK-CRYPTO-R: "-cc1"{{.*}} "-target-cpu" "generic"{{.*}} "-target-feature" "+sha2" "-target-feature" "+aes" "-target-feature" "+neon"
// RUN: %clang -target arm-arm-none-eabi -march=armv8-m.base+crypto -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-NOCRYPTO5 %s
// RUN: %clang -target arm-arm-none-eabi -march=armv8-m.main+crypto -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-NOCRYPTO5 %s
// RUN: %clang -target arm-arm-none-eabi -mcpu=cortex-m23+crypto -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-NOCRYPTO5 %s
Expand All @@ -97,3 +97,11 @@
// CHECK-NOSHA-DAG: "-target-feature" "-sha2"
// CHECK-NOAES-DAG: "-target-feature" "-aes"
//
// Check that adding features that depend on NEON enable the feature
// RUN: %clang -target arm-none-none-eabi -march=armv8-r+sha2 -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-NEON-ENABLED-WITH-FEATURE %s
// RUN: %clang -target arm-none-none-eabi -march=armv8-r+aes -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-NEON-ENABLED-WITH-FEATURE %s
// RUN: %clang -target arm-none-none-eabi -march=armv8-r+crypto -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-NEON-ENABLED-WITH-FEATURE %s
// RUN: %clang -target arm-none-none-eabi -march=armv8-r+dotprod -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-NEON-ENABLED-WITH-FEATURE %s
// RUN: %clang -target arm-none-none-eabi -march=armv8-r+bf16 -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-NEON-ENABLED-WITH-FEATURE %s
// RUN: %clang -target arm-none-none-eabi -march=armv8-r+i8mm -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-NEON-ENABLED-WITH-FEATURE %s
// CHECK-NEON-ENABLED-WITH-FEATURE: "-target-feature" "+neon"
4 changes: 2 additions & 2 deletions clang/test/Driver/arm-mfpu.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -407,7 +407,7 @@
// CHECK-ARM8-ANDROID-FP-DEFAULT-DAG: "-target-feature" "+fp-armv8"
// CHECK-ARM8-ANDROID-FP-DEFAULT-DAG: "-target-feature" "+aes"
// CHECK-ARM8-ANDROID-FP-DEFAULT-DAG: "-target-feature" "+sha2"
// CHECK-ARM8-ANDROID-FP-DEFAULT-NOT: "-target-feature" "+neon"
// CHECK-ARM8-ANDROID-FP-DEFAULT-DAG: "-target-feature" "+neon"

// RUN: %clang -target armv8-linux-android %s -### -c 2>&1 \
// RUN: | FileCheck --check-prefix=CHECK-ARM8-ANDROID-DEFAULT %s
Expand All @@ -416,7 +416,7 @@
// CHECK-ARM8-ANDROID-DEFAULT-DAG: "-target-feature" "+fp-armv8"
// CHECK-ARM8-ANDROID-DEFAULT-DAG: "-target-feature" "+aes"
// CHECK-ARM8-ANDROID-DEFAULT-DAG: "-target-feature" "+sha2"
// CHECK-ARM8-ANDROID-DEFAULT-NOT: "-target-feature" "+neon"
// CHECK-ARM8-ANDROID-DEFAULT-DAG: "-target-feature" "+neon"

// RUN: %clang -target armv7-linux-androideabi21 %s -mfpu=vfp3-d16 -### -c 2>&1 \
// RUN: | FileCheck --check-prefix=CHECK-ARM7-ANDROID-FP-D16 %s
Expand Down
10 changes: 8 additions & 2 deletions clang/test/Preprocessor/arm-target-features.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1036,10 +1036,16 @@
// CHECK-SIMD: #define __ARM_NEON_FP 0x6
// CHECK-SIMD: #define __ARM_NEON__ 1

// Check that on AArch32 appropriate targets, +nosimd correctly disables NEON instructions.
// RUN: %clang -target arm-none-eabi -march=armv8-a+nosimd -mfloat-abi=hard -x c -E -dM -o - %s | FileCheck -check-prefix=CHECK-NOSIMD %s
// Check that on AArch32 appropriate targets, +nosimd correctly disables NEON instructions. All features that rely on NEON should also be disabled.
// RUN: %clang -target arm-none-eabi -march=armv9.6-a+nosimd -mfloat-abi=hard -x c -E -dM -o - %s | FileCheck -check-prefix=CHECK-NOSIMD %s
// RUN: %clang -target arm-none-eabi -mcpu=cortex-r52+nosimd -mfloat-abi=hard -x c -E -dM -o - %s | FileCheck -check-prefix=CHECK-NOSIMD %s
// RUN: %clang -target arm-none-eabi -mcpu=cortex-a57+nosimd -mfloat-abi=hard -x c -E -dM -o - %s | FileCheck -check-prefix=CHECK-NOSIMD %s
// CHECK-NOSIMD-NOT: #define __ARM_FEATURE_BF16 1
// CHECK-NOSIMD-NOT: #define __ARM_FEATURE_BF16_VECTOR_ARITHMETIC 1
// CHECK-NOSIMD-NOT: #define __ARM_FEATURE_AES 1
// CHECK-NOSIMD-NOT: #define __ARM_FEATURE_CRYPTO 1
// CHECK-NOSIMD-NOT: #define __ARM_FEATURE_DOTPROD 1
// CHECK-NOSIMD-NOT: #define __ARM_FEATURE_SHA2 1
// CHECK-NOSIMD-NOT: #define __ARM_NEON 1
// CHECK-NOSIMD-NOT: #define __ARM_NEON_FP 0x6
// CHECK-NOSIMD-NOT: #define __ARM_NEON__ 1