This repository was archived by the owner on Oct 12, 2022. It is now read-only.
Bionic: Add x64 declarations, switch to single TLS function, and define __USE_GNU#2325
Merged
dlang-bot merged 3 commits intodlang:masterfrom Nov 9, 2018
joakim-noah:android
Merged
Bionic: Add x64 declarations, switch to single TLS function, and define __USE_GNU#2325dlang-bot merged 3 commits intodlang:masterfrom joakim-noah:android
dlang-bot merged 3 commits intodlang:masterfrom
joakim-noah:android
Conversation
joakim-noah
requested review from
CyberShadow,
MartinNowak,
andralex,
ibuclaw,
schveiguy and
wilzbach
as code owners
Contributor
|
Thanks for your pull request, @joakim-noah! Bugzilla referencesYour PR doesn't reference any Bugzilla issue. If your PR contains non-trivial changes, please reference a Bugzilla issue or create a manual changelog. Testing this PR locallyIf you don't have a local development environment setup, you can use Digger to test this PR: dub fetch digger
dub run digger -- build "master + druntime#2325" |
joakim-noah
commented
Oct 10, 2018
| alias uint mode_t; | ||
| alias uint nlink_t; | ||
| } | ||
| else version (MIPS32) |
Contributor
Author
There was a problem hiding this comment.
Android NDK 17, released earlier this year, dropped support for MIPS, allowing this simplification. Nobody was using Android/MIPS anyway.
The x64 mode_t declaration above was wrong.
ibuclaw
reviewed
Oct 10, 2018
src/core/sys/posix/setjmp.d
Outdated
| { | ||
| enum _JBLEN = 32; | ||
| } | ||
| else version( X86_64 ) |
joakim-noah
changed the title
Contributor
|
@joakim-noah whats the status of this? |
Contributor
Author
Contributor
|
This was green, thats all. |
Contributor
Author
|
OK, yeah, go ahead and merge, I'll get that last TLS commit in separately. |
joakim-noah
changed the title
Contributor
Author
|
I think it needs an approved review first, merge won't go through otherwise. |
thewilsonator
approved these changes
Nov 9, 2018
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
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
4 participants
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.
This
is a WIPpull is ready,asI'm going to add another commit (Update: in a later pull) that reworks how druntime initializes TLS data on Android before passing it to the GC, based on a suggestion of Martin from the ldc team.This first commit adding Bionic/x64 declarations and consolidating where possible is pretty much done, so putting it up for review. All druntime tests for Android/x86 and x64 pass when cross-compiled from linux/x64 to Termux running in an Anbox Android/x64 container.
Pretty much the same Phobos tests pass as other Android platforms, ie only some math-related modules like
std.mathorstd.internal.math.gammafunctionassert, with the only difference thatstd.randomasserts or segfaults in the massive last test block on both x86/x64 andstd.variantsegfaults in various tests on Android/x64.I'll look into those more and update this pull if needed.