Skip to content
This repository was archived by the owner on Oct 12, 2022. It is now read-only.
/ druntime Public archive

Comments

Bionic bindings for AArch64 and a Glibc/AArch64 fix#2148

Merged
dlang-bot merged 2 commits intodlang:masterfrom
joakim-noah:aarch64
Jun 3, 2018
Merged

Bionic bindings for AArch64 and a Glibc/AArch64 fix#2148
dlang-bot merged 2 commits intodlang:masterfrom
joakim-noah:aarch64

Conversation

@joakim-noah
Copy link
Contributor

This is WIP, as right now it assumes llvm's brand of emulated TLS, which cannot be passed to the GC, so that part will have to change. Just putting this up early in case @ZombineDev or someone else wants to help with the Android/AArch64 port.

@dlang-bot
Copy link
Contributor

dlang-bot commented Mar 22, 2018

Thanks for your pull request, @joakim-noah!

Bugzilla references

Your 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 locally

If 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#2148"

alias int key_t;
alias c_long suseconds_t;
alias c_long useconds_t;
alias uint useconds_t; // Updated in Lollipop
Copy link
Member

Choose a reason for hiding this comment

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

So the bindings will only ever support one version of Android?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, I believe every version since uses this now, just noting the version where the switch was made.

memcpy(p, pbeg, pend - pbeg);
*pary = p[0 .. pend - pbeg];
}
}*/
Copy link
Member

Choose a reason for hiding this comment

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

Just remove it if it is no longer used.

Copy link
Contributor Author

@joakim-noah joakim-noah Mar 23, 2018

Choose a reason for hiding this comment

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

This and all subsequent comments are because this pull is a Work-In-Progress, as noted in the OP. I'll clean it up once the functionality is set.

void* _tlsend;
size_t __bss_end__;

}extern
Copy link
Member

Choose a reason for hiding this comment

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

What's going on here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Avoids linker issues related to emulated TLS, just a hack for now.

else version (CRuntime_Bionic)
{
enum SIGRTMIN = 32;
//enum SIGRTMIN = 32;
Copy link
Member

Choose a reason for hiding this comment

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

Remove rather than comment out?

@trusted
{
version(CRuntime_Bionic)
version(none)
Copy link
Member

Choose a reason for hiding this comment

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

Don't use version(none)

Copy link
Member

@ibuclaw ibuclaw left a comment

Choose a reason for hiding this comment

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

On the surface, it looks like there are a couple quick-fix or debug related changes here that need cleaning up.

enum F_GETLK = 5;
enum F_SETLK = 6;
enum F_SETLKW = 7;
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This turned up when trying to run this Phobos test on linux/AArch64, dug this fix out of the Glibc headers. It appears Glibc uses these constants on AArch64 regardless of whether __USE_FILE_OFFSET64 is set or not, just like on x64.

I don't want to go digging through the rat's nest of platform-specific Glibc headers to figure out why this is, so I just added this exception. If someone wants to do that digging and suggest a better way to normalize this, I'd be glad to make that change.

}
}
else version (linux)
else version (CRuntime_Bionic)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I switched this to linux a couple years ago, maybe thinking it matched the kernel APIs, but now see that it doesn't. All other C runtimes that usually run on linux have their own version blocks above, so nobody other than Bionic users were likely using this.

@joakim-noah joakim-noah changed the title Bionic bindings for AArch64 Bionic bindings for AArch64 and a Glibc/AArch64 fix Jun 3, 2018
@joakim-noah
Copy link
Contributor Author

ping @ibuclaw, no longer WIP, this pull is ready to go.

int __libc_current_sigrtmax();
}

@property int SIGRTMIN() nothrow @nogc {
Copy link
Member

Choose a reason for hiding this comment

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

Could this be a template?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's a straight copy-and-paste of the Glibc block above, as Bionic switched to using the same function-calling scheme as Glibc. If you'd prefer that both blocks use template functions, I can modify them to that, don't care.

@dnadlinger dnadlinger dismissed ibuclaw’s stale review June 3, 2018 16:56

Comments have been addressed; other platform SIGRTMIN implementations aren't templates either.

@joakim-noah
Copy link
Contributor Author

I don't know what's up with CircleCI, it can't build dmd? Would closing and re-opening this pull get it to kick off again and start working?

@rainers
Copy link
Member

rainers commented Jun 3, 2018

Would closing and re-opening this pull get it to kick off again and start working?

Should work now as the compiler has been upgraded by #2205

@dnadlinger
Copy link
Contributor

This is fallout from Walter insisting on requiring an updated bootstrap compiler for no good reason – I've restarted the build.

@dlang-bot dlang-bot merged commit 551cbef into dlang:master Jun 3, 2018
@joakim-noah
Copy link
Contributor Author

Thanks, now to cherry pick to ldc, which actually has an AArch64 backend.

@joakim-noah joakim-noah deleted the aarch64 branch June 3, 2018 17:32
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants