From 8e753042a302e4fc95e844d975f8af3d14711ed8 Mon Sep 17 00:00:00 2001 From: Martin Kinkelin Date: Sat, 11 Aug 2018 23:18:51 +0200 Subject: [PATCH 1/6] AArch64: Fix C++ mangling of va_list C++ va_list is mangled as `std::__va_list` struct. According to clang IR, it's special in the sense of not ever being passed by value. Let's do the same and pass a pointer while at the same time mangling the function as taking the arg by value. --- gen/abi-aarch64.cpp | 96 +++++++++++++++------------------------------ runtime/druntime | 2 +- 2 files changed, 32 insertions(+), 66 deletions(-) diff --git a/gen/abi-aarch64.cpp b/gen/abi-aarch64.cpp index c2402c1bea3..ecb6a6427aa 100644 --- a/gen/abi-aarch64.cpp +++ b/gen/abi-aarch64.cpp @@ -17,6 +17,23 @@ #include "gen/abi-generic.h" #include "gen/abi-aarch64.h" +/** + * The AACPS64 uses a special native va_list type: + * + * typedef struct __va_list { + * void *__stack; // next stack param + * void *__gr_top; // end of GP arg reg save area + * void *__vr_top; // end of FP/SIMD arg reg save area + * int __gr_offs; // offset from __gr_top to next GP register arg + * int __vr_offs; // offset from __vr_top to next FP/SIMD register arg + * } va_list; + * + * In druntime, the struct is defined as object.__va_list, an alias of + * ldc.internal.vararg.std.__va_list. + * Arguments of this type are never passed by value, only by reference (even + * though the mangled function name indicates otherwise!). This requires a + * little bit of compiler magic in the following implementations. + */ struct AArch64TargetABI : TargetABI { HFAToArray hfaToArray; CompositeToArray64 compositeToArray64; @@ -35,10 +52,15 @@ struct AArch64TargetABI : TargetABI { return passByVal(tf, rt); } + bool isVaList(Type *t) { + return t->ty == Tstruct && strcmp(t->toPrettyChars(true), + "ldc.internal.vararg.std.__va_list") == 0; + } + bool passByVal(TypeFunction *, Type *t) override { t = t->toBasetype(); - return t->ty == Tsarray || - (t->ty == Tstruct && t->size() > 16 && !isHFA((TypeStruct *)t)); + return t->ty == Tsarray || (t->ty == Tstruct && t->size() > 16 && + !isHFA((TypeStruct *)t) && !isVaList(t)); } void rewriteFunctionType(IrFuncTy &fty) override { @@ -64,9 +86,14 @@ struct AArch64TargetABI : TargetABI { Type *ty = arg.type->toBasetype(); if (ty->ty == Tstruct || ty->ty == Tsarray) { + if (isVaList(ty)) { + // compiler magic: pass va_list args implicitly by reference + arg.byref = true; + arg.ltype = arg.ltype->getPointerTo(); + } // Rewrite HFAs only because union HFAs are turned into IR types that are // non-HFA and messes up register selection - if (ty->ty == Tstruct && isHFA((TypeStruct *)ty, &arg.ltype)) { + else if (ty->ty == Tstruct && isHFA((TypeStruct *)ty, &arg.ltype)) { hfaToArray.applyTo(arg, arg.ltype); } else { compositeToArray64.applyTo(arg); @@ -74,73 +101,12 @@ struct AArch64TargetABI : TargetABI { } } - /** - * The AACPS64 uses a special native va_list type: - * - * typedef struct __va_list { - * void *__stack; // next stack param - * void *__gr_top; // end of GP arg reg save area - * void *__vr_top; // end of FP/SIMD arg reg save area - * int __gr_offs; // offset from __gr_top to next GP register arg - * int __vr_offs; // offset from __vr_top to next FP/SIMD register arg - * } va_list; - * - * In druntime, the struct is defined as object.__va_list; the actually used - * core.stdc.stdarg.va_list type is a __va_list* pointer though to achieve - * byref semantics. - * This requires a little bit of compiler magic in the following - * implementations. - */ - - LLType *getValistType() { - LLType *intType = LLType::getInt32Ty(gIR->context()); - LLType *voidPointerType = getVoidPtrType(); - - std::vector parts; // struct __va_list { - parts.push_back(voidPointerType); // void *__stack; - parts.push_back(voidPointerType); // void *__gr_top; - parts.push_back(voidPointerType); // void *__vr_top; - parts.push_back(intType); // int __gr_offs; - parts.push_back(intType); // int __vr_offs; }; - - return LLStructType::get(gIR->context(), parts); - } - - LLValue *prepareVaStart(DLValue *ap) override { - // Since the user only created a __va_list* pointer (ap) on the stack before - // invoking va_start, we first need to allocate the actual __va_list struct - // and set `ap` to its address. - LLValue *valistmem = DtoRawAlloca(getValistType(), 0, "__va_list_mem"); - DtoStore(valistmem, - DtoBitCast(DtoLVal(ap), getPtrToType(valistmem->getType()))); - // Pass a i8* pointer to the actual struct to LLVM's va_start intrinsic. - return DtoBitCast(valistmem, getVoidPtrType()); - } - - void vaCopy(DLValue *dest, DValue *src) override { - // Analog to va_start, we first need to allocate a new __va_list struct on - // the stack and set `dest` to its address. - LLValue *valistmem = DtoRawAlloca(getValistType(), 0, "__va_list_mem"); - DtoStore(valistmem, - DtoBitCast(DtoLVal(dest), getPtrToType(valistmem->getType()))); - // Then fill the new struct with a bitcopy of the source struct. - // `src` is a __va_list* pointer to the source struct. - DtoMemCpy(valistmem, DtoRVal(src)); - } - - LLValue *prepareVaArg(DLValue *ap) override { - // Pass a i8* pointer to the actual __va_list struct to LLVM's va_arg - // intrinsic. - return DtoBitCast(DtoRVal(ap), getVoidPtrType()); - } - Type *vaListType() override { // We need to pass the actual va_list type for correct mangling. Simply // 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 createTypeIdentifier(Loc(), Identifier::idPool("__va_list")) - ->pointerTo(); + return createTypeIdentifier(Loc(), Identifier::idPool("__va_list")); } const char *objcMsgSendFunc(Type *ret, IrFuncTy &fty) override { diff --git a/runtime/druntime b/runtime/druntime index 8f488105f32..976bd15e14d 160000 --- a/runtime/druntime +++ b/runtime/druntime @@ -1 +1 @@ -Subproject commit 8f488105f3234a9399cc38958664aa1927d0f6ba +Subproject commit 976bd15e14da2529c16d1517c176f288e50b9cc2 From 9148235ef59fd3af8a607f97c7ddd5650d217043 Mon Sep 17 00:00:00 2001 From: Martin Kinkelin Date: Mon, 13 Aug 2018 23:52:40 +0200 Subject: [PATCH 2/6] AArch64: Apply ltsmaster's IndirectByvalRewrite fix --- gen/abi-aarch64.cpp | 81 +++++++++++++++++++++++++-------------------- 1 file changed, 46 insertions(+), 35 deletions(-) diff --git a/gen/abi-aarch64.cpp b/gen/abi-aarch64.cpp index ecb6a6427aa..a6b7499dc21 100644 --- a/gen/abi-aarch64.cpp +++ b/gen/abi-aarch64.cpp @@ -35,10 +35,24 @@ * little bit of compiler magic in the following implementations. */ struct AArch64TargetABI : TargetABI { +private: + IndirectByvalRewrite byvalRewrite; HFAToArray hfaToArray; CompositeToArray64 compositeToArray64; IntegerRewrite integerRewrite; + bool isVaList(Type *t) { + return t->ty == Tstruct && strcmp(t->toPrettyChars(true), + "ldc.internal.vararg.std.__va_list") == 0; + } + + bool passIndirectlyByValue(Type *t) { + t = t->toBasetype(); + return t->ty == Tsarray || (t->ty == Tstruct && t->size() > 16 && + !isHFA(static_cast(t))); + } + +public: bool returnInArg(TypeFunction *tf, bool) override { if (tf->isref) { return false; @@ -49,58 +63,55 @@ struct AArch64TargetABI : TargetABI { if (!isPOD(rt)) return true; - return passByVal(tf, rt); - } - - bool isVaList(Type *t) { - return t->ty == Tstruct && strcmp(t->toPrettyChars(true), - "ldc.internal.vararg.std.__va_list") == 0; + return passIndirectlyByValue(rt); } - bool passByVal(TypeFunction *, Type *t) override { - t = t->toBasetype(); - return t->ty == Tsarray || (t->ty == Tstruct && t->size() > 16 && - !isHFA((TypeStruct *)t) && !isVaList(t)); - } + bool passByVal(TypeFunction *, Type *) override { return false; } void rewriteFunctionType(IrFuncTy &fty) override { - Type *retTy = fty.ret->type->toBasetype(); - if (!fty.ret->byref && retTy->ty == Tstruct) { - // Rewrite HFAs only because union HFAs are turned into IR types that are - // non-HFA and messes up register selection - if (isHFA((TypeStruct *)retTy, &fty.ret->ltype)) { - hfaToArray.applyTo(*fty.ret, fty.ret->ltype); - } else { - integerRewrite.applyTo(*fty.ret); - } + Type *rt = fty.ret->type->toBasetype(); + if (!fty.ret->byref && rt->ty != Tvoid) { + rewriteArgument(fty, *fty.ret, true); } for (auto arg : fty.args) { - if (!arg->byref) + if (!arg->byref) { rewriteArgument(fty, *arg); + } } } - void rewriteArgument(IrFuncTy &fty, IrFuncTyArg &arg) override { - // FIXME - Type *ty = arg.type->toBasetype(); - - if (ty->ty == Tstruct || ty->ty == Tsarray) { - if (isVaList(ty)) { - // compiler magic: pass va_list args implicitly by reference - arg.byref = true; - arg.ltype = arg.ltype->getPointerTo(); - } - // Rewrite HFAs only because union HFAs are turned into IR types that are - // non-HFA and messes up register selection - else if (ty->ty == Tstruct && isHFA((TypeStruct *)ty, &arg.ltype)) { - hfaToArray.applyTo(arg, arg.ltype); + void rewriteArgument(IrFuncTy &fty, IrFuncTyArg &arg, bool isReturnVal) { + const auto t = arg.type->toBasetype(); + + if (t->ty != Tstruct && t->ty != Tsarray) + return; + + if (!isReturnVal && isVaList(t)) { + // compiler magic: pass va_list args implicitly by reference + arg.byref = true; + arg.ltype = arg.ltype->getPointerTo(); + } else if (!isReturnVal && passIndirectlyByValue(t)) { + byvalRewrite.applyTo(arg); + } + // Rewrite HFAs only because union HFAs are turned into IR types that are + // non-HFA and messes up register selection + else if (t->ty == Tstruct && + isHFA(static_cast(t), &arg.ltype)) { + hfaToArray.applyTo(arg, arg.ltype); + } else { + if (isReturnVal) { + integerRewrite.applyTo(arg); } else { compositeToArray64.applyTo(arg); } } } + void rewriteArgument(IrFuncTy &fty, IrFuncTyArg &arg) override { + rewriteArgument(fty, arg, false); + } + Type *vaListType() override { // We need to pass the actual va_list type for correct mangling. Simply // using TypeIdentifier here is a bit wonky but works, as long as the name From c73ea7d08aca59820024cfe0443a0d343282a361 Mon Sep 17 00:00:00 2001 From: Martin Kinkelin Date: Tue, 14 Aug 2018 20:49:34 +0200 Subject: [PATCH 3/6] Extend Shippable CI by full build and tests --- shippable.yml | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/shippable.yml b/shippable.yml index 6a6e902e300..7b0cfbd18d8 100644 --- a/shippable.yml +++ b/shippable.yml @@ -48,11 +48,21 @@ build: - cd build - | cmake -G Ninja \ - -DCMAKE_BUILD_TYPE=Release \ + -DCMAKE_BUILD_TYPE=RelWithDebInfo \ -DLLVM_ROOT_DIR=$PWD/../llvm \ -DLDC_INSTALL_LTOPLUGIN=ON \ -DLDC_INSTALL_LLVM_RUNTIME_LIBS=ON \ -DD_COMPILER=$PWD/../bootstrap/bin/ldmd2 \ .. - - ninja ldc2 + - ninja - bin/ldc2 --version + # Build druntime/Phobos unittest runners + - ninja -j16 all-test-runners + # Build and run LDC D unittests + - ctest --output-on-failure -R ldc2-unittest + # Run LIT testsuite + - ctest -V -R lit-tests || true + # Run druntime/Phobos unittests + - ctest -j16 --output-on-failure -E "dmd-testsuite|ldc2-unittest|lit-tests" || true + # Run DMD testsuite + - DMD_TESTSUITE_MAKE_ARGS='-j16 -k' ctest -V -R dmd-testsuite || true From 69358856abbcee131d523c551d15afd37f1fa1b6 Mon Sep 17 00:00:00 2001 From: Martin Kinkelin Date: Tue, 14 Aug 2018 21:37:12 +0200 Subject: [PATCH 4/6] Shippable CI: Add self-compilation step --- shippable.yml | 25 +++++++++++++++++++------ 1 file changed, 19 insertions(+), 6 deletions(-) diff --git a/shippable.yml b/shippable.yml index 7b0cfbd18d8..d0f80dc22fd 100644 --- a/shippable.yml +++ b/shippable.yml @@ -30,20 +30,33 @@ build: - git checkout -b _backup - git checkout ltsmaster - git submodule update - - mkdir build-bootstrap - - cd build-bootstrap + - mkdir build-ltsmaster + - cd build-ltsmaster - | cmake -G Ninja \ -DCMAKE_BUILD_TYPE=Release \ -DLLVM_ROOT_DIR=$PWD/../llvm \ - -DCMAKE_INSTALL_PREFIX=$PWD/../bootstrap \ + -DCMAKE_INSTALL_PREFIX=$PWD/../ldc-ltsmaster \ .. - ninja install - cd .. - - bootstrap/bin/ldc2 --version - # Build actual version + - ldc-ltsmaster/bin/ldc2 --version + # Build actual version, for another bootstrapping step - git checkout _backup - git submodule update + - mkdir build-bootstrap + - cd build-bootstrap + - | + cmake -G Ninja \ + -DCMAKE_BUILD_TYPE=RelWithDebInfo \ + -DLLVM_ROOT_DIR=$PWD/../llvm \ + -DCMAKE_INSTALL_PREFIX=$PWD/../ldc-bootstrap \ + -DD_COMPILER=$PWD/../ldc-ltsmaster/bin/ldmd2 \ + .. + - ninja install + - cd .. + - ldc-bootstrap/bin/ldc2 --version + # Build with itself - mkdir build - cd build - | @@ -52,7 +65,7 @@ build: -DLLVM_ROOT_DIR=$PWD/../llvm \ -DLDC_INSTALL_LTOPLUGIN=ON \ -DLDC_INSTALL_LLVM_RUNTIME_LIBS=ON \ - -DD_COMPILER=$PWD/../bootstrap/bin/ldmd2 \ + -DD_COMPILER=$PWD/../ldc-bootstrap/bin/ldmd2 \ .. - ninja - bin/ldc2 --version From e6509d01cc1ee2c5b31b649b96d2e6d4e4090203 Mon Sep 17 00:00:00 2001 From: Martin Kinkelin Date: Tue, 14 Aug 2018 23:13:46 +0200 Subject: [PATCH 5/6] Shippable CI: Disable email notifications --- shippable.yml | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/shippable.yml b/shippable.yml index d0f80dc22fd..55ebf8a5cc5 100644 --- a/shippable.yml +++ b/shippable.yml @@ -75,7 +75,16 @@ build: - ctest --output-on-failure -R ldc2-unittest # Run LIT testsuite - ctest -V -R lit-tests || true - # Run druntime/Phobos unittests - - ctest -j16 --output-on-failure -E "dmd-testsuite|ldc2-unittest|lit-tests" || true # Run DMD testsuite - DMD_TESTSUITE_MAKE_ARGS='-j16 -k' ctest -V -R dmd-testsuite || true + # Run druntime/Phobos unittests + - ctest -j16 --output-on-failure -E "dmd-testsuite|ldc2-unittest|lit-tests" || true + +integrations: + notifications: + - integrationName: email + type: email + on_success: never + on_failure: never + on_cancel: never + on_pull_request: never From b4a2a90709a9d53e40de4353deb4f6a69e5d1527 Mon Sep 17 00:00:00 2001 From: Martin Kinkelin Date: Wed, 15 Aug 2018 13:58:32 +0200 Subject: [PATCH 6/6] Shippable CI: Don't run debug unittests/dmd-testsuite Some of those make the whole test run hang. --- shippable.yml | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/shippable.yml b/shippable.yml index 55ebf8a5cc5..6f723b3497d 100644 --- a/shippable.yml +++ b/shippable.yml @@ -75,10 +75,10 @@ build: - ctest --output-on-failure -R ldc2-unittest # Run LIT testsuite - ctest -V -R lit-tests || true - # Run DMD testsuite - - DMD_TESTSUITE_MAKE_ARGS='-j16 -k' ctest -V -R dmd-testsuite || true - # Run druntime/Phobos unittests - - ctest -j16 --output-on-failure -E "dmd-testsuite|ldc2-unittest|lit-tests" || true + # Run DMD testsuite (non-debug only for now) + - DMD_TESTSUITE_MAKE_ARGS='-j16 -k' ctest -V -R dmd-testsuite -E "-debug$" || true + # Run druntime/Phobos unittests (non-debug only for now) + - ctest -j16 --output-on-failure -E "dmd-testsuite|ldc2-unittest|lit-tests|-debug(-shared)?$" || true integrations: notifications: