From 39f274a7598632af37b84743d016441b45743b67 Mon Sep 17 00:00:00 2001 From: Martin Kinkelin Date: Sun, 14 Nov 2021 16:42:19 +0100 Subject: [PATCH] Don't reverse parameters order for extern(D) --- dmd/clone.d | 6 +++--- gen/abi-nvptx.cpp | 1 - gen/abi-spirv.cpp | 1 - gen/abi-x86-64.cpp | 20 +++++--------------- gen/abi-x86.cpp | 30 +++++++++++++++--------------- gen/abi.cpp | 9 --------- gen/abi.h | 5 ----- gen/dibuilder.cpp | 10 +--------- gen/functions.cpp | 12 +----------- gen/tocall.cpp | 6 +----- gen/uda.cpp | 8 ++------ ir/irfuncty.cpp | 3 +-- ir/irfuncty.h | 3 --- runtime/druntime | 2 +- runtime/phobos | 2 +- tests/codegen/asm_gcc_no_fp.d | 2 +- tests/codegen/attr_allocsize.d | 4 ++-- tests/codegen/attr_param.d | 2 +- 18 files changed, 35 insertions(+), 91 deletions(-) diff --git a/dmd/clone.d b/dmd/clone.d index 5ef344906f7..0ed6ea67e46 100644 --- a/dmd/clone.d +++ b/dmd/clone.d @@ -594,12 +594,12 @@ FuncDeclaration buildXopEquals(StructDeclaration sd, Scope* sc) /****************************************** * Build __xopCmp for TypeInfo_Struct - * static bool __xopCmp(ref const S p, ref const S q) + * static int __xopCmp(ref const S p, ref const S q) * { * return p.opCmp(q); * } * - * This is called by TypeInfo.compare(p1, p2). If the struct does not support + * This is called by TypeInfo_Struct.compare(p1, p2). If the struct does not support * const objects comparison, it will throw "not implemented" Error in runtime. */ FuncDeclaration buildXopCmp(StructDeclaration sd, Scope* sc) @@ -698,7 +698,7 @@ FuncDeclaration buildXopCmp(StructDeclaration sd, Scope* sc) fop.generated = true; Expression e1 = new IdentifierExp(loc, Id.p); Expression e2 = new IdentifierExp(loc, Id.q); - Expression e = new CallExp(loc, new DotIdExp(loc, e2, Id.cmp), e1); + Expression e = new CallExp(loc, new DotIdExp(loc, e1, Id.cmp), e2); fop.fbody = new ReturnStatement(loc, e); uint errors = global.startGagging(); // Do not report errors Scope* sc2 = sc.push(); diff --git a/gen/abi-nvptx.cpp b/gen/abi-nvptx.cpp index b64de73c691..92c8730c4be 100644 --- a/gen/abi-nvptx.cpp +++ b/gen/abi-nvptx.cpp @@ -28,7 +28,6 @@ struct NVPTXTargetABI : TargetABI { t = t->toBasetype(); return ((t->ty == TY::Tsarray || t->ty == TY::Tstruct) && t->size() > 64); } - bool reverseExplicitParams(TypeFunction *) override { return false; } void rewriteFunctionType(IrFuncTy &fty) override { for (auto arg : fty.args) { if (!arg->byref) diff --git a/gen/abi-spirv.cpp b/gen/abi-spirv.cpp index ee1582230d6..f6e569a0d7e 100644 --- a/gen/abi-spirv.cpp +++ b/gen/abi-spirv.cpp @@ -28,7 +28,6 @@ struct SPIRVTargetABI : TargetABI { t = t->toBasetype(); return ((t->ty == TY::Tsarray || t->ty == TY::Tstruct) && t->size() > 64); } - bool reverseExplicitParams(TypeFunction *) override { return false; } void rewriteFunctionType(IrFuncTy &fty) override { for (auto arg : fty.args) { if (!arg->byref) diff --git a/gen/abi-x86-64.cpp b/gen/abi-x86-64.cpp index 992f8a161c2..a00dc93b009 100644 --- a/gen/abi-x86-64.cpp +++ b/gen/abi-x86-64.cpp @@ -295,24 +295,14 @@ void X86_64TargetABI::rewriteFunctionType(IrFuncTy &fty) { Logger::println("x86-64 ABI: Transforming argument types"); LOG_SCOPE; - int begin = 0, end = fty.args.size(), step = 1; - if (fty.reverseParams) { - begin = end - 1; - end = -1; - step = -1; - } - for (int i = begin; i != end; i += step) { - IrFuncTyArg &arg = *fty.args[i]; - - if (arg.byref) { - if (!arg.isByVal() && regCount.int_regs > 0) { + for (IrFuncTyArg *arg : fty.args) { + if (arg->byref) { + if (!arg->isByVal() && regCount.int_regs > 0) { regCount.int_regs--; } - - continue; + } else { + rewriteArgument(fty, *arg, regCount); } - - rewriteArgument(fty, arg, regCount); } // regCount (fty.tag) is now in the state after all implicit & formal args, diff --git a/gen/abi-x86.cpp b/gen/abi-x86.cpp index 81e228c8979..f0524e6b4c7 100644 --- a/gen/abi-x86.cpp +++ b/gen/abi-x86.cpp @@ -192,31 +192,31 @@ struct X86TargetABI : TargetABI { sret->attrs.addAttribute(LLAttribute::InReg); } - // ... otherwise try the last argument + // ... otherwise try the first argument else if (!fty.args.empty()) { - // The last parameter is passed in EAX rather than being pushed on the + // The first parameter is passed in EAX rather than being pushed on the // stack if the following conditions are met: // * It fits in EAX. // * It is not a 3 byte struct. // * It is not a floating point type. - IrFuncTyArg *last = fty.args.back(); - if (last->rewrite == &indirectByvalRewrite || - (last->byref && !last->isByVal())) { - Logger::println("Putting last (byref) parameter in register"); - last->attrs.addAttribute(LLAttribute::InReg); + IrFuncTyArg &first = *fty.args[0]; + if (first.rewrite == &indirectByvalRewrite || + (first.byref && !first.isByVal())) { + Logger::println("Putting first (byref) parameter in register"); + first.attrs.addAttribute(LLAttribute::InReg); } else { - Type *lastTy = last->type->toBasetype(); - auto sz = lastTy->size(); - if (!lastTy->isfloating() && (sz == 1 || sz == 2 || sz == 4)) { + Type *firstTy = first.type->toBasetype(); + auto sz = firstTy->size(); + if (!firstTy->isfloating() && (sz == 1 || sz == 2 || sz == 4)) { // rewrite aggregates as integers to make inreg work - if (lastTy->ty == TY::Tstruct || lastTy->ty == TY::Tsarray) { - integerRewrite.applyTo(*last); + if (firstTy->ty == TY::Tstruct || firstTy->ty == TY::Tsarray) { + integerRewrite.applyTo(first); // undo byval semantics applied via passByVal() returning true - last->byref = false; - last->attrs.clear(); + first.byref = false; + first.attrs.clear(); } - last->attrs.addAttribute(LLAttribute::InReg); + first.attrs.addAttribute(LLAttribute::InReg); } } } diff --git a/gen/abi.cpp b/gen/abi.cpp index 9861062d136..2eee1f58d8d 100644 --- a/gen/abi.cpp +++ b/gen/abi.cpp @@ -171,13 +171,6 @@ bool TargetABI::preferPassByRef(Type *t) { ////////////////////////////////////////////////////////////////////////////// -bool TargetABI::reverseExplicitParams(TypeFunction *tf) { - // Required by druntime for extern(D), except for `, ...`-style variadics. - return isExternD(tf) && tf->parameterList.length() > 1; -} - -////////////////////////////////////////////////////////////////////////////// - void TargetABI::rewriteVarargs(IrFuncTy &fty, std::vector &args) { for (auto arg : args) { @@ -297,8 +290,6 @@ struct IntrinsicABI : TargetABI { bool passByVal(TypeFunction *, Type *t) override { return false; } - bool reverseExplicitParams(TypeFunction *) override { return false; } - void rewriteArgument(IrFuncTy &fty, IrFuncTyArg &arg) override { Type *ty = arg.type->toBasetype(); if (ty->ty != TY::Tstruct) { diff --git a/gen/abi.h b/gen/abi.h index a565f8a5f64..dc4d31a4807 100644 --- a/gen/abi.h +++ b/gen/abi.h @@ -143,11 +143,6 @@ struct TargetABI { /// argument. virtual bool passThisBeforeSret(TypeFunction *tf) { return false; } - /// Returns true if the explicit parameters order is to be reversed. - /// Defaults to true for non-variadic extern(D) functions as required by - /// druntime. - virtual bool reverseExplicitParams(TypeFunction *tf); - /// Called to give ABI the chance to rewrite the types virtual void rewriteFunctionType(IrFuncTy &fty) = 0; virtual void rewriteVarargs(IrFuncTy &fty, std::vector &args); diff --git a/gen/dibuilder.cpp b/gen/dibuilder.cpp index f945ba14353..549b0a5852a 100644 --- a/gen/dibuilder.cpp +++ b/gen/dibuilder.cpp @@ -707,9 +707,6 @@ DIType DIBuilder::CreateAArrayType(TypeAArray *type) { //////////////////////////////////////////////////////////////////////////////// -// new calling convention constant being proposed as a Dwarf extension -const unsigned DW_CC_D_dmd = 0x43; - DISubroutineType DIBuilder::CreateFunctionType(Type *type) { TypeFunction *t = type->isTypeFunction(); assert(t); @@ -720,12 +717,7 @@ DISubroutineType DIBuilder::CreateFunctionType(Type *type) { LLMetadata *params = {CreateTypeDescription(retType)}; auto paramsArray = DBuilder.getOrCreateTypeArray(params); - // The calling convention has to be recorded to distinguish - // extern(D) functions from extern(C++) ones. - unsigned CC = - getIrType(t, true)->getIrFuncTy().reverseParams ? DW_CC_D_dmd : 0; - - return DBuilder.createSubroutineType(paramsArray, DIFlags::FlagZero, CC); + return DBuilder.createSubroutineType(paramsArray, DIFlags::FlagZero, 0); } DISubroutineType DIBuilder::CreateEmptyFunctionType() { diff --git a/gen/functions.cpp b/gen/functions.cpp index 96454ff3cc5..3feb7f9a482 100644 --- a/gen/functions.cpp +++ b/gen/functions.cpp @@ -222,8 +222,6 @@ llvm::FunctionType *DtoFunctionType(Type *type, IrFuncTy &irFty, Type *thistype, ++nextLLArgIdx; } - newIrFty.reverseParams = abi->reverseExplicitParams(f); - // let the ABI rewrite the types as necessary abi->rewriteFunctionType(newIrFty); @@ -254,17 +252,11 @@ llvm::FunctionType *DtoFunctionType(Type *type, IrFuncTy &irFty, Type *thistype, std::swap(argtypes[0], argtypes[1]); } - const size_t firstExplicitArg = argtypes.size(); const size_t numExplicitLLArgs = irFty.args.size(); for (size_t i = 0; i < numExplicitLLArgs; i++) { argtypes.push_back(irFty.args[i]->ltype); } - // reverse params? - if (irFty.reverseParams && numExplicitLLArgs > 1) { - std::reverse(argtypes.begin() + firstExplicitArg, argtypes.end()); - } - irFty.funcType = LLFunctionType::get(irFty.ret->ltype, argtypes, isLLVMVariadic); @@ -782,9 +774,7 @@ void DtoDeclareFunction(FuncDeclaration *fdecl, const bool willDefine) { unsigned int k = 0; for (; iarg != func->arg_end(); ++iarg) { - size_t llExplicitIdx = irFty.reverseParams ? irFty.args.size() - k - 1 : k; - ++k; - IrFuncTyArg *arg = irFty.args[llExplicitIdx]; + IrFuncTyArg *arg = irFty.args[k++]; if (!fdecl->parameters || arg->parametersIdx >= fdecl->parameters->length) { iarg->setName("unnamed"); diff --git a/gen/tocall.cpp b/gen/tocall.cpp index 28967a64c3b..d6ab1a505fe 100644 --- a/gen/tocall.cpp +++ b/gen/tocall.cpp @@ -155,8 +155,6 @@ static void addExplicitArguments(std::vector &args, AttrSet &attrs, args.resize(implicitLLArgCount + explicitLLArgCount, static_cast(nullptr)); - // Iterate the explicit arguments from left to right in the D source, - // which is the reverse of the LLVM order if irFty.reverseParams is true. size_t dArgIndex = 0; for (size_t i = 0; i < explicitLLArgCount; ++i, ++dArgIndex) { const bool isVararg = (i >= formalLLArgCount); @@ -199,9 +197,7 @@ static void addExplicitArguments(std::vector &args, AttrSet &attrs, llvm::Value *llVal = irFty.putArg(*irArg, dval, isLValueExp, dArgIndex == explicitDArgCount - 1); - const size_t llArgIdx = - implicitLLArgCount + - (irFty.reverseParams ? explicitLLArgCount - i - 1 : i); + const size_t llArgIdx = implicitLLArgCount + i; llvm::Type *const paramType = (isVararg ? nullptr : calleeType->getParamType(llArgIdx)); diff --git a/gen/uda.cpp b/gen/uda.cpp index 53a548402d8..97e676390e3 100644 --- a/gen/uda.cpp +++ b/gen/uda.cpp @@ -181,12 +181,8 @@ void applyAttrAllocSize(StructLiteralExp *sle, IrFunction *irFunc) { unsigned offset = llvmNumParams - numUserParams; // Calculate the param indices for the function as defined in LLVM IR - auto llvmSizeIdx = - irFunc->irFty.reverseParams ? numUserParams - sizeArgIdx - 1 : sizeArgIdx; - auto llvmNumIdx = - irFunc->irFty.reverseParams ? numUserParams - numArgIdx - 1 : numArgIdx; - llvmSizeIdx += offset; - llvmNumIdx += offset; + const auto llvmSizeIdx = sizeArgIdx + offset; + const auto llvmNumIdx = numArgIdx + offset; llvm::AttrBuilder builder; if (numArgIdx >= 0) { diff --git a/ir/irfuncty.cpp b/ir/irfuncty.cpp index 8952670b83a..cdc55706819 100644 --- a/ir/irfuncty.cpp +++ b/ir/irfuncty.cpp @@ -143,8 +143,7 @@ AttrSet IrFuncTy::getParamAttrs(bool passThisBeforeSret) { // Set attributes on the explicit parameters. const size_t n = args.size(); for (size_t k = 0; k < n; k++) { - const size_t i = idx + (reverseParams ? (n - k - 1) : k); - newAttrs.addToParam(i, args[k]->attrs); + newAttrs.addToParam(idx + k, args[k]->attrs); } return newAttrs; diff --git a/ir/irfuncty.h b/ir/irfuncty.h index 4877da52b89..38f0ecf97aa 100644 --- a/ir/irfuncty.h +++ b/ir/irfuncty.h @@ -105,9 +105,6 @@ struct IrFuncTy { using ArgList = std::vector; ArgList args; - // range of normal parameters to reverse - bool reverseParams = false; - // reserved for ABI-specific data void *tag = nullptr; diff --git a/runtime/druntime b/runtime/druntime index ec3c0aafbf4..d6aefea2f38 160000 --- a/runtime/druntime +++ b/runtime/druntime @@ -1 +1 @@ -Subproject commit ec3c0aafbf4b6f3345e276e21a26ffee077470cf +Subproject commit d6aefea2f381263c6f6adab4ec32077344c62463 diff --git a/runtime/phobos b/runtime/phobos index 1e1971a940b..1c3af507ec4 160000 --- a/runtime/phobos +++ b/runtime/phobos @@ -1 +1 @@ -Subproject commit 1e1971a940b31ac85aaef7acb052846ef547d093 +Subproject commit 1c3af507ec4d88eb6ded94ce3e1a7f6d6de11775 diff --git a/tests/codegen/asm_gcc_no_fp.d b/tests/codegen/asm_gcc_no_fp.d index efaea974201..27c0b7dc92d 100644 --- a/tests/codegen/asm_gcc_no_fp.d +++ b/tests/codegen/asm_gcc_no_fp.d @@ -13,7 +13,7 @@ byte32 xor(byte32 a, byte32 b) // CHECK-NEXT: .cfi_startproc byte32 r; // CHECK-NEXT: #APP - // CHECK-NEXT: vxorps %ymm0, %ymm1, %ymm0 + // CHECK-NEXT: vxorps %ymm0, %ymm0, %ymm1 // CHECK-NEXT: #NO_APP asm { "vxorps %0, %1, %2" : "=v" (r) : "v" (a), "v" (b); } // CHECK-NEXT: retq diff --git a/tests/codegen/attr_allocsize.d b/tests/codegen/attr_allocsize.d index 8b98ee3a612..026d073217d 100644 --- a/tests/codegen/attr_allocsize.d +++ b/tests/codegen/attr_allocsize.d @@ -39,5 +39,5 @@ class A // CHECK-DAG: attributes #[[ATTR0]] ={{.*}} allocsize(1,0) // CHECK-DAG: attributes #[[ATTR1]] ={{.*}} allocsize(2) -// CHECK-DAG: attributes #[[ATTR2]] ={{.*}} allocsize(3,1) -// CHECK-DAG: attributes #[[ATTR3]] ={{.*}} allocsize(4,2) +// CHECK-DAG: attributes #[[ATTR2]] ={{.*}} allocsize(0,2) +// CHECK-DAG: attributes #[[ATTR3]] ={{.*}} allocsize(1,3) diff --git a/tests/codegen/attr_param.d b/tests/codegen/attr_param.d index 0bdffb887dd..0a9c2be3998 100644 --- a/tests/codegen/attr_param.d +++ b/tests/codegen/attr_param.d @@ -7,8 +7,8 @@ import ldc.attributes; void foo(@llvmAttr("noalias") void* p) {} // CHECK: define{{.*}} @{{.*}}3bar -// CHECK-SAME: [16 x float]*{{.*}} noalias dereferenceable(64) %kernel // CHECK-SAME: float*{{.*}} noalias %data_arg +// CHECK-SAME: [16 x float]*{{.*}} noalias dereferenceable(64) %kernel void bar(@restrict float* data, @restrict ref const float[16] kernel) {} // CHECK: define{{.*}} @{{.*}}14classReference