-
Notifications
You must be signed in to change notification settings - Fork 14.3k
[clang][driver] Set TLSDESC as the default for Android on RISC-V #81198
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
[clang][driver] Set TLSDESC as the default for Android on RISC-V #81198
Conversation
Created using spr 1.3.4
Can you add a test similar to clang/test/Driver/emulated-tls.cpp? |
I knew I was forgetting something when I threw this up. Thank you! |
… enabled) Created using spr 1.3.4
@llvm/pr-subscribers-clang @llvm/pr-subscribers-clang-driver Author: Paul Kirth (ilovepi) ChangesSets TLSDESC as the default on Android versions newer than 29, or if its Full diff: https://github.com/llvm/llvm-project/pull/81198.diff 2 Files Affected:
diff --git a/clang/test/Driver/tls-dialect.c b/clang/test/Driver/tls-dialect.c
index 4e105ce3cea5d9..f309f4a44fbc3f 100644
--- a/clang/test/Driver/tls-dialect.c
+++ b/clang/test/Driver/tls-dialect.c
@@ -3,6 +3,11 @@
// RUN: %clang -### --target=riscv64-linux %s 2>&1 | FileCheck --check-prefix=NODESC %s
// RUN: %clang -### --target=x86_64-linux -mtls-dialect=gnu %s 2>&1 | FileCheck --check-prefix=NODESC %s
+/// Android supports TLSDESC by default after Android version 29 and all RISC-V
+/// TLSDESC is not on by default in Linux, even on RISC-V
+// RUN: %clang -### --target=riscv64-android %s 2>&1 | FileCheck --check-prefix=DESC %s
+// RUN: %clang -### --target=riscv64-linux %s 2>&1 | FileCheck --check-prefix=NODESC %s
+
/// LTO
// RUN: %clang -### --target=riscv64-linux -flto -mtls-dialect=desc %s 2>&1 | FileCheck --check-prefix=LTO-DESC %s
// RUN: %clang -### --target=riscv64-linux -flto %s 2>&1 | FileCheck --check-prefix=LTO-NODESC %s
diff --git a/llvm/include/llvm/TargetParser/Triple.h b/llvm/include/llvm/TargetParser/Triple.h
index 98d8490cc9f7a2..0382d97d3fe38b 100644
--- a/llvm/include/llvm/TargetParser/Triple.h
+++ b/llvm/include/llvm/TargetParser/Triple.h
@@ -1036,8 +1036,7 @@ class Triple {
/// True if the target supports both general-dynamic and TLSDESC, and TLSDESC
/// is enabled by default.
bool hasDefaultTLSDESC() const {
- // TODO: Improve check for other platforms, like Android, and RISC-V
- return false;
+ return isAndroid() && (!isAndroidVersionLT(29) || isRISCV64());
}
/// Tests whether the target uses -data-sections as default.
|
LGTM but let's wait for any comments from enh and maskray as well. |
i think my suggestion would be instead of
just go with
that solves today's problem, and we can worry about arm64 later[1]. (and x86-64 if/when that's implemented. and 32-bit never.)
|
Sounds good. I’ll update this when I have a chance tomorrow. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM once the !isAndroidVersionLT(29)
check is removed.
clang/test/Driver/tls-dialect.c
Outdated
/// Android supports TLSDESC by default after Android version 29 and all RISC-V | ||
/// TLSDESC is not on by default in Linux, even on RISC-V | ||
// RUN: %clang -### --target=riscv64-android %s 2>&1 | FileCheck --check-prefix=DESC %s | ||
// RUN: %clang -### --target=riscv64-linux %s 2>&1 | FileCheck --check-prefix=NODESC %s |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Delete riscv64-linux
check. It's redundant.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's a good point. I went ahead and updated the comment to point that out.
Created using spr 1.3.4
…droid on RISC-V (#81198) Temporarily revert llvm/llvm-project#81198 to fix a build error. We can remove the revert once and backport fixes once it is fixed in upstream. Bug: 352589494 Test: build Change-Id: I78c1963da714a68b69c00f2d19a3c06cb5ea7ea2
No description provided.