Skip to content

Commit ace8cea

Browse files
Stylie777Stylie777
authored
[ARM][Driver] Ensure NEON is enabled and disabled correctly (#137595)
In #130623 support was added for `+nosimd` in the clang driver. Following this PR, it was discovered that, if NEON is disabled in the command line, it did not disable features that depend on this, such as Crypto or AES. To achieve this, This PR does the following: - Ensure that disabling NEON (e.g., via +nosimd) also disables dependent features like Crypto and AES. - Update the driver to automatically enable NEON when enabling features that require it (e.g., AES). This fixes inconsistent behavior where features relying on NEON could be enabled without NEON itself being active, or where disabling NEON left dependent features incorrectly enabled.
1 parent a618ae2 commit ace8cea

File tree

5 files changed

+62
-11
lines changed

5 files changed

+62
-11
lines changed

‎clang/docs/ReleaseNotes.rst

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -608,6 +608,8 @@ Arm and AArch64 Support
608608
- The ``+nosimd`` attribute is now fully supported for ARM. Previously, this had no effect when being used with
609609
ARM targets, however this will now disable NEON instructions being generated. The ``simd`` option is
610610
also now printed when the ``--print-supported-extensions`` option is used.
611+
- When a feature that depends on NEON (``simd``) is used, NEON is now automatically enabled.
612+
- When NEON is disabled (``+nosimd``), all features that depend on NEON will now be disabled.
611613

612614
- Support for __ptrauth type qualifier has been added.
613615

‎clang/lib/Driver/ToolChains/Arch/ARM.cpp

Lines changed: 41 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -781,6 +781,22 @@ llvm::ARM::FPUKind arm::getARMTargetFeatures(const Driver &D,
781781
if (FPUKind == llvm::ARM::FK_FPV5_D16 || FPUKind == llvm::ARM::FK_FPV5_SP_D16)
782782
Features.push_back("-mve.fp");
783783

784+
// If SIMD has been disabled and the selected FPU supports NEON, then features
785+
// that rely on NEON instructions should also be disabled.
786+
bool HasSimd = false;
787+
const auto ItSimd =
788+
llvm::find_if(llvm::reverse(Features),
789+
[](const StringRef F) { return F.contains("neon"); });
790+
const bool FPUSupportsNeon = (llvm::ARM::FPUNames[FPUKind].NeonSupport ==
791+
llvm::ARM::NeonSupportLevel::Neon) ||
792+
(llvm::ARM::FPUNames[FPUKind].NeonSupport ==
793+
llvm::ARM::NeonSupportLevel::Crypto);
794+
if (ItSimd != Features.rend())
795+
HasSimd = ItSimd->starts_with("+");
796+
if (!HasSimd && FPUSupportsNeon)
797+
Features.insert(Features.end(),
798+
{"-sha2", "-aes", "-crypto", "-dotprod", "-bf16", "-imm8"});
799+
784800
// For Arch >= ARMv8.0 && A or R profile: crypto = sha2 + aes
785801
// Rather than replace within the feature vector, determine whether each
786802
// algorithm is enabled and append this to the end of the vector.
@@ -791,6 +807,9 @@ llvm::ARM::FPUKind arm::getARMTargetFeatures(const Driver &D,
791807
// FIXME: this needs reimplementation after the TargetParser rewrite
792808
bool HasSHA2 = false;
793809
bool HasAES = false;
810+
bool HasBF16 = false;
811+
bool HasDotprod = false;
812+
bool HasI8MM = false;
794813
const auto ItCrypto =
795814
llvm::find_if(llvm::reverse(Features), [](const StringRef F) {
796815
return F.contains("crypto");
@@ -803,12 +822,25 @@ llvm::ARM::FPUKind arm::getARMTargetFeatures(const Driver &D,
803822
llvm::find_if(llvm::reverse(Features), [](const StringRef F) {
804823
return F.contains("crypto") || F.contains("aes");
805824
});
806-
const bool FoundSHA2 = ItSHA2 != Features.rend();
807-
const bool FoundAES = ItAES != Features.rend();
808-
if (FoundSHA2)
809-
HasSHA2 = ItSHA2->take_front() == "+";
810-
if (FoundAES)
811-
HasAES = ItAES->take_front() == "+";
825+
const auto ItBF16 =
826+
llvm::find_if(llvm::reverse(Features),
827+
[](const StringRef F) { return F.contains("bf16"); });
828+
const auto ItDotprod =
829+
llvm::find_if(llvm::reverse(Features),
830+
[](const StringRef F) { return F.contains("dotprod"); });
831+
const auto ItI8MM =
832+
llvm::find_if(llvm::reverse(Features),
833+
[](const StringRef F) { return F.contains("i8mm"); });
834+
if (ItSHA2 != Features.rend())
835+
HasSHA2 = ItSHA2->starts_with("+");
836+
if (ItAES != Features.rend())
837+
HasAES = ItAES->starts_with("+");
838+
if (ItBF16 != Features.rend())
839+
HasBF16 = ItBF16->starts_with("+");
840+
if (ItDotprod != Features.rend())
841+
HasDotprod = ItDotprod->starts_with("+");
842+
if (ItI8MM != Features.rend())
843+
HasI8MM = ItI8MM->starts_with("+");
812844
if (ItCrypto != Features.rend()) {
813845
if (HasSHA2 && HasAES)
814846
Features.push_back("+crypto");
@@ -823,6 +855,9 @@ llvm::ARM::FPUKind arm::getARMTargetFeatures(const Driver &D,
823855
else
824856
Features.push_back("-aes");
825857
}
858+
// If any of these features are enabled, NEON should also be enabled.
859+
if (HasAES || HasSHA2 || HasBF16 || HasDotprod || HasI8MM)
860+
Features.push_back("+neon");
826861

827862
if (HasSHA2 || HasAES) {
828863
StringRef ArchSuffix = arm::getLLVMArchSuffixForARM(

‎clang/test/Driver/arm-features.c

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,7 @@
7474
// Check +crypto for M and R profiles:
7575
//
7676
// RUN: %clang -target arm-arm-none-eabi -march=armv8-r+crypto -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-CRYPTO-R %s
77-
// CHECK-CRYPTO-R: "-cc1"{{.*}} "-target-cpu" "generic"{{.*}} "-target-feature" "+sha2" "-target-feature" "+aes"
77+
// CHECK-CRYPTO-R: "-cc1"{{.*}} "-target-cpu" "generic"{{.*}} "-target-feature" "+sha2" "-target-feature" "+aes" "-target-feature" "+neon"
7878
// RUN: %clang -target arm-arm-none-eabi -march=armv8-m.base+crypto -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-NOCRYPTO5 %s
7979
// RUN: %clang -target arm-arm-none-eabi -march=armv8-m.main+crypto -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-NOCRYPTO5 %s
8080
// RUN: %clang -target arm-arm-none-eabi -mcpu=cortex-m23+crypto -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-NOCRYPTO5 %s
@@ -97,3 +97,11 @@
9797
// CHECK-NOSHA-DAG: "-target-feature" "-sha2"
9898
// CHECK-NOAES-DAG: "-target-feature" "-aes"
9999
//
100+
// Check that adding features that depend on NEON enable the feature
101+
// RUN: %clang -target arm-none-none-eabi -march=armv8-r+sha2 -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-NEON-ENABLED-WITH-FEATURE %s
102+
// RUN: %clang -target arm-none-none-eabi -march=armv8-r+aes -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-NEON-ENABLED-WITH-FEATURE %s
103+
// RUN: %clang -target arm-none-none-eabi -march=armv8-r+crypto -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-NEON-ENABLED-WITH-FEATURE %s
104+
// RUN: %clang -target arm-none-none-eabi -march=armv8-r+dotprod -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-NEON-ENABLED-WITH-FEATURE %s
105+
// RUN: %clang -target arm-none-none-eabi -march=armv8-r+bf16 -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-NEON-ENABLED-WITH-FEATURE %s
106+
// RUN: %clang -target arm-none-none-eabi -march=armv8-r+i8mm -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-NEON-ENABLED-WITH-FEATURE %s
107+
// CHECK-NEON-ENABLED-WITH-FEATURE: "-target-feature" "+neon"

‎clang/test/Driver/arm-mfpu.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -407,7 +407,7 @@
407407
// CHECK-ARM8-ANDROID-FP-DEFAULT-DAG: "-target-feature" "+fp-armv8"
408408
// CHECK-ARM8-ANDROID-FP-DEFAULT-DAG: "-target-feature" "+aes"
409409
// CHECK-ARM8-ANDROID-FP-DEFAULT-DAG: "-target-feature" "+sha2"
410-
// CHECK-ARM8-ANDROID-FP-DEFAULT-NOT: "-target-feature" "+neon"
410+
// CHECK-ARM8-ANDROID-FP-DEFAULT-DAG: "-target-feature" "+neon"
411411

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

421421
// RUN: %clang -target armv7-linux-androideabi21 %s -mfpu=vfp3-d16 -### -c 2>&1 \
422422
// RUN: | FileCheck --check-prefix=CHECK-ARM7-ANDROID-FP-D16 %s

‎clang/test/Preprocessor/arm-target-features.c

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1036,10 +1036,16 @@
10361036
// CHECK-SIMD: #define __ARM_NEON_FP 0x6
10371037
// CHECK-SIMD: #define __ARM_NEON__ 1
10381038

1039-
// Check that on AArch32 appropriate targets, +nosimd correctly disables NEON instructions.
1040-
// 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
1039+
// Check that on AArch32 appropriate targets, +nosimd correctly disables NEON instructions. All features that rely on NEON should also be disabled.
1040+
// 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
10411041
// 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
10421042
// 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
1043+
// CHECK-NOSIMD-NOT: #define __ARM_FEATURE_BF16 1
1044+
// CHECK-NOSIMD-NOT: #define __ARM_FEATURE_BF16_VECTOR_ARITHMETIC 1
1045+
// CHECK-NOSIMD-NOT: #define __ARM_FEATURE_AES 1
1046+
// CHECK-NOSIMD-NOT: #define __ARM_FEATURE_CRYPTO 1
1047+
// CHECK-NOSIMD-NOT: #define __ARM_FEATURE_DOTPROD 1
1048+
// CHECK-NOSIMD-NOT: #define __ARM_FEATURE_SHA2 1
10431049
// CHECK-NOSIMD-NOT: #define __ARM_NEON 1
10441050
// CHECK-NOSIMD-NOT: #define __ARM_NEON_FP 0x6
10451051
// CHECK-NOSIMD-NOT: #define __ARM_NEON__ 1

0 commit comments

Comments
 (0)