[ARM][Driver] Ensure NEON is enabled and disabled correctly #137595
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
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.
llvmbot
added
clang
Stylie777
changed the title
|
@llvm/pr-subscribers-clang-driver @llvm/pr-subscribers-clang Author: Jack Styles (Stylie777) ChangesIn #130623 support was added for 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. Full diff: https://github.com/llvm/llvm-project/pull/137595.diff 5 Files Affected:
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
|
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
sivan-shani
reviewed
Apr 28, 2025
There was a problem hiding this comment.
Testing the enablement of neon when: (HasAES || HasSHA2 || HasBF16 || HasDotprod || HasI8MM)
It might be a good idea to add a direct test for one/some of those condition directly.
Currently there are only indirect tests in clang/test/Driver/arm-features.c when crypto is on which enable aes and sha2 which in turn enable neon
| @@ -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 | |||
There was a problem hiding this comment.
"support" → "supports"
| @@ -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 | |||
There was a problem hiding this comment.
"Instructions" -> "instructions"
| HasSHA2 = ItSHA2->take_front() == "+"; | ||
| if (FoundAES) | ||
| if (ItAES != Features.rend()) |
| (llvm::ARM::FPUNames[FPUKind].NeonSupport == | ||
| llvm::ARM::NeonSupportLevel::Crypto); | ||
| if (FoundSimd) | ||
| HasSimd = ItSimd->take_front() == "+"; |
There was a problem hiding this comment.
The function that should be used here is starts_with("+") and not take_front()
Even though the existing code below:
HasSHA2 = ItSHA2->take_front() == "+"; already use take_front(). I think it might be better to change all, including existing one, to starts_with
| HasSHA2 = ItSHA2->take_front() == "+"; | ||
| if (FoundAES) | ||
| if (ItAES != Features.rend()) | ||
| HasAES = ItAES->take_front() == "+"; |
| HasAES = ItAES->take_front() == "+"; | ||
| if (ItBF16 != Features.rend()) | ||
| HasBF16 = ItBF16->take_front() == "+"; |
| if (ItBF16 != Features.rend()) | ||
| HasBF16 = ItBF16->take_front() == "+"; | ||
| if (ItDotprod != Features.rend()) | ||
| HasDotprod = ItDotprod->take_front() == "+"; |
| if (ItDotprod != Features.rend()) | ||
| HasDotprod = ItDotprod->take_front() == "+"; | ||
| if (ItI8MM != Features.rend()) | ||
| HasI8MM = ItI8MM->take_front() == "+"; |
| llvm::ARM::NeonSupportLevel::Neon) || | ||
| (llvm::ARM::FPUNames[FPUKind].NeonSupport == | ||
| llvm::ARM::NeonSupportLevel::Crypto); | ||
| if (FoundSimd) |
There was a problem hiding this comment.
Replace FoundSimd condition-check with ItSimd != Features.rend() The variable is redundant.
| llvm::ARM::NeonSupportLevel::Crypto); | ||
| if (FoundSimd) | ||
| HasSimd = ItSimd->take_front() == "+"; | ||
| if (!HasSimd && FPUSupportsNeon) { |
There was a problem hiding this comment.
Optional:
if (!HasSimd && FPUSupportsNeon)
for (auto &F: {"-sha2","-aes","-crypto","-dotprod","-bf16","-imm8"}) Features.push_back(F);
There was a problem hiding this comment.
I have done this, it is a break from how multiple Features are usually added but it does make a cleaner implementation.
There was a problem hiding this comment.
2nd comment, I have updated this (again) to use Features.insert() as this is used elsewhere in ARM.cpp and removes the need for the for loop and the .insert() function handles this.
Stylie777
commented
Apr 28, 2025
There was a problem hiding this comment.
Thanks for the review @sivan-shani! I have made some updates based on your comments.
| @@ -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 | |||
| llvm::ARM::NeonSupportLevel::Neon) || | ||
| (llvm::ARM::FPUNames[FPUKind].NeonSupport == | ||
| llvm::ARM::NeonSupportLevel::Crypto); | ||
| if (FoundSimd) |
| (llvm::ARM::FPUNames[FPUKind].NeonSupport == | ||
| llvm::ARM::NeonSupportLevel::Crypto); | ||
| if (FoundSimd) | ||
| HasSimd = ItSimd->take_front() == "+"; |
| llvm::ARM::NeonSupportLevel::Crypto); | ||
| if (FoundSimd) | ||
| HasSimd = ItSimd->take_front() == "+"; | ||
| if (!HasSimd && FPUSupportsNeon) { |
There was a problem hiding this comment.
I have done this, it is a break from how multiple Features are usually added but it does make a cleaner implementation.
| HasSHA2 = ItSHA2->take_front() == "+"; | ||
| if (FoundAES) | ||
| if (ItAES != Features.rend()) |
| HasSHA2 = ItSHA2->take_front() == "+"; | ||
| if (FoundAES) | ||
| if (ItAES != Features.rend()) | ||
| HasAES = ItAES->take_front() == "+"; |
| HasAES = ItAES->take_front() == "+"; | ||
| if (ItBF16 != Features.rend()) | ||
| HasBF16 = ItBF16->take_front() == "+"; |
| if (ItBF16 != Features.rend()) | ||
| HasBF16 = ItBF16->take_front() == "+"; | ||
| if (ItDotprod != Features.rend()) | ||
| HasDotprod = ItDotprod->take_front() == "+"; |
| if (ItDotprod != Features.rend()) | ||
| HasDotprod = ItDotprod->take_front() == "+"; | ||
| if (ItI8MM != Features.rend()) | ||
| HasI8MM = ItI8MM->take_front() == "+"; |
sivan-shani
approved these changes
Apr 28, 2025
Stylie777
added a commit
that referenced
this pull request
Apr 29, 2025
#137595 changed the behaviour for SIMD on ARM to ensure it is enabled and disabled correctly depending on the options used by the user. In this, the functionality to disable all features that depend on SIMD was added. However, there was a spelling error for the i8mm feature, which caused the `+nosimd` command to fail. This fixes the error, and allows the command to work again.
gizmondo
pushed a commit
to gizmondo/llvm-project
that referenced
this pull request
Apr 29, 2025
) In llvm#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.
gizmondo
pushed a commit
to gizmondo/llvm-project
that referenced
this pull request
Apr 29, 2025
llvm#137595 changed the behaviour for SIMD on ARM to ensure it is enabled and disabled correctly depending on the options used by the user. In this, the functionality to disable all features that depend on SIMD was added. However, there was a spelling error for the i8mm feature, which caused the `+nosimd` command to fail. This fixes the error, and allows the command to work again.
IanWood1
pushed a commit
to IanWood1/llvm-project
that referenced
this pull request
May 6, 2025
) In llvm#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.
IanWood1
pushed a commit
to IanWood1/llvm-project
that referenced
this pull request
May 6, 2025
llvm#137595 changed the behaviour for SIMD on ARM to ensure it is enabled and disabled correctly depending on the options used by the user. In this, the functionality to disable all features that depend on SIMD was added. However, there was a spelling error for the i8mm feature, which caused the `+nosimd` command to fail. This fixes the error, and allows the command to work again.
IanWood1
pushed a commit
to IanWood1/llvm-project
that referenced
this pull request
May 6, 2025
) In llvm#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.
IanWood1
pushed a commit
to IanWood1/llvm-project
that referenced
this pull request
May 6, 2025
llvm#137595 changed the behaviour for SIMD on ARM to ensure it is enabled and disabled correctly depending on the options used by the user. In this, the functionality to disable all features that depend on SIMD was added. However, there was a spelling error for the i8mm feature, which caused the `+nosimd` command to fail. This fixes the error, and allows the command to work again.
IanWood1
pushed a commit
to IanWood1/llvm-project
that referenced
this pull request
May 6, 2025
) In llvm#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.
IanWood1
pushed a commit
to IanWood1/llvm-project
that referenced
this pull request
May 6, 2025
llvm#137595 changed the behaviour for SIMD on ARM to ensure it is enabled and disabled correctly depending on the options used by the user. In this, the functionality to disable all features that depend on SIMD was added. However, there was a spelling error for the i8mm feature, which caused the `+nosimd` command to fail. This fixes the error, and allows the command to work again.
GeorgeARM
pushed a commit
to GeorgeARM/llvm-project
that referenced
this pull request
May 7, 2025
) In llvm#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.
GeorgeARM
pushed a commit
to GeorgeARM/llvm-project
that referenced
this pull request
May 7, 2025
llvm#137595 changed the behaviour for SIMD on ARM to ensure it is enabled and disabled correctly depending on the options used by the user. In this, the functionality to disable all features that depend on SIMD was added. However, there was a spelling error for the i8mm feature, which caused the `+nosimd` command to fail. This fixes the error, and allows the command to work again.
Ankur-0429
pushed a commit
to Ankur-0429/llvm-project
that referenced
this pull request
May 9, 2025
) In llvm#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.
Ankur-0429
pushed a commit
to Ankur-0429/llvm-project
that referenced
this pull request
May 9, 2025
llvm#137595 changed the behaviour for SIMD on ARM to ensure it is enabled and disabled correctly depending on the options used by the user. In this, the functionality to disable all features that depend on SIMD was added. However, there was a spelling error for the i8mm feature, which caused the `+nosimd` command to fail. This fixes the error, and allows the command to work again.
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.
In #130623 support was added for
+nosimdin 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: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.