Skip to content

Commit 2400745

Browse files
committed
[ARM][Driver] Ensure NEON is enabled and Disabled correctly
In llvm#130623 support was added for `+nosimd` in the clang driver. Following this PR, it was discovered that, if NEONS is disabled in the command line, it did not disable features that have NEON as a requirement, such as Crypto or AES. To achieve this, clang will now check if SIMD has been disabled when using a NEON supported FPU. If this is the case, it will disable all features that depend on NEON. While working on this Patch, I spotted that for features that rely on NEON, when used, do not enable NEON in the Driver. This meant that when using AES for example, NEON would not be activated. NEON is required when using features such as AES, so this is now enabled when using such features.
1 parent 6c56160 commit 2400745

File tree

5 files changed

+65
-9
lines changed

5 files changed

+65
-9
lines changed

clang/docs/ReleaseNotes.rst

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -601,6 +601,8 @@ Arm and AArch64 Support
601601
- The ``+nosimd`` attribute is now fully supported for ARM. Previously, this had no effect when being used with
602602
ARM targets, however this will now disable NEON instructions being generated. The ``simd`` option is
603603
also now printed when the ``--print-supported-extensions`` option is used.
604+
- NEON is correctly enabled when using features that depend on NEON , and disables all features that depend on
605+
NEON when using ``+nosimd``.
604606

605607
- Support for __ptrauth type qualifier has been added.
606608

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

Lines changed: 52 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -781,6 +781,30 @@ 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 support NEON, then features
785+
// that rely on NEON Instructions should also be disabled. Cases where NEON
786+
// needs activating to support another feature is handled below with the
787+
// crypto feature.
788+
bool HasSimd = false;
789+
const auto ItSimd =
790+
llvm::find_if(llvm::reverse(Features), [](const StringRef F) {
791+
return F.contains("neon");
792+
});
793+
const bool FoundSimd = ItSimd != Features.rend();
794+
const bool FPUSupportsNeon =
795+
(llvm::ARM::FPUNames[FPUKind].NeonSupport == llvm::ARM::NeonSupportLevel::Neon) ||
796+
(llvm::ARM::FPUNames[FPUKind].NeonSupport == llvm::ARM::NeonSupportLevel::Crypto);
797+
if(FoundSimd)
798+
HasSimd = ItSimd->take_front() == "+";
799+
if(!HasSimd && FPUSupportsNeon) {
800+
Features.push_back("-sha2");
801+
Features.push_back("-aes");
802+
Features.push_back("-crypto");
803+
Features.push_back("-dotprod");
804+
Features.push_back("-bf16");
805+
Features.push_back("-imm8");
806+
}
807+
784808
// For Arch >= ARMv8.0 && A or R profile: crypto = sha2 + aes
785809
// Rather than replace within the feature vector, determine whether each
786810
// algorithm is enabled and append this to the end of the vector.
@@ -791,6 +815,9 @@ llvm::ARM::FPUKind arm::getARMTargetFeatures(const Driver &D,
791815
// FIXME: this needs reimplementation after the TargetParser rewrite
792816
bool HasSHA2 = false;
793817
bool HasAES = false;
818+
bool HasBF16 = false;
819+
bool HasDotprod = false;
820+
bool HasI8MM = false;
794821
const auto ItCrypto =
795822
llvm::find_if(llvm::reverse(Features), [](const StringRef F) {
796823
return F.contains("crypto");
@@ -803,12 +830,28 @@ llvm::ARM::FPUKind arm::getARMTargetFeatures(const Driver &D,
803830
llvm::find_if(llvm::reverse(Features), [](const StringRef F) {
804831
return F.contains("crypto") || F.contains("aes");
805832
});
806-
const bool FoundSHA2 = ItSHA2 != Features.rend();
807-
const bool FoundAES = ItAES != Features.rend();
808-
if (FoundSHA2)
833+
const auto ItBF16 =
834+
llvm::find_if(llvm::reverse(Features), [](const StringRef F) {
835+
return F.contains("bf16");
836+
});
837+
const auto ItDotprod =
838+
llvm::find_if(llvm::reverse(Features), [](const StringRef F) {
839+
return F.contains("dotprod");
840+
});
841+
const auto ItI8MM =
842+
llvm::find_if(llvm::reverse(Features), [](const StringRef F) {
843+
return F.contains("i8mm");
844+
});
845+
if (ItSHA2 != Features.rend())
809846
HasSHA2 = ItSHA2->take_front() == "+";
810-
if (FoundAES)
847+
if (ItAES != Features.rend())
811848
HasAES = ItAES->take_front() == "+";
849+
if (ItBF16 != Features.rend())
850+
HasBF16 = ItBF16->take_front() == "+";
851+
if (ItDotprod != Features.rend())
852+
HasDotprod = ItDotprod->take_front() == "+";
853+
if (ItI8MM != Features.rend())
854+
HasI8MM = ItI8MM->take_front() == "+";
812855
if (ItCrypto != Features.rend()) {
813856
if (HasSHA2 && HasAES)
814857
Features.push_back("+crypto");
@@ -823,6 +866,11 @@ llvm::ARM::FPUKind arm::getARMTargetFeatures(const Driver &D,
823866
else
824867
Features.push_back("-aes");
825868
}
869+
// If any of these features are enabled, NEON should also be enabled.
870+
if (HasAES || HasSHA2 || HasBF16 || HasDotprod || HasI8MM)
871+
Features.push_back("+neon");
872+
873+
826874

827875
if (HasSHA2 || HasAES) {
828876
StringRef ArchSuffix = arm::getLLVMArchSuffixForARM(

clang/test/Driver/arm-features.c

Lines changed: 1 addition & 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

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)