From 3ca43c51b82c2cea91ee018b65ddd1ea83a113c4 Mon Sep 17 00:00:00 2001 From: Martin Kinkelin Date: Tue, 19 Jun 2018 20:09:35 +0200 Subject: [PATCH] Use IndirectByvalRewrite for non-POD args and extern(C++) on Posix (#2728) Fixing one aspect of issue #2702; not tackling the different destruction rules yet. --- gen/abi-aarch64.cpp | 4 ++-- gen/abi-arm.cpp | 2 +- gen/abi-mips64.cpp | 2 +- gen/abi-nvptx.cpp | 2 +- gen/abi-ppc.cpp | 2 +- gen/abi-ppc64le.cpp | 4 ++-- gen/abi-spirv.cpp | 2 +- gen/abi-win64.cpp | 2 +- gen/abi-x86-64.cpp | 43 +++++++++++++++++++++++++++++------------- gen/abi-x86.cpp | 14 +++++++++++++- gen/abi.cpp | 6 ++++-- gen/abi.h | 2 +- gen/functions.cpp | 2 +- gen/tocall.cpp | 2 +- tests/d2/dmd-testsuite | 2 +- 15 files changed, 61 insertions(+), 30 deletions(-) diff --git a/gen/abi-aarch64.cpp b/gen/abi-aarch64.cpp index 3a3894287ad..6b36ae1f841 100644 --- a/gen/abi-aarch64.cpp +++ b/gen/abi-aarch64.cpp @@ -32,10 +32,10 @@ struct AArch64TargetABI : TargetABI { if (!isPOD(rt)) return true; - return passByVal(rt); + return passByVal(tf, rt); } - bool passByVal(Type *t) override { + bool passByVal(TypeFunction *, Type *t) override { t = t->toBasetype(); return t->ty == Tsarray || (t->ty == Tstruct && t->size() > 16 && !isHFA((TypeStruct *)t)); diff --git a/gen/abi-arm.cpp b/gen/abi-arm.cpp index 8b9a4213235..ef09c40327f 100644 --- a/gen/abi-arm.cpp +++ b/gen/abi-arm.cpp @@ -42,7 +42,7 @@ struct ArmTargetABI : TargetABI { !isHFA((TypeStruct *)rt))); } - bool passByVal(Type *t) override { + bool passByVal(TypeFunction *, Type *t) override { // AAPCS does not use an indirect arg to pass aggregates, however // clang uses byval for types > 64-bytes, then llvm backend // converts back to non-byval. Without this special handling the diff --git a/gen/abi-mips64.cpp b/gen/abi-mips64.cpp index 061de51c33c..db8b4fdebca 100644 --- a/gen/abi-mips64.cpp +++ b/gen/abi-mips64.cpp @@ -42,7 +42,7 @@ struct MIPS64TargetABI : TargetABI { return (rt->ty == Tstruct || rt->ty == Tsarray); } - bool passByVal(Type *t) override { + bool passByVal(TypeFunction *, Type *t) override { TY ty = t->toBasetype()->ty; return ty == Tstruct || ty == Tsarray; } diff --git a/gen/abi-nvptx.cpp b/gen/abi-nvptx.cpp index 52d7de8e592..6eb2f04b9dc 100644 --- a/gen/abi-nvptx.cpp +++ b/gen/abi-nvptx.cpp @@ -24,7 +24,7 @@ struct NVPTXTargetABI : TargetABI { else return llvm::CallingConv::PTX_Device; } - bool passByVal(Type *t) override { + bool passByVal(TypeFunction *, Type *t) override { t = t->toBasetype(); return ((t->ty == Tsarray || t->ty == Tstruct) && t->size() > 64); } diff --git a/gen/abi-ppc.cpp b/gen/abi-ppc.cpp index d56caec3dc4..fc6f108a05b 100644 --- a/gen/abi-ppc.cpp +++ b/gen/abi-ppc.cpp @@ -49,7 +49,7 @@ struct PPCTargetABI : TargetABI { return rt->ty == Tsarray || rt->ty == Tstruct; } - bool passByVal(Type *t) override { + bool passByVal(TypeFunction *, Type *t) override { // On ppc, aggregates are always passed as an indirect value. // On ppc64, they are always passed by value. However, clang // used byval for type > 64 bytes. diff --git a/gen/abi-ppc64le.cpp b/gen/abi-ppc64le.cpp index c113a754092..a040e497a05 100644 --- a/gen/abi-ppc64le.cpp +++ b/gen/abi-ppc64le.cpp @@ -38,10 +38,10 @@ struct PPC64LETargetABI : TargetABI { if (!isPOD(rt)) return true; - return passByVal(rt); + return passByVal(tf, rt); } - bool passByVal(Type *t) override { + bool passByVal(TypeFunction *, Type *t) override { t = t->toBasetype(); return t->ty == Tsarray || (t->ty == Tstruct && t->size() > 16 && !isHFA((TypeStruct *)t, nullptr, 8)); diff --git a/gen/abi-spirv.cpp b/gen/abi-spirv.cpp index fedb0c4802d..afce3452926 100644 --- a/gen/abi-spirv.cpp +++ b/gen/abi-spirv.cpp @@ -24,7 +24,7 @@ struct SPIRVTargetABI : TargetABI { else return llvm::CallingConv::SPIR_FUNC; } - bool passByVal(Type *t) override { + bool passByVal(TypeFunction *, Type *t) override { t = t->toBasetype(); return ((t->ty == Tsarray || t->ty == Tstruct) && t->size() > 64); } diff --git a/gen/abi-win64.cpp b/gen/abi-win64.cpp index 11d65d410a7..a75194b032a 100644 --- a/gen/abi-win64.cpp +++ b/gen/abi-win64.cpp @@ -117,7 +117,7 @@ struct Win64TargetABI : TargetABI { return passPointerToHiddenCopy(rt, /*isReturnValue=*/true, tf->linkage); } - bool passByVal(Type *t) override { + bool passByVal(TypeFunction *, Type *) override { // LLVM's byval attribute is not compatible with the Win64 ABI return false; } diff --git a/gen/abi-x86-64.cpp b/gen/abi-x86-64.cpp index e9d2e256d5d..dc8398c2b7d 100644 --- a/gen/abi-x86-64.cpp +++ b/gen/abi-x86-64.cpp @@ -217,15 +217,16 @@ struct ImplicitByvalRewrite : ABIRewrite { struct X86_64TargetABI : TargetABI { X86_64_C_struct_rewrite struct_rewrite; ImplicitByvalRewrite byvalRewrite; + IndirectByvalRewrite indirectByvalRewrite; bool returnInArg(TypeFunction *tf) override; - bool passByVal(Type *t) override; + bool passByVal(TypeFunction *tf, Type *t) override; void rewriteFunctionType(IrFuncTy &fty) override; void rewriteVarargs(IrFuncTy &fty, std::vector &args) override; void rewriteArgument(IrFuncTy &fty, IrFuncTyArg &arg) override; - void rewriteArgument(IrFuncTyArg &arg, RegCount ®Count); + void rewriteArgument(IrFuncTy &fty, IrFuncTyArg &arg, RegCount ®Count); LLValue *prepareVaStart(DLValue *ap) override; @@ -252,11 +253,15 @@ bool X86_64TargetABI::returnInArg(TypeFunction *tf) { return false; } - Type *rt = tf->next; - return passByVal(rt); + Type *rt = tf->next->toBasetype(); + return dmd_abi::passByVal(rt); } -bool X86_64TargetABI::passByVal(Type *t) { +bool X86_64TargetABI::passByVal(TypeFunction *tf, Type *t) { + // indirectly by-value for extern(C++) functions and non-POD args + if (tf->linkage == LINKcpp && !isPOD(t)) + return false; + return dmd_abi::passByVal(t->toBasetype()); } @@ -264,10 +269,21 @@ void X86_64TargetABI::rewriteArgument(IrFuncTy &fty, IrFuncTyArg &arg) { llvm_unreachable("Please use the other overload explicitly."); } -void X86_64TargetABI::rewriteArgument(IrFuncTyArg &arg, RegCount ®Count) { +void X86_64TargetABI::rewriteArgument(IrFuncTy &fty, IrFuncTyArg &arg, + RegCount ®Count) { LLType *originalLType = arg.ltype; Type *t = arg.type->toBasetype(); + // indirectly by-value for extern(C++) functions and non-POD args + if (fty.type->linkage == LINKcpp && !isPOD(t)) { + indirectByvalRewrite.applyTo(arg); + if (regCount.int_regs > 0) { + regCount.int_regs--; + } + + return; + } + LLType *abiTy = getAbiType(t); if (abiTy && !LLTypeMemoryLayout::typesAreEquivalent(abiTy, originalLType)) { IF_LOG { @@ -280,11 +296,12 @@ void X86_64TargetABI::rewriteArgument(IrFuncTyArg &arg, RegCount ®Count) { } if (regCount.trySubtract(arg) == RegCount::ArgumentWouldFitInPartially) { - // pass LL structs implicitly ByVal, otherwise LLVM passes - // them partially in registers, partially in memory + // pass the LL struct with byval attribute to prevent LLVM from passing it + // partially in registers, partially in memory assert(originalLType->isStructTy()); - IF_LOG Logger::cout() << "Passing implicitly ByVal: " << arg.type->toChars() - << " (" << *originalLType << ")\n"; + IF_LOG Logger::cout() << "Passing byval to prevent register/memory mix: " + << arg.type->toChars() << " (" << *originalLType + << ")\n"; byvalRewrite.applyTo(arg); } } @@ -298,7 +315,7 @@ void X86_64TargetABI::rewriteFunctionType(IrFuncTy &fty) { Logger::println("x86-64 ABI: Transforming return type"); LOG_SCOPE; RegCount dummy; - rewriteArgument(*fty.ret, dummy); + rewriteArgument(fty, *fty.ret, dummy); } // IMPLICIT PARAMETERS @@ -336,7 +353,7 @@ void X86_64TargetABI::rewriteFunctionType(IrFuncTy &fty) { continue; } - rewriteArgument(arg, regCount); + rewriteArgument(fty, arg, regCount); } // regCount (fty.tag) is now in the state after all implicit & formal args, @@ -351,7 +368,7 @@ void X86_64TargetABI::rewriteVarargs(IrFuncTy &fty, for (auto arg : args) { if (!arg->byref) { // don't rewrite ByVal arguments - rewriteArgument(*arg, regCount); + rewriteArgument(fty, *arg, regCount); } } } diff --git a/gen/abi-x86.cpp b/gen/abi-x86.cpp index 4a43581d854..f9348ac3654 100644 --- a/gen/abi-x86.cpp +++ b/gen/abi-x86.cpp @@ -24,6 +24,7 @@ struct X86TargetABI : TargetABI { const bool isMSVC; bool returnStructsInRegs; IntegerRewrite integerRewrite; + IndirectByvalRewrite indirectByvalRewrite; X86TargetABI() : isOSX(global.params.targetTriple->isMacOSX()), @@ -118,7 +119,11 @@ struct X86TargetABI : TargetABI { return !canRewriteAsInt(rt); } - bool passByVal(Type *t) override { + bool passByVal(TypeFunction *tf, Type *t) override { + // indirectly by-value for extern(C++) functions and non-POD args on Posix + if (!isMSVC && tf->linkage == LINKcpp && !isPOD(t)) + return false; + // pass all structs and static arrays with the LLVM byval attribute return DtoIsInMemoryOnly(t); } @@ -182,6 +187,13 @@ struct X86TargetABI : TargetABI { // all other arguments are passed on the stack, don't rewrite } + // extern(C++) on Posix: non-POD args are passed indirectly by-value + else if (!isMSVC && fty.type->linkage == LINKcpp) { + for (auto arg : fty.args) { + if (!arg->byref && !isPOD(arg->type)) + indirectByvalRewrite.applyTo(*arg); + } + } workaroundIssue1356(fty.args); diff --git a/gen/abi.cpp b/gen/abi.cpp index acf32e86b8a..2b37bc63306 100644 --- a/gen/abi.cpp +++ b/gen/abi.cpp @@ -310,7 +310,9 @@ struct UnknownTargetABI : TargetABI { return (rt->ty == Tstruct || rt->ty == Tsarray); } - bool passByVal(Type *t) override { return t->toBasetype()->ty == Tstruct; } + bool passByVal(TypeFunction *, Type *t) override { + return t->toBasetype()->ty == Tstruct; + } void rewriteFunctionType(IrFuncTy &) override { // why? @@ -361,7 +363,7 @@ struct IntrinsicABI : TargetABI { bool returnInArg(TypeFunction *tf) override { return false; } - bool passByVal(Type *t) override { return false; } + bool passByVal(TypeFunction *, Type *t) override { return false; } bool reverseExplicitParams(TypeFunction *) override { return false; } diff --git a/gen/abi.h b/gen/abi.h index 9fb229eb12c..a3290aae1c4 100644 --- a/gen/abi.h +++ b/gen/abi.h @@ -137,7 +137,7 @@ struct TargetABI { /// parameter. /// The LL caller needs to pass a pointer to the original argument (the memcpy /// source). - virtual bool passByVal(Type *t) = 0; + virtual bool passByVal(TypeFunction *tf, Type *t) = 0; /// Returns true if the 'this' argument is to be passed before the 'sret' /// argument. diff --git a/gen/functions.cpp b/gen/functions.cpp index b127ad176d2..fa453a8b91e 100644 --- a/gen/functions.cpp +++ b/gen/functions.cpp @@ -170,7 +170,7 @@ llvm::FunctionType *DtoFunctionType(Type *type, IrFuncTy &irFty, Type *thistype, // ref/out attrs.addDereferenceable(loweredDType->size()); } else { - if (abi->passByVal(loweredDType)) { + if (abi->passByVal(f, loweredDType)) { // LLVM ByVal parameters are pointers to a copy in the function // parameters stack. The caller needs to provide a pointer to the // original argument. diff --git a/gen/tocall.cpp b/gen/tocall.cpp index 8c467238cc9..ac408c00406 100644 --- a/gen/tocall.cpp +++ b/gen/tocall.cpp @@ -129,7 +129,7 @@ static void addExplicitArguments(std::vector &args, AttrSet &attrs, std::vector optionalIrArgs; for (size_t i = formalDArgCount; i < explicitDArgCount; i++) { Type *argType = argexps[i]->type; - bool passByVal = gABI->passByVal(argType); + bool passByVal = gABI->passByVal(irFty.type, argType); AttrBuilder initialAttrs; if (passByVal) { diff --git a/tests/d2/dmd-testsuite b/tests/d2/dmd-testsuite index a0f157abcc2..b0b28297a8b 160000 --- a/tests/d2/dmd-testsuite +++ b/tests/d2/dmd-testsuite @@ -1 +1 @@ -Subproject commit a0f157abcc2d2779aa44cd9d6dec84aa56950d81 +Subproject commit b0b28297a8befba66640c7b4462ecdc12c40c318