[Clang][Driver][AArch64] Add support for aarch64-amazon-linux triple #109263
Conversation
I note that the comment nearby says "They are not needed when the user has correct LLVM_DEFAULT_TARGET_TRIPLE" and "The lists should shrink over time". The behviour was diverged between X86 and aarch64: prior to this patch, a standard clang with no configuration works on x86 but not aarch64 amazon linux; the latter fails to find C++ headers and libraries.
llvmbot
added
clang
|
@llvm/pr-subscribers-clang @llvm/pr-subscribers-clang-driver Author: Peter Waller (peterwaller-arm) ChangesI note that the comment nearby says "They are not needed when the user has The behviour was diverged between X86 and aarch64: prior to this patch, Full diff: https://github.com/llvm/llvm-project/pull/109263.diff 5 Files Affected:
diff --git a/clang/lib/Driver/ToolChains/Gnu.cpp b/clang/lib/Driver/ToolChains/Gnu.cpp
index 603d0468dd3f3c..f075ed0b35b6c5 100644
--- a/clang/lib/Driver/ToolChains/Gnu.cpp
+++ b/clang/lib/Driver/ToolChains/Gnu.cpp
@@ -2470,7 +2470,8 @@ void Generic_GCC::GCCInstallationDetector::AddDefaultGCCPrefixes(
// lists should shrink over time. Please don't add more elements to *Triples.
static const char *const AArch64LibDirs[] = {"/lib64", "/lib"};
static const char *const AArch64Triples[] = {
- "aarch64-none-linux-gnu", "aarch64-redhat-linux", "aarch64-suse-linux"};
+ "aarch64-none-linux-gnu", "aarch64-redhat-linux", "aarch64-suse-linux",
+ "aarch64-amazon-linux"};
static const char *const AArch64beLibDirs[] = {"/lib"};
static const char *const AArch64beTriples[] = {"aarch64_be-none-linux-gnu"};
diff --git a/clang/test/Driver/Inputs/ami_linux_tree/usr/lib/gcc/aarch64-amazon-linux/7/crtbegin.o b/clang/test/Driver/Inputs/ami_linux_tree/usr/lib/gcc/aarch64-amazon-linux/7/crtbegin.o
new file mode 100644
index 00000000000000..e69de29bb2d1d6
diff --git a/clang/test/Driver/Inputs/ami_linux_tree/usr/lib/gcc/aarch64-amazon-linux/7/crtbeginT.o b/clang/test/Driver/Inputs/ami_linux_tree/usr/lib/gcc/aarch64-amazon-linux/7/crtbeginT.o
new file mode 100644
index 00000000000000..e69de29bb2d1d6
diff --git a/clang/test/Driver/Inputs/ami_linux_tree/usr/lib/gcc/aarch64-amazon-linux/7/crtend.o b/clang/test/Driver/Inputs/ami_linux_tree/usr/lib/gcc/aarch64-amazon-linux/7/crtend.o
new file mode 100644
index 00000000000000..e69de29bb2d1d6
diff --git a/clang/test/Driver/linux-ld.c b/clang/test/Driver/linux-ld.c
index 28fb075a80dbbc..3f775cc7b1003b 100644
--- a/clang/test/Driver/linux-ld.c
+++ b/clang/test/Driver/linux-ld.c
@@ -1705,23 +1705,40 @@
// CHECK-LD-RHEL7-DTS-NOT: /usr/bin/ld
// CHECK-LD-RHEL7-DTS: [[SYSROOT]]/usr/lib/gcc/x86_64-redhat-linux/7/../../../../bin/ld
-// Check whether gcc7 install works fine on Amazon Linux AMI
+// Check whether gcc7 install works fine on Amazon Linux AMI targeting x86_64
// RUN: %clang -### %s -Werror -no-pie 2>&1 \
// RUN: --target=x86_64-amazon-linux -rtlib=libgcc --unwindlib=platform \
// RUN: --sysroot=%S/Inputs/ami_linux_tree \
-// RUN: | FileCheck --check-prefix=CHECK-LD-AMI %s
-// CHECK-LD-AMI: "{{.*}}ld{{(.exe)?}}" "--sysroot=[[SYSROOT:[^"]+]]"
-// CHECK-LD-AMI: "--eh-frame-hdr"
-// CHECK-LD-AMI: "-m" "elf_x86_64"
-// CHECK-LD-AMI: "-dynamic-linker"
-// CHECK-LD-AMI: "{{.*}}/usr/lib/gcc/x86_64-amazon-linux/7{{/|\\\\}}crtbegin.o"
-// CHECK-LD-AMI: "-L[[SYSROOT]]/usr/lib/gcc/x86_64-amazon-linux/7"
-// CHECK-LD-AMI: "-L[[SYSROOT]]/usr/lib/gcc/x86_64-amazon-linux/7/../../../../lib64"
-// CHECK-LD-AMI: "-L[[SYSROOT]]/lib"
-// CHECK-LD-AMI: "-L[[SYSROOT]]/usr/lib"
-// CHECK-LD-AMI: "-lgcc" "--as-needed" "-lgcc_s" "--no-as-needed"
-// CHECK-LD-AMI: "-lc"
-// CHECK-LD-AMI: "-lgcc" "--as-needed" "-lgcc_s" "--no-as-needed"
+// RUN: | FileCheck --check-prefix=CHECK-LD-AMI-X86_64 %s
+// CHECK-LD-AMI-X86_64: "{{.*}}ld{{(.exe)?}}" "--sysroot=[[SYSROOT:[^"]+]]"
+// CHECK-LD-AMI-X86_64: "--eh-frame-hdr"
+// CHECK-LD-AMI-X86_64: "-m" "elf_x86_64"
+// CHECK-LD-AMI-X86_64: "-dynamic-linker"
+// CHECK-LD-AMI-X86_64: "{{.*}}/usr/lib/gcc/x86_64-amazon-linux/7{{/|\\\\}}crtbegin.o"
+// CHECK-LD-AMI-X86_64: "-L[[SYSROOT]]/usr/lib/gcc/x86_64-amazon-linux/7"
+// CHECK-LD-AMI-X86_64: "-L[[SYSROOT]]/usr/lib/gcc/x86_64-amazon-linux/7/../../../../lib64"
+// CHECK-LD-AMI-X86_64: "-L[[SYSROOT]]/lib"
+// CHECK-LD-AMI-X86_64: "-L[[SYSROOT]]/usr/lib"
+// CHECK-LD-AMI-X86_64: "-lgcc" "--as-needed" "-lgcc_s" "--no-as-needed"
+// CHECK-LD-AMI-X86_64: "-lc"
+// CHECK-LD-AMI-X86_64: "-lgcc" "--as-needed" "-lgcc_s" "--no-as-needed"
+
+// Check whether gcc7 install works fine on Amazon Linux AMI targeting aarch64
+// RUN: %clang -### %s -Werror -no-pie 2>&1 \
+// RUN: --target=aarch64-amazon-linux -rtlib=libgcc --unwindlib=platform \
+// RUN: --sysroot=%S/Inputs/ami_linux_tree \
+// RUN: | FileCheck --check-prefix=CHECK-LD-AMI-AARCH64 %s
+// CHECK-LD-AMI-AARCH64: "{{.*}}ld{{(.exe)?}}" "--sysroot=[[SYSROOT:[^"]+]]"
+// CHECK-LD-AMI-AARCH64: "--eh-frame-hdr"
+// CHECK-LD-AMI-AARCH64: "-dynamic-linker"
+// CHECK-LD-AMI-AARCH64: "{{.*}}/usr/lib/gcc/aarch64-amazon-linux/7{{/|\\\\}}crtbegin.o"
+// CHECK-LD-AMI-AARCH64: "-L[[SYSROOT]]/usr/lib/gcc/aarch64-amazon-linux/7"
+// CHECK-LD-AMI-AARCH64: "-L[[SYSROOT]]/usr/lib/gcc/aarch64-amazon-linux/7/../../../../lib64"
+// CHECK-LD-AMI-AARCH64: "-L[[SYSROOT]]/lib"
+// CHECK-LD-AMI-AARCH64: "-L[[SYSROOT]]/usr/lib"
+// CHECK-LD-AMI-AARCH64: "-lgcc" "--as-needed" "-lgcc_s" "--no-as-needed"
+// CHECK-LD-AMI-AARCH64: "-lc"
+// CHECK-LD-AMI-AARCH64: "-lgcc" "--as-needed" "-lgcc_s" "--no-as-needed"
// Check whether the OpenEmbedded ARM libs are added correctly.
// RUN: %clang -### %s -no-pie 2>&1 \
|
|
I've checked the buildkite failures are in cfi-standalone-lld-thinlto-x86_64, which I believe are unrelated to this change. Original fix and bug report for x86_64: |
This is not an issue of the source code. The system needs to do The CMake build system could be adjusted to make |
Thanks - Is there precedence for this? Do you have any ideas about how to detect the correct default triple? |
There was a problem hiding this comment.
Whilst I understand the rational for requesting an alternate solution, it feels unfair. If the PR was to add a new OS or Vendor I'd have more sympathy but here we're talking about extending the existing Vendor-OS support to cover another target (i.e. x86_64-amazon-linux -> aarch64-amazon-linux) and so it seems fair to do so using the same method and maintain consistency.
|
@MaskRay please would you kindly reconsider aarch64-amazon-linux triple as an exception given the presence of x86_64 equivalent? I've explored that config.guess is copy of an external project with a different license; this is what reports |
|
For context and history on Amazon Linux and triplets: Amazon Linux 1 (otherwise known as Amazon Linux AMI / al-ami) is (was?) the now End-of-Life first Amazon Linux version. It was only ever built for x86 and x86-64, and used the $ARCH-amazon-linux triplet. The last 32bit AMI being part of the 2014.09 release, which means it's been close to a decade now of booting 32bit not being supported in AL. Runtime support for 32-bit continued however, until EoL. Amazon Linux 2 (released 2018, still supported) was the first to add The current release, Amazon Linux 2023, is back to using the tl;dr: AL2 has I've attached the patch we carry in Going forward, I can't see any future version of Amazon Linux changing away from 0003-Add-Amazon-Linux-vendor-triplets-to-set-of-GNU-toolc.patch For the tests being added here, if covering the (EoL) AL1 is desired, then that'd be separate from what covering AL2023 would be. |
|
Consensus expressed so far seems to be in favour of merging but I'll wait to give further opportunity for comment or objection before merging on Tuesday 8th around 0900 UTC. |
|
I don't think we've reached consensus. The comment there serves a purpose and specifically discourages people from doing this mismatching triple things. If the GCC installation on Amazon Linux uses If for some reason you have a Clang with the default target triple While I appreciate @stewartsmith 's long context, I don't think it provides real justification. If you want a llvm-project build to have correct |
|
OK, Thanks for the input all. I'm abandoning this change, I'm not following up for the time being if someone is able to fix this. Part of the problem is that it's not clear what a clean/correct cmake check would be to determine the triple. My current thinking is that cmake could take the existing inferred triple from config.guess and then amend the vendor. What's unclear is how to determine the vendor in an accurate and scalable way, without making a mess. Perhaps it would be even better if someone can fix config.guess since that is used by other projects, and then LLVM would not need special logic and it keeps the detection logic in one place. |
|
So what should we (Amazon Linux) do to help with this ? I'm not the most familiar with autotools and cmake, I know we did some tweaking of triples in the version of llvm/clang we ship as part of the distro and it's very possible that we did something wrong. I would love to help make things work better out of the box. |
What I think is needed is:
I don't know enough about the history of the current GetHostTriple and the fact it calls into the (long) GNU-licensed vendored config.guess file. My guess is that a solution should continue to use the existing config.guess; but that would need to be fixed by someone else in https://git.savannah.gnu.org/cgit/config.git/ and then the vendored copy updating. It seems to me this is the cleanest minimal solution (which will also fix other things), but it assumes config.guess is here to stay in the LLVM repository. If not, then an alternative method would be to add logic to GetHostTriple cmake file linked above to determine the triple without use of config.guess [0]. There is also the possibility of a hybrid solution of patching the triple coming out of config.guess to insert the vendor but that would seem to be adding to the mess, so I assume that's a non-starter. [0] I'm not sure how; maybe it's as simple as checking for or enumerating some paths, but I'm not sure we ideally want to be 'in the limit' adding many path checks during configure time either. If only there existed a |
Amazon Linux uses different target triple than other distributions. An attempt to accomodate it in Clang's frontend driver has been rejected already [1], so the only way to handle it is to set the default target triple explicitly when building LLVM. This will affect the name of the runtimes library which LLVM creates. [1] llvm/llvm-project#109263
…gets (#129140) The default triple of Amazon Linux on AArch64 is aarch64-amazon-linux, see issue highlighded by PR #109263, somewhat serious linker issues are encountered if any other triple is being used. Unfortunately, this makes XFAIL lines like: `XFAIL: target=aarch64{{.*}}-linux-gnu` ineffective, making it impossible to complete all of the check-cxx without failures.
Amazon Linux uses different target triple than other distributions. An attempt to accomodate it in Clang's frontend driver has been rejected already [1], so the only way to handle it is to set the default target triple explicitly when building LLVM. This will affect the name of the runtimes library which LLVM creates. [1] llvm/llvm-project#109263
It's a cherry-pick from the arm-software branch. Amazon Linux uses different target triple than other distributions. An attempt to accomodate it in Clang's frontend driver has been rejected already [1], so the only way to handle it is to set the default target triple explicitly when building LLVM. This will affect the name of the runtimes library which LLVM creates. [1] llvm/llvm-project#109263
…gets (llvm#129140) The default triple of Amazon Linux on AArch64 is aarch64-amazon-linux, see issue highlighded by PR llvm#109263, somewhat serious linker issues are encountered if any other triple is being used. Unfortunately, this makes XFAIL lines like: `XFAIL: target=aarch64{{.*}}-linux-gnu` ineffective, making it impossible to complete all of the check-cxx without failures.
The default triple of Amazon Linux on AArch64 is aarch64-amazon-linux, see issue highlighded by PR llvm#109263, somewhat serious linker issues are encountered if any other triple is being used. Unfortunately, this makes XFAIL lines like `XFAIL: target=aarch64{{.*}}-linux-gnu` ineffective, making it impossible to complete all of the check-cxx on Amazon Linux without failing.
It's a cherry-pick from the arm-software branch. Amazon Linux uses different target triple than other distributions. An attempt to accomodate it in Clang's frontend driver has been rejected already [1], so the only way to handle it is to set the default target triple explicitly when building LLVM. This will affect the name of the runtimes library which LLVM creates. [1] llvm/llvm-project#109263
The default triple of Amazon Linux on AArch64 is aarch64-amazon-linux, see issue highlighded by PR llvm#109263, somewhat serious linker issues are encountered if any other triple is being used. Unfortunately, this makes XFAIL lines like `XFAIL: target=aarch64{{.*}}-linux-gnu` ineffective, making it impossible to complete all of the check-cxx on Amazon Linux without failing.
The default triple of Amazon Linux on AArch64 is aarch64-amazon-linux, see issue highlighded by PR #109263, somewhat serious linker issues are encountered if any other triple is being used. Unfortunately, this makes XFAIL lines like `XFAIL: target=aarch64{{.*}}-linux-gnu` ineffective, making it impossible to complete all of the check-cxx on Amazon Linux without failing.
…29377) The default triple of Amazon Linux on AArch64 is aarch64-amazon-linux, see issue highlighded by PR llvm#109263, somewhat serious linker issues are encountered if any other triple is being used. Unfortunately, this makes XFAIL lines like `XFAIL: target=aarch64{{.*}}-linux-gnu` ineffective, making it impossible to complete all of the check-cxx on Amazon Linux without failing. (cherry picked from commit 8f4ee42)
…29377) The default triple of Amazon Linux on AArch64 is aarch64-amazon-linux, see issue highlighded by PR llvm#109263, somewhat serious linker issues are encountered if any other triple is being used. Unfortunately, this makes XFAIL lines like `XFAIL: target=aarch64{{.*}}-linux-gnu` ineffective, making it impossible to complete all of the check-cxx on Amazon Linux without failing.
|
Would it be possible to reconsider the decision and add Amazon Linux carries a donstream patch adding the triple |
|
I continue to hear reports of people tripping over this problem, and it continues to be unfortunate that it works as expected out of the box on x86 but not on Arm. So I remain in favour of adding it for this specific case, since the x86 legacy is there already. The alternative route to solution would be that someone needs to fix config.guess upstream so that it reports the correct triple (and get that into LLVM), then, at least if you build the compiler on Amazon Linux, I believe it should work as intended. This second route does mean that if you build a compiler off-of amazon linux it won't work when run on amazon linux. Which I think is unfortunate (e.g. the manyclangs project supplies a compiler where it would be good if it could work in this scenario); but my belief on that subject is conditioned on whether there are other concrete reasons such a compiler should not work in this configuration. |
|
As the comment specifies, we really should not add new elements to --gcc-triple= is available to support this scenario. |
Thanks for following up again @MaskRay. Forgive me for asking, but I didn't see a clear justification for "why" along side that comment. Only that it is so. I can imagine some justification for the rule, but where do you see the pressure coming from against adding it in this specific scenario? Can it be added in the interim until a proper fix has been identified/made? Is anyone in a position to fix the config.guess machinery, or suggest a method to reliably detect the triple in LLVM's cmake? Is this merely a matter of adding a test for the existence of a directory in the cmake, or is something more complicated necessary? |
|
@MaskRay Fedora and RHEL also don't use -gnu ... we had no idea this mattered when we did this and we can't really change it at this point without breaking customers. I would understand applying the rule for new use cases, but this is an established one that cannot be fixed, and an existing discrepancy in clang since it does already have the amazon linux triplet for x86_64, just missing the aarch64 one. |
I note that the comment nearby says "They are not needed when the user has
correct LLVM_DEFAULT_TARGET_TRIPLE" and "The lists should shrink over time".
The behviour was diverged between X86 and aarch64: prior to this patch,
a standard clang with no configuration works on x86 but not aarch64
amazon linux; the latter fails to find C++ headers and libraries.