-
Notifications
You must be signed in to change notification settings - Fork 258
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
[FR] auto-enable ELF TLS in clang driver when minSdkVersion is sufficiently high #1679
Comments
@stephenhines Is this WAI, or is there some way that the IR can express the intended TLS mode? |
AFAIK we haven't added an ELF TLS -vs- emutls flag to the IR. i.e. This is WAI, at least for now.
If I understand correctly: |
Thanks for the reply and pointing out the improper ld flag. By removing the ldflag
And just use cflags
Then will have the same result: works find when LTO disabled, but lost .tbss section in the static library. Now this is WAI(works as intended), Whether I can assume that it is not supported using ELF TLS in static library with LTO in Android? Or whether there is another way to use both ELF TLS and LTO at the same time in static library? |
I think |
Thanks for the reminding. I do miss the I'll close the issue. |
Sorry I need to reopen this issue.. :) Because I find it do have the same result after adding the ldflags, there isn't When LTO is not enabled, it have
I've already add |
Update: By adding the ldflag correctly can do solve this problem. The wired behavior not works fine described above is causing by a confusing cmake ldflags behavior. This can add the
This can't generate wright ldflags in the final
But according cmake document Whatever, by adding the ldflag correctly can do solve this problem, thanks again. |
I probably should have said this is working as expected rather than as intended. The follow up work we have planned that will make this work smoothly is to teach the clang driver to do this automatically when your minSdkVersion is high enough for ELF TLS to be available. There's no reason each developer should need to configure this on their own; clang has all the information it needs to do the right thing by default. It doesn't appear we have a bug filed to track that (it doesn't show up when I search for "tls", anyway), so I'll reopen this to track since it contains some useful context. |
I happened upon this issue searching via Google, but I think the diff I commited was a start in the right direction.. |
That may actually be "done" aside from us needing to pull that update (that might be in r26 but I'd have to dig to check)? Thanks again for that patch 👍 @rprichard do you know of anything else we'd need to do? |
No, I think that LLVM change is all we would need. |
So the Android NDK r26 beta includes an update to LLVM 17 that would include this? |
It seems so. Updated the changelog: https://android-review.googlesource.com/c/platform/ndk/+/2724316 For future reference, you can look at the
That's newer than that commit, so yes. |
Bug: android/ndk#1679 Test: None (cherry picked from https://android-review.googlesource.com/q/commit:7d6e9b021806d6943cc13fc46ba5eb447f8283fe) Merged-In: Ic6bd847379a99ad18910192fb3b3295f095259fa Change-Id: Ic6bd847379a99ad18910192fb3b3295f095259fa
Bug: android/ndk#1679 Test: None (cherry picked from https://android-review.googlesource.com/q/commit:7d6e9b021806d6943cc13fc46ba5eb447f8283fe) Merged-In: Ic6bd847379a99ad18910192fb3b3295f095259fa Change-Id: Ic6bd847379a99ad18910192fb3b3295f095259fa
Description
Hi,
Disabled emulate tls With cflags to enable TLS:
Then ELF TLS works fine with the static library, and I can see .tbss section in the binary :
But when adding LTO in flag:
Then the static library compiled is not an ELF format any more, instead it is LLVM IR code:
Then .tbss section is not in the static library, and when a shared library compiled with this prebuilt static library, it will use
emulate tls
rather thanelf tls
.Thanks.
Affected versions
r23
Canary version
No response
Host OS
Mac
Host OS version
macOS 12.2.1
Affected ABIs
arm64-v8a
Build system
CMake
Other build system
No response
minSdkVersion
30
Device API level
No response
The text was updated successfully, but these errors were encountered: