[Clang][ARM]Ensure both -mno-unaligned-access and -munaligned-access are passed to multilib selection logic #134099
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
…eatures. Update the multilib selection logic to bypass the alignment option based on each architecture’s feature set, rather than relying solely on command-line options. Previously, alignment option was bypassed only when -mno-unaligned-access was explicitly specified on the commandline. This change makes the selection more robust and architecture aware.
llvmbot
added
clang
|
@llvm/pr-subscribers-clang-driver @llvm/pr-subscribers-clang Author: Simi Pallipurath (simpal01) ChangesUpdate the multilib selection logic to bypass the alignment option based on each architecture’s feature set, rather than relying on command-line options. Previously, alignment option was bypassed only when -mno-unaligned-access was explicitly specified on the command line. This change makes the selection more robust and architecture aware. Full diff: https://github.com/llvm/llvm-project/pull/134099.diff 2 Files Affected:
diff --git a/clang/lib/Driver/ToolChain.cpp b/clang/lib/Driver/ToolChain.cpp
index 8a922b283daf5..476026f82b32b 100644
--- a/clang/lib/Driver/ToolChain.cpp
+++ b/clang/lib/Driver/ToolChain.cpp
@@ -239,12 +239,12 @@ static void getAArch64MultilibFlags(const Driver &D,
Result.push_back(BranchProtectionArg->getAsString(Args));
}
- if (Arg *AlignArg = Args.getLastArg(
- options::OPT_mstrict_align, options::OPT_mno_strict_align,
- options::OPT_mno_unaligned_access, options::OPT_munaligned_access)) {
- if (AlignArg->getOption().matches(options::OPT_mstrict_align) ||
- AlignArg->getOption().matches(options::OPT_mno_unaligned_access))
- Result.push_back(AlignArg->getAsString(Args));
+ if (FeatureSet.contains("+strict-align")) {
+ Result.push_back("-mno-unaligned-access");
+ Result.push_back("-mstrict-align");
+ } else {
+ Result.push_back("-munaligned-access");
+ Result.push_back("-mno-strict-align");
}
if (Arg *Endian = Args.getLastArg(options::OPT_mbig_endian,
@@ -313,12 +313,12 @@ static void getARMMultilibFlags(const Driver &D,
Result.push_back(BranchProtectionArg->getAsString(Args));
}
- if (Arg *AlignArg = Args.getLastArg(
- options::OPT_mstrict_align, options::OPT_mno_strict_align,
- options::OPT_mno_unaligned_access, options::OPT_munaligned_access)) {
- if (AlignArg->getOption().matches(options::OPT_mstrict_align) ||
- AlignArg->getOption().matches(options::OPT_mno_unaligned_access))
- Result.push_back(AlignArg->getAsString(Args));
+ if (FeatureSet.contains("+strict-align")) {
+ Result.push_back("-mno-unaligned-access");
+ Result.push_back("-mstrict-align");
+ } else {
+ Result.push_back("-munaligned-access");
+ Result.push_back("-mno-strict-align");
}
if (Arg *Endian = Args.getLastArg(options::OPT_mbig_endian,
diff --git a/clang/test/Driver/print-multi-selection-flags.c b/clang/test/Driver/print-multi-selection-flags.c
index 5a35ae374f011..b21c2d11c1009 100644
--- a/clang/test/Driver/print-multi-selection-flags.c
+++ b/clang/test/Driver/print-multi-selection-flags.c
@@ -69,9 +69,15 @@
// CHECK-BRANCH-PROTECTION: -mbranch-protection=standard
// RUN: %clang -multi-lib-config=%S/Inputs/multilib/empty.yaml -print-multi-flags-experimental --target=arm-none-eabi -mno-unaligned-access | FileCheck --check-prefix=CHECK-NO-UNALIGNED-ACCESS %s
+// RUN: %clang -multi-lib-config=%S/Inputs/multilib/empty.yaml -print-multi-flags-experimental --target=arm-none-eabi | FileCheck --check-prefix=CHECK-NO-UNALIGNED-ACCESS %s
// RUN: %clang -multi-lib-config=%S/Inputs/multilib/empty.yaml -print-multi-flags-experimental --target=aarch64-none-elf -mno-unaligned-access | FileCheck --check-prefix=CHECK-NO-UNALIGNED-ACCESS %s
// CHECK-NO-UNALIGNED-ACCESS: -mno-unaligned-access
+// RUN: %clang -multi-lib-config=%S/Inputs/multilib/empty.yaml -print-multi-flags-experimental --target=arm-none-eabi -munaligned-access | FileCheck --check-prefix=CHECK-UNALIGNED-ACCESS %s
+// RUN: %clang -multi-lib-config=%S/Inputs/multilib/empty.yaml -print-multi-flags-experimental --target=aarch64-none-elf | FileCheck --check-prefix=CHECK-UNALIGNED-ACCESS %s
+// RUN: %clang -multi-lib-config=%S/Inputs/multilib/empty.yaml -print-multi-flags-experimental --target=aarch64-none-elf -munaligned-access | FileCheck --check-prefix=CHECK-UNALIGNED-ACCESS %s
+// CHECK-UNALIGNED-ACCESS: -munaligned-access
+
// RUN: %clang -multi-lib-config=%S/Inputs/multilib/empty.yaml -print-multi-flags-experimental --target=arm-none-eabi -mbig-endian | FileCheck --check-prefix=CHECK-BIG-ENDIAN %s
// RUN: %clang -multi-lib-config=%S/Inputs/multilib/empty.yaml -print-multi-flags-experimental --target=aarch64-none-elf -mbig-endian | FileCheck --check-prefix=CHECK-BIG-ENDIAN %s
// CHECK-BIG-ENDIAN: -mbig-endian
|
simpal01
changed the title
vhscampos
reviewed
Apr 2, 2025
There was a problem hiding this comment.
Review of the commit msg:
- Add some categories to the title, like [Clang][ARM]
- The new solution still uses the command-line options (inside the function
getARMTargetFeatures). However it also takes into account the architecture in order to determine the alignment access policy. - I think "bypass" is the wrong term. One correct term I believe could be "pass along", as in "Clang passes along the options to the multilib system".
simpal01
changed the title
Done |
smithp35
reviewed
Apr 2, 2025
smithp35
reviewed
Apr 3, 2025
smithp35
approved these changes
Apr 3, 2025
simpal01
added a commit
to arm/arm-toolchain
that referenced
this pull request
Apr 3, 2025
…builds. (#250) We don’t have a -mno-unaligned-access specific variant for the v8M libraries. For v8-M with strict aligned access, it’s more appropriate to use the v7-M variants, as they include a version with strict alignment support. It was a deliberate decision to provide strict-alignment variants only for a subset of architectures. To support this, both -mno-unaligned-access and -munaligned-access options must be passed into the multilib selection logic. This aligns with the approach in llvm/llvm-project#134099, which also considers the target architecture when determining the alignment access policy. Since Armv6-M does not support unaligned accesses, test changes are needed to reflect the updated behaviour when the target architecture is taken into account. This patch fixes below issues. 1. The naming of the v8-M libraries should explicitly include ‘unaligned’ to reflect their behaviour. 2. When using -mno-unaligned-access, the multilib system should instead select the v7-M variant. 3. Armv6m test changes are needed to reflect the updated behaviour when the target architecture is taken into account.
simpal01
added a commit
to simpal01/arm-toolchain
that referenced
this pull request
Apr 3, 2025
…are passed to multilib selection logic We need to incorporate the upstreamed patch llvm/llvm-project#134099 into the 20.x branch. So applying this as patch file.
simpal01
added a commit
to simpal01/arm-toolchain
that referenced
this pull request
Apr 3, 2025
…builds. (arm#250) We don’t have a -mno-unaligned-access specific variant for the v8M libraries. For v8-M with strict aligned access, it’s more appropriate to use the v7-M variants, as they include a version with strict alignment support. It was a deliberate decision to provide strict-alignment variants only for a subset of architectures. To support this, both -mno-unaligned-access and -munaligned-access options must be passed into the multilib selection logic. This aligns with the approach in llvm/llvm-project#134099, which also considers the target architecture when determining the alignment access policy. Since Armv6-M does not support unaligned accesses, test changes are needed to reflect the updated behaviour when the target architecture is taken into account. This patch fixes below issues. 1. The naming of the v8-M libraries should explicitly include ‘unaligned’ to reflect their behaviour. 2. When using -mno-unaligned-access, the multilib system should instead select the v7-M variant. 3. Armv6m test changes are needed to reflect the updated behaviour when the target architecture is taken into account. (cherry picked from commit 8c3bbfc)
simpal01
added a commit
to arm/arm-toolchain
that referenced
this pull request
Apr 4, 2025
…are passed to multilib selection logic We need to incorporate the upstreamed patch llvm/llvm-project#134099 into the 20.x branch. So applying this as patch file.
simpal01
added a commit
to arm/arm-toolchain
that referenced
this pull request
Apr 4, 2025
…builds. (#250) We don’t have a -mno-unaligned-access specific variant for the v8M libraries. For v8-M with strict aligned access, it’s more appropriate to use the v7-M variants, as they include a version with strict alignment support. It was a deliberate decision to provide strict-alignment variants only for a subset of architectures. To support this, both -mno-unaligned-access and -munaligned-access options must be passed into the multilib selection logic. This aligns with the approach in llvm/llvm-project#134099, which also considers the target architecture when determining the alignment access policy. Since Armv6-M does not support unaligned accesses, test changes are needed to reflect the updated behaviour when the target architecture is taken into account. This patch fixes below issues. 1. The naming of the v8-M libraries should explicitly include ‘unaligned’ to reflect their behaviour. 2. When using -mno-unaligned-access, the multilib system should instead select the v7-M variant. 3. Armv6m test changes are needed to reflect the updated behaviour when the target architecture is taken into account. (cherry picked from commit 8c3bbfc)
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.
Previously, alignment option was passed to multilib selection logic only when -mno-unaligned-access was explicitly specified on the command line.
Now this change ensure both -mno-unaligned-access and -munaligned-access are passed to the multilib selection logic, which now also considers the target architecture when determining alignment access policy.