AArch64: Fix va_list and add preliminary Shippable CI

[kinke]

No description provided.

[kinke]

Thx David for apparently enabling aarch64.

Looking pretty good, CPU speed is way better than expected (ltsmaster built in 1.5 mins, master would probably take < 4 mins if the stdlibs would compile too).

[dnadlinger]

AArch64 still needs a druntime implementation of va_arg, which is non-trivial due to the in-register calling convention (similar to x86_64).

[kinke]

@joakim-noah: With the tentative varargs fix, std.outbuffer (cross-)compiles fine for aarch64-linux-gnu. The problem was that va_list was defined as ldc.internal.vararg.__va_list struct, while the TargetABI code was apparently mostly copied from the x86_64 one, which uses a hidden struct too, but with byref semantics and thus defining va_list as pointer to some struct; I guess the latter also applies to AArch64 here, but I haven't tested it. ;)

[kinke]

@klickverbot: I was hoping LLVM's intrinsic would work for primitive data types.

[joakim-noah]

Tried this commit out, fixes the std.outbuffer compile issue I tried to work around in ldc-developers/phobos#67 and all the same stdlib tests pass, plus the test I was commenting out from that module.

Also tried it out on ltsmaster natively on my AArch64 smartphone, where std.outbuffer always compiled before but that test failed, with this backported patch:

diff --git a/gen/abi-aarch64.cpp b/gen/abi-aarch64.cpp
index f20f7e2f..ee1df4a4 100644
--- a/gen/abi-aarch64.cpp
+++ b/gen/abi-aarch64.cpp
@@ -156,7 +156,7 @@ public:
   LLValue *prepareVaArg(LLValue *pAp) override {
     // pass a void* pointer to the actual __va_list struct to LLVM's va_arg
     // intrinsic
-    return DtoLoad(pAp);
+    return DtoBitCast(DtoLoad(pAp), getVoidPtrType());
   }
 
   Type *vaListType() override {
@@ -164,7 +164,7 @@ public:
     // using TypeIdentifier here is a bit wonky but works, as long as the name
     // is actually available in the scope (this is what DMD does, so if a better
     // solution is found there, this should be adapted).
-    return (new TypeIdentifier(Loc(), Identifier::idPool("__va_list")));
+    return (new TypeIdentifier(Loc(), Identifier::idPool("__va_list"))) ->pointerTo();
   }
 };
 

I also had to modify core.stdc.stdarg a bit, along with the va_list re-definition, to get std.stream to compile:

diff --git a/src/core/stdc/stdarg.d b/src/core/stdc/stdarg.d
index f798d9df..08c61f15 100644
--- a/src/core/stdc/stdarg.d
+++ b/src/core/stdc/stdarg.d
@@ -303,12 +303,12 @@ version( LDC )
 {
     version( AArch64 )
     {
-        void va_arg_aarch64(T)(ref __va_list ap, ref T parmn)
+        void va_arg_aarch64(T)(__va_list* ap, ref T parmn)
         {
             assert(false, "Not yet implemented");
         }
 
-        void va_arg_aarch64()(ref __va_list ap, TypeInfo ti, void* parmn)
+        void va_arg_aarch64()(__va_list* ap, TypeInfo ti, void* parmn)
         {
             assert(false, "Not yet implemented");
         }

With those changes, the std.outbuffer test now passes and the remaining stdlib tests exhibit the same results as before.

[kinke]

Thx for checking; it's the wrong way though, https://godbolt.org/g/2DpFU8 shows that va_list is indeed the struct itself.

[joakim-noah]

New Shippable CI log looking good, just change this to ninja ldc2 since building the stdlib doesn't work with master, and we can get this in.

Thanks for setting this up.

[joakim-noah]

Latest iteration of this pull gets std.outbuffer to compile, but the test runner segfaults when outbuffer.printf is called, so updated ldc-developers/phobos#67 to disable that test for now.

[kinke]

Hmm, that test worked with the earlier variant, right?

[joakim-noah]

Yes, it passed when you changed it to a pointer.

[kinke]

Okay I hope it's only a non-by-ref va_list param then in the forwarding chain.

[kinke]

Nope, the pointer seems to be the correct way, it's just that clang handles it differently in IR (no extra pointer when allocating a va_list variable).

[kinke]

@joakim-noah: Any ideas about these huge tuple indices for std.typecons? Can you reproduce that with a cross-compiled build? Wondering if it's just a miscompile by ltsmaster or a general issue.

std/typecons.d-mixin-613(613): Error: tuple index `281474784893872` exceeds length 0
std/regex/internal/parser.d(477): Error: template instance `std.typecons.Tuple!(bool, uint)` error instantiating

[joakim-noah]

That's from ldc master building Phobos? Always reproducible when using a native ldc master on AArch64, guessing it's because ltsmaster va_arg isn't working, as ldc master calls core.stdc.stdarg a few places, but haven't looked into it.

[kinke]

That's from ldc master building Phobos?

Yep, from the previous Shippable logs, when built with ltsmaster.

Always reproducible when using a native ldc master on AArch64, guessing it's because ltsmaster va_arg isn't working, as ldc master calls core.stdc.stdarg a few places, but haven't looked into it.

I hoped you had a cross-compiled master, compiled with itself from another host (with hopefully a few bugs less than ltsmaster).

I checked for va_arg usages in my entire src tree (current master, incl. submodules). There's one spot in gen/cl_helpers.cpp (but C++, so fine), all others (just a few really) are tests or other va_arg overloads; not even Phobos seems to make use of it. So a working va_arg doesn't seem to be a pressing issue for now and shouldn't stand in the way of using ltsmaster for bootstrapping.

[kinke]

So a working va_arg doesn't seem to be a pressing issue for now and shouldn't stand in the way of using ltsmaster for bootstrapping.

Nope, that'd be true for master and cross-compilation, but ltsmaster's Phobos does use va_arg in std.{format,stream}, which LDC master might use somewhere (indirectly).

[joakim-noah]

I hoped you had a cross-compiled master, compiled with itself from another host (with hopefully a few bugs less than ltsmaster).

Never tried that, figured the same issues would crop up as when built with ltsmaster.

I noticed a lot of mixins failing to compile with native ldc master, but a lot of other failures too.

If you're done here, I can merge. I'm done with the pull to not build the runtime if the submodules aren't there.

[kinke]

Never tried that, figured the same issues would crop up as when built with ltsmaster.

Nope, the D parts of LDC itself are vulnerable to miscompiles due to non-backported fixes (and on the other hand, regression-free since v2.068 ;)). That's the main reason I'd like to focus on cross-compilation instead of trying to maintain ltsmaster.

If you're done here, I can merge. I'm done with the pull to not build the runtime if the submodules aren't there.

Heh, I'll have a look at it soon. I'm done here. And will follow up with an ltsmaster backport.