Skip to content

Comments

AArch64: temporary patch to allow cross-compiling from non-AArch64#67

Merged
joakim-noah merged 1 commit intoldcfrom
aarch64
Aug 10, 2018
Merged

AArch64: temporary patch to allow cross-compiling from non-AArch64#67
joakim-noah merged 1 commit intoldcfrom
aarch64

Conversation

@joakim-noah
Copy link

@joakim-noah joakim-noah commented Aug 7, 2018

Temporarily disable so the upcoming 1.11 release doesn't require patching, though I don't know that we'll ever bother getting 128-bit floating-point CTFE for AArch64 cross-compilation working. I'llve removed the std.outbuffer excision, since @kinke's va_list patch gets it to work once we get va_list working on AArch64.

I'll also merge the core.internal.convert commit from dlang/druntime#2257 once I finish the pull and it's merged upstream or before the 1.11 release, whichever comes first, so that no patches will be needed to cross-compile the 1.11 stdlib and its tests for AArch64.

@joakim-noah joakim-noah requested a review from dnadlinger as a code owner August 7, 2018 08:25
@kinke
Copy link
Member

kinke commented Aug 7, 2018

This isn't worth it IMO. Why cheat for AArch64 instead of letting tests for unimplemented/buggy stuff fail as expected?

}
else
// Proves that sqrt(real.max) ~~ 0.5/sqrt(real.min_normal)
static assert(real.min_normal*real.max > 2 && real.min_normal*real.max <= 4);
Copy link
Member

@kinke kinke Aug 7, 2018

Choose a reason for hiding this comment

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

Ah okay this will be required (not inside a test).

std/outbuffer.d Outdated
{
// Remove once va_arg is working.
}
else
Copy link
Author

Choose a reason for hiding this comment

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

This isn't inside a test either, only the last one is. I don't care if tests fail either, this pull is needed to simply get the stdlib and test runner to compile.

Copy link
Member

Choose a reason for hiding this comment

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

[This body doesn't even use va_arg, only va_copy and va_end, both LDC intrinsics.]

Copy link
Author

Choose a reason for hiding this comment

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

Try it for yourself with the latest ldc beta:

./ldc2-1.11.0-beta2-linux-x86_64/bin/ldc2 -mtriple=aarch64-none-linux-android -c ./ldc2-1.11.0-beta2-linux-x86_64/import/std/outbuffer.d

Invalid bitcast
  %23 = bitcast %ldc.internal.vararg.std.__va_list %21 to i8*
LLVM ERROR: Broken function found, compilation aborted!

Copy link
Member

Choose a reason for hiding this comment

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

It hasn't anything to do with va_arg (which btw has an implementation for AArch64), but apparently rather with the va_list type (which it gets from the TargetABI).

Copy link
Member

Choose a reason for hiding this comment

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

... or the va* functions in the TargetABI (used by the va_start LDC intrinsic etc. IIRC).

Copy link
Author

Choose a reason for hiding this comment

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

Sure, I haven't looked into varargs much: just going off the asserts in va_arg for non-iOS AArch64 because it's unimplemented, figured this was related to that.

std/outbuffer.d Outdated
}
else
{
buf.printf(" %d", 62665);
Copy link
Author

Choose a reason for hiding this comment

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

The latest version of ldc-developers/ldc#2802 gets this module to cross-compile again, but this line segfaults in the test runner in vsnprintf. I don't mind tests asserting, but a segfault means you can't run the rest of the tests with one invocation of the test runner, so disable this for AArch64 until we get va_{list,arg} working.

@joakim-noah joakim-noah merged commit f761623 into ldc Aug 10, 2018
@kinke kinke deleted the aarch64 branch August 10, 2018 20:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants