Skip to content

[TargetParser] Parse Amazon Linux triple and handle RuntimeLibcalls #136114

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion llvm/include/llvm/TargetParser/Triple.h
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,8 @@ class Triple {
SUSE,
OpenEmbedded,
Intel,
LastVendorType = Intel
Amazon,
LastVendorType = Amazon
};
enum OSType {
UnknownOS,
Expand Down Expand Up @@ -900,6 +901,11 @@ class Triple {
return getArch() == Triple::arm || getArch() == Triple::armeb;
}

/// Tests whether the target is Amazon Linux.
bool isAmazonLinux() const {
return getOS() == Triple::Linux && getVendor() == Triple::Amazon;
}

/// Tests whether the target supports the EHABI exception
/// handling standard.
bool isTargetEHABICompatible() const {
Expand Down
2 changes: 1 addition & 1 deletion llvm/lib/IR/RuntimeLibcalls.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,7 @@ void RuntimeLibcallsInfo::initLibcalls(const Triple &TT) {
setLibcallName(RTLIB::EXP10_F64, "__exp10");
}

if (TT.isGNUEnvironment() || TT.isOSFuchsia() ||
if (TT.isGNUEnvironment() || TT.isOSFuchsia() || TT.isAmazonLinux() ||
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding a TT.isAmazonLinux() condition doesn't scale as there are many isGNUEnvironment conditions.

Ideally you should switch to aarch64-amazon-linux-gnu. As an alternative (but it's not a good idea to not switch to linux-gnu, set Environment to GNU, perhaps in Triple::Triple.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ideally you should switch to aarch64-amazon-linux-gnu.

I agree, but I don't think that's a change I can make. AFAIK this is just the triple Amazon Linux uses.

As an alternative (but it's not a good idea to not switch to linux-gnu, set Environment to GNU, perhaps in Triple::Triple.

/// At its core the Triple class is designed to be a wrapper for a triple
/// string; the constructor does not change or normalize the triple string.

That would seem to go against the documentation for Triple.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like setting in the environment within Triple::normalize could work though.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ideally you should switch to aarch64-amazon-linux-gnu.

I've tried that. I've set LLVM_DEFAULT_TARGET_TRIPLE=aarch64-amazon-linux-gnu in CMake. The compiler built that way has proven to be useless on Amazon Linux, not to mention it is not passing any make check tests, it fails very quickly like this:

/usr/bin/ld: cannot find -lstdc++: No such file or directory
clang++: error: linker command failed with exit code 1 (use -v to see invocation)

And the reason for this is pretty obvious, Amazon Linux treats its triple pretty seriously and uses it wherever it can, e.g., where is libstdc++.so?:

$ rpm -ql libstdc++-devel|grep libstdc++.so
/usr/lib/gcc/aarch64-amazon-linux/11/libstdc++.so
/usr/lib/gcc/aarch64-amazon-linux/11/libstdc++.so.6.0.29

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@MaskRay any thoughts on the above, or the possible alternative solution with Triple::normalize in #140070?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for trying multiple solutions. #140070 looks cleaner and avoids adding isAmazonlinux to other places isGNUEnvironment is used. Will we treat aarch64-amazon-linux-gnu as the triple and prefer it elsewhere? Perhaps yes.

So we just need a way to recognize the aarch64-amazon-linux gcc installation.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So we just need a way to recognize the aarch64-amazon-linux gcc installation.

@MaskRay that's an interesting proposal worth pursuing. Do you have any specific mechanism of doing so in mind? At what level such recognition activity could be implemented?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To restore LLVM's consistency, LLVM internally should use aarch64-amazon-linux-gnu. The configure-time LLVM_DEFAULT_TARGET_TRIPLE, and the target triple in clang --version output, in the runtime library directory (lib/target/$triple), and the clang -S -emit-llvm output, should all use linux-gnu.

Perhaps we need a llvm/cmake/config.guess change.

clang driver needs a way to pick up aarch64-amazon-linux GCC installation. The user should specify --gcc-triple= correctly, or perhaps we can have a small hack within clang driver to do it.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps we need a llvm/cmake/config.guess change.

Yes, that's one way we could explore, perhaps after restoring LLVM's consistency (and neutrality). This can happen in parallel, meanwhile we'd have to propose something that would fit into the LLVM21 timeframe.

clang driver needs a way to pick up aarch64-amazon-linux GCC installation. The user should specify --gcc-triple= correctly, or perhaps we can have a small hack within clang driver to do it.

We should not require the user to do anything more on AArch64 than they do on x86_64, and nothing more on Amazon Linux than they do on RedHat. That's consistency and neutrality.

(TT.isAndroid() && !TT.isAndroidVersionLT(9))) {
setLibcallName(RTLIB::SINCOS_F32, "sincosf");
setLibcallName(RTLIB::SINCOS_F64, "sincos");
Expand Down
3 changes: 3 additions & 0 deletions llvm/lib/TargetParser/Triple.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -270,6 +270,8 @@ StringRef Triple::getVendorTypeName(VendorType Kind) {
case PC: return "pc";
case SCEI: return "scei";
case SUSE: return "suse";
case Amazon:
return "amazon";
}

llvm_unreachable("Invalid VendorType!");
Expand Down Expand Up @@ -664,6 +666,7 @@ static Triple::VendorType parseVendor(StringRef VendorName) {
.Case("suse", Triple::SUSE)
.Case("oe", Triple::OpenEmbedded)
.Case("intel", Triple::Intel)
.Case("amazon", Triple::Amazon)
.Default(Triple::UnknownVendor);
}

Expand Down
3 changes: 3 additions & 0 deletions llvm/test/CodeGen/AArch64/veclib-llvm.sincos.ll
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py UTC_ARGS: --filter "(bl|ptrue)" --version 5
; RUN: llc -mtriple=aarch64-gnu-linux -mattr=+neon,+sve -vector-library=sleefgnuabi < %s | FileCheck %s -check-prefix=SLEEF
; RUN: llc -mtriple=aarch64-gnu-linux -mattr=+neon,+sve -vector-library=ArmPL < %s | FileCheck %s -check-prefix=ARMPL
; Check we expand to a vector library call on aarch64-amazon-linux:
; RUN: llc -mtriple=aarch64-amazon-linux -mattr=+neon,+sve -vector-library=sleefgnuabi < %s | FileCheck %s -check-prefix=SLEEF
; RUN: llc -mtriple=aarch64-amazon-linux -mattr=+neon,+sve -vector-library=ArmPL < %s | FileCheck %s -check-prefix=ARMPL

define void @test_sincos_v4f32(<4 x float> %x, ptr noalias %out_sin, ptr noalias %out_cos) {
; SLEEF-LABEL: test_sincos_v4f32:
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,11 @@
; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --filter "(:|sincos|modf|extractvalue|store)" --version 5
; RUN: opt -passes=loop-vectorize -mtriple=aarch64-gnu-linux -mcpu=neoverse-v1 -mattr=+sve < %s -S -o - -debug-only=loop-vectorize 2>%t.1 | FileCheck %s --check-prefix=CHECK
; RUN: opt -passes=loop-vectorize -mtriple=aarch64-gnu-linux -mcpu=neoverse-v1 -mattr=+sve -vector-library=ArmPL < %s -S -o - -debug-only=loop-vectorize 2>%t.2 | FileCheck %s --check-prefix=CHECK-ARMPL
; RUN: opt -passes=loop-vectorize -mtriple=aarch64-amazon-linux -mcpu=neoverse-v1 -mattr=+sve -vector-library=ArmPL < %s -S -o - -debug-only=loop-vectorize 2>%t.3 | FileCheck %s --check-prefix=CHECK-ARMPL
; RUN: FileCheck --input-file=%t.1 --check-prefix=CHECK-COST %s
; RUN: FileCheck --input-file=%t.2 --check-prefix=CHECK-COST-ARMPL %s
; Check we vectorize the functions with the vector-library on aarch64-amazon-linux:
; RUN: FileCheck --input-file=%t.3 --check-prefix=CHECK-COST-ARMPL %s
; REQUIRES: asserts

; CHECK-COST-LABEL: sincos_f32
Expand Down
5 changes: 5 additions & 0 deletions llvm/unittests/TargetParser/TripleTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,11 @@ TEST(TripleTest, ParsedIDs) {
EXPECT_EQ(Triple::Hurd, T.getOS());
EXPECT_EQ(Triple::GNU, T.getEnvironment());

T = Triple("aarch64-amazon-linux");
EXPECT_EQ(Triple::aarch64, T.getArch());
EXPECT_EQ(Triple::Amazon, T.getVendor());
EXPECT_EQ(Triple::Linux, T.getOS());

T = Triple("arm-unknown-linux-android16");
EXPECT_EQ(Triple::arm, T.getArch());
EXPECT_EQ(Triple::UnknownVendor, T.getVendor());
Expand Down
Loading