https://github.com/Stylie777 updated
https://github.com/llvm/llvm-project/pull/137595
>From 2400745e8724ea8616f68899fb5b6a71a1cf522e Mon Sep 17 00:00:00 2001
From: Jack Styles <jack.sty...@arm.com>
Date: Fri, 25 Apr 2025 14:57:40 +0100
Subject: [PATCH 1/4] [ARM][Driver] Ensure NEON is enabled and Disabled
correctly
In #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.
---
clang/docs/ReleaseNotes.rst | 2 +
clang/lib/Driver/ToolChains/Arch/ARM.cpp | 56 +++++++++++++++++--
clang/test/Driver/arm-features.c | 2 +-
clang/test/Driver/arm-mfpu.c | 4 +-
clang/test/Preprocessor/arm-target-features.c | 10 +++-
5 files changed, 65 insertions(+), 9 deletions(-)
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index bec670e573ca6..b19c6faae989a 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -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.
+- NEON is correctly enabled when using features that depend on NEON , and
disables all features that depend on
+ NEON when using ``+nosimd``.
- Support for __ptrauth type qualifier has been added.
diff --git a/clang/lib/Driver/ToolChains/Arch/ARM.cpp
b/clang/lib/Driver/ToolChains/Arch/ARM.cpp
index 5084058b3fef0..61a98475de93f 100644
--- a/clang/lib/Driver/ToolChains/Arch/ARM.cpp
+++ b/clang/lib/Driver/ToolChains/Arch/ARM.cpp
@@ -781,6 +781,30 @@ 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 support NEON, then features
+ // that rely on NEON Instructions should also be disabled. Cases where NEON
+ // needs activating to support another feature is handled below with the
+ // crypto feature.
+ bool HasSimd = false;
+ const auto ItSimd =
+ llvm::find_if(llvm::reverse(Features), [](const StringRef F) {
+ return F.contains("neon");
+ });
+ const bool FoundSimd = ItSimd != Features.rend();
+ const bool FPUSupportsNeon =
+ (llvm::ARM::FPUNames[FPUKind].NeonSupport ==
llvm::ARM::NeonSupportLevel::Neon) ||
+ (llvm::ARM::FPUNames[FPUKind].NeonSupport ==
llvm::ARM::NeonSupportLevel::Crypto);
+ if(FoundSimd)
+ HasSimd = ItSimd->take_front() == "+";
+ if(!HasSimd && FPUSupportsNeon) {
+ Features.push_back("-sha2");
+ Features.push_back("-aes");
+ Features.push_back("-crypto");
+ Features.push_back("-dotprod");
+ Features.push_back("-bf16");
+ Features.push_back("-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.
@@ -791,6 +815,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");
@@ -803,12 +830,28 @@ 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)
+ 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->take_front() == "+";
- if (FoundAES)
+ if (ItAES != Features.rend())
HasAES = ItAES->take_front() == "+";
+ if (ItBF16 != Features.rend())
+ HasBF16 = ItBF16->take_front() == "+";
+ if (ItDotprod != Features.rend())
+ HasDotprod = ItDotprod->take_front() == "+";
+ if (ItI8MM != Features.rend())
+ HasI8MM = ItI8MM->take_front() == "+";
if (ItCrypto != Features.rend()) {
if (HasSHA2 && HasAES)
Features.push_back("+crypto");
@@ -823,6 +866,11 @@ 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(
diff --git a/clang/test/Driver/arm-features.c b/clang/test/Driver/arm-features.c
index eb424f5f61116..5f837667a2614 100644
--- a/clang/test/Driver/arm-features.c
+++ b/clang/test/Driver/arm-features.c
@@ -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
diff --git a/clang/test/Driver/arm-mfpu.c b/clang/test/Driver/arm-mfpu.c
index 640e1b35c84b8..b97cc7bf12545 100644
--- a/clang/test/Driver/arm-mfpu.c
+++ b/clang/test/Driver/arm-mfpu.c
@@ -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
@@ -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
diff --git a/clang/test/Preprocessor/arm-target-features.c
b/clang/test/Preprocessor/arm-target-features.c
index 7b2a0ecdf9040..0f9063a27371e 100644
--- a/clang/test/Preprocessor/arm-target-features.c
+++ b/clang/test/Preprocessor/arm-target-features.c
@@ -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
>From 19f6d04b2cc7ac2ee10aa5226de82483e79b83ef Mon Sep 17 00:00:00 2001
From: Jack Styles <jack.sty...@arm.com>
Date: Mon, 28 Apr 2025 09:38:58 +0100
Subject: [PATCH 2/4] Formatting fixes
---
clang/lib/Driver/ToolChains/Arch/ARM.cpp | 35 ++++++++++--------------
1 file changed, 15 insertions(+), 20 deletions(-)
diff --git a/clang/lib/Driver/ToolChains/Arch/ARM.cpp
b/clang/lib/Driver/ToolChains/Arch/ARM.cpp
index 61a98475de93f..a0db34a3ad093 100644
--- a/clang/lib/Driver/ToolChains/Arch/ARM.cpp
+++ b/clang/lib/Driver/ToolChains/Arch/ARM.cpp
@@ -787,16 +787,16 @@ llvm::ARM::FPUKind arm::getARMTargetFeatures(const Driver
&D,
// crypto feature.
bool HasSimd = false;
const auto ItSimd =
- llvm::find_if(llvm::reverse(Features), [](const StringRef F) {
- return F.contains("neon");
- });
+ llvm::find_if(llvm::reverse(Features),
+ [](const StringRef F) { return F.contains("neon"); });
const bool FoundSimd = ItSimd != Features.rend();
- const bool FPUSupportsNeon =
- (llvm::ARM::FPUNames[FPUKind].NeonSupport ==
llvm::ARM::NeonSupportLevel::Neon) ||
- (llvm::ARM::FPUNames[FPUKind].NeonSupport ==
llvm::ARM::NeonSupportLevel::Crypto);
- if(FoundSimd)
- HasSimd = ItSimd->take_front() == "+";
- if(!HasSimd && FPUSupportsNeon) {
+ const bool FPUSupportsNeon = (llvm::ARM::FPUNames[FPUKind].NeonSupport ==
+ llvm::ARM::NeonSupportLevel::Neon) ||
+ (llvm::ARM::FPUNames[FPUKind].NeonSupport ==
+ llvm::ARM::NeonSupportLevel::Crypto);
+ if (FoundSimd)
+ HasSimd = ItSimd->take_front() == "+";
+ if (!HasSimd && FPUSupportsNeon) {
Features.push_back("-sha2");
Features.push_back("-aes");
Features.push_back("-crypto");
@@ -831,17 +831,14 @@ llvm::ARM::FPUKind arm::getARMTargetFeatures(const Driver
&D,
return F.contains("crypto") || F.contains("aes");
});
const auto ItBF16 =
- llvm::find_if(llvm::reverse(Features), [](const StringRef F) {
- return F.contains("bf16");
- });
+ 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");
- });
+ 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");
- });
+ llvm::find_if(llvm::reverse(Features),
+ [](const StringRef F) { return F.contains("i8mm"); });
if (ItSHA2 != Features.rend())
HasSHA2 = ItSHA2->take_front() == "+";
if (ItAES != Features.rend())
@@ -870,8 +867,6 @@ llvm::ARM::FPUKind arm::getARMTargetFeatures(const Driver
&D,
if (HasAES || HasSHA2 || HasBF16 || HasDotprod || HasI8MM)
Features.push_back("+neon");
-
-
if (HasSHA2 || HasAES) {
StringRef ArchSuffix = arm::getLLVMArchSuffixForARM(
arm::getARMTargetCPU(CPUName, ArchName, Triple), ArchName, Triple);
>From ab34913b1d2afceb3ff26114d1267ad8a927399c Mon Sep 17 00:00:00 2001
From: Jack Styles <jack.sty...@arm.com>
Date: Mon, 28 Apr 2025 14:16:13 +0100
Subject: [PATCH 3/4] Updates after review comments.
---
clang/docs/ReleaseNotes.rst | 4 +--
clang/lib/Driver/ToolChains/Arch/ARM.cpp | 32 +++++++++---------------
clang/test/Driver/arm-features.c | 8 ++++++
3 files changed, 22 insertions(+), 22 deletions(-)
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index b19c6faae989a..15b4eb540f831 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -601,8 +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.
-- NEON is correctly enabled when using features that depend on NEON , and
disables all features that depend on
- NEON when using ``+nosimd``.
+- 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.
diff --git a/clang/lib/Driver/ToolChains/Arch/ARM.cpp
b/clang/lib/Driver/ToolChains/Arch/ARM.cpp
index a0db34a3ad093..451d6e7581f50 100644
--- a/clang/lib/Driver/ToolChains/Arch/ARM.cpp
+++ b/clang/lib/Driver/ToolChains/Arch/ARM.cpp
@@ -781,29 +781,21 @@ 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 support NEON, then features
- // that rely on NEON Instructions should also be disabled. Cases where NEON
- // needs activating to support another feature is handled below with the
- // crypto feature.
+ // 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 FoundSimd = ItSimd != Features.rend();
const bool FPUSupportsNeon = (llvm::ARM::FPUNames[FPUKind].NeonSupport ==
llvm::ARM::NeonSupportLevel::Neon) ||
(llvm::ARM::FPUNames[FPUKind].NeonSupport ==
llvm::ARM::NeonSupportLevel::Crypto);
- if (FoundSimd)
- HasSimd = ItSimd->take_front() == "+";
- if (!HasSimd && FPUSupportsNeon) {
- Features.push_back("-sha2");
- Features.push_back("-aes");
- Features.push_back("-crypto");
- Features.push_back("-dotprod");
- Features.push_back("-bf16");
- Features.push_back("-imm8");
- }
+ if (ItSimd != Features.rend())
+ HasSimd = ItSimd->starts_with("+");
+ if (!HasSimd && FPUSupportsNeon)
+ for (auto &F : {"-sha2", "-aes", "-crypto", "-dotprod", "-bf16", "-imm8"})
+ Features.push_back(F);
// For Arch >= ARMv8.0 && A or R profile: crypto = sha2 + aes
// Rather than replace within the feature vector, determine whether each
@@ -840,15 +832,15 @@ llvm::ARM::FPUKind arm::getARMTargetFeatures(const Driver
&D,
llvm::find_if(llvm::reverse(Features),
[](const StringRef F) { return F.contains("i8mm"); });
if (ItSHA2 != Features.rend())
- HasSHA2 = ItSHA2->take_front() == "+";
+ HasSHA2 = ItSHA2->starts_with("+");
if (ItAES != Features.rend())
- HasAES = ItAES->take_front() == "+";
+ HasAES = ItAES->starts_with("+");
if (ItBF16 != Features.rend())
- HasBF16 = ItBF16->take_front() == "+";
+ HasBF16 = ItBF16->starts_with("+");
if (ItDotprod != Features.rend())
- HasDotprod = ItDotprod->take_front() == "+";
+ HasDotprod = ItDotprod->starts_with("+");
if (ItI8MM != Features.rend())
- HasI8MM = ItI8MM->take_front() == "+";
+ HasI8MM = ItI8MM->starts_with("+");
if (ItCrypto != Features.rend()) {
if (HasSHA2 && HasAES)
Features.push_back("+crypto");
diff --git a/clang/test/Driver/arm-features.c b/clang/test/Driver/arm-features.c
index 5f837667a2614..830bd54198e99 100644
--- a/clang/test/Driver/arm-features.c
+++ b/clang/test/Driver/arm-features.c
@@ -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"
>From 2bb9077faeae5a3c4f0f849ff35f6b1890dc0f0d Mon Sep 17 00:00:00 2001
From: Jack Styles <jack.sty...@arm.com>
Date: Mon, 28 Apr 2025 14:23:56 +0100
Subject: [PATCH 4/4] Change disabling features to use .insert rather than a
for loop
---
clang/lib/Driver/ToolChains/Arch/ARM.cpp | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/clang/lib/Driver/ToolChains/Arch/ARM.cpp
b/clang/lib/Driver/ToolChains/Arch/ARM.cpp
index 451d6e7581f50..b6b1c2e28a96c 100644
--- a/clang/lib/Driver/ToolChains/Arch/ARM.cpp
+++ b/clang/lib/Driver/ToolChains/Arch/ARM.cpp
@@ -794,8 +794,8 @@ llvm::ARM::FPUKind arm::getARMTargetFeatures(const Driver
&D,
if (ItSimd != Features.rend())
HasSimd = ItSimd->starts_with("+");
if (!HasSimd && FPUSupportsNeon)
- for (auto &F : {"-sha2", "-aes", "-crypto", "-dotprod", "-bf16", "-imm8"})
- Features.push_back(F);
+ 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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
