Merge pull request #3873 from kinke/no_reverse

Don't reverse parameters order for extern(D)

CHANGELOG.md

@@ -6,6 +6,7 @@
 - On Linux, LDC doesn't default to the `ld.gold` linker anymore. The combination of LLVM 13 and older gold linkers can apparently cause problems. We recommend using LLD, e.g., via `-linker=lld` or by setting your default `/usr/bin/ld` symlink; it's significantly faster too.
 - `-linkonce-templates` is less aggressive by default now and IMHO production-ready. (#3924)
 - When linking manually (not via LDC) against *shared* druntime, it is now required to link the bundled `lib/ldc_rt.dso.o[bj]` object file into each binary. It replaces the previously Windows-specific `dso_windows.obj`. (#3850)
+- Breaking `extern(D)` ABI change for all targets: formal parameters of non-variadic functions aren't reversed anymore, in line with the spec. For 32-bit x86, the *first* parameter is accordingly now potentially passed in EAX, not the last one. So non-variadic `extern(D)` functions with multiple explicit parameters will break if expecting parameters in specific registers or stack slots, e.g., naked DMD-style inline assembly. (#3873, ldc-developers/phobos@3d725fce8f0acb78bf6cb984a8462e81e8e1b715)
 
 #### Bug fixes
 - Linux: Make LTO work with LLD. (#3786, #3850)

gen/abi-nvptx.cpp

@@ -27,7 +27,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)

gen/abi-spirv.cpp

@@ -27,7 +27,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)

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;
+  for (IrFuncTyArg *arg : fty.args) {
-  if (fty.reverseParams) {
+    if (arg->byref) {
-    begin = end - 1;
+      if (!arg->isByVal() && regCount.int_regs > 0) {
-    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) {
         regCount.int_regs--;
       }
-
+    } else {
-      continue;
+      rewriteArgument(fty, *arg, regCount);
     }
-
-    rewriteArgument(fty, arg, regCount);
   }
 
   // regCount (fty.tag) is now in the state after all implicit & formal args,

gen/abi-x86.cpp

@@ -193,31 +193,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();
+        IrFuncTyArg &first = *fty.args[0];
-        if (last->rewrite == &indirectByvalRewrite ||
+        if (first.rewrite == &indirectByvalRewrite ||
-            (last->byref && !last->isByVal())) {
+            (first.byref && !first.isByVal())) {
-          Logger::println("Putting last (byref) parameter in register");
+          Logger::println("Putting first (byref) parameter in register");
-          last->attrs.addAttribute(LLAttribute::InReg);
+          first.attrs.addAttribute(LLAttribute::InReg);
         } else {
-          Type *lastTy = last->type->toBasetype();
+          Type *firstTy = first.type->toBasetype();
-          auto sz = lastTy->size();
+          auto sz = firstTy->size();
-          if (!lastTy->isfloating() && (sz == 1 || sz == 2 || sz == 4)) {
+          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) {
+            if (firstTy->ty == TY::Tstruct || firstTy->ty == TY::Tsarray) {
-              integerRewrite.applyTo(*last);
+              integerRewrite.applyTo(first);
               // undo byval semantics applied via passByVal() returning true
-              last->byref = false;
+              first.byref = false;
-              last->attrs.clear();
+              first.attrs.clear();
             }
-            last->attrs.addAttribute(LLAttribute::InReg);
+            first.attrs.addAttribute(LLAttribute::InReg);
           }
         }
       }

gen/abi.cpp

@@ -185,13 +185,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<IrFuncTyArg *> &args) {
   for (auto arg : args) {
@@ -311,8 +304,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) {

gen/abi.h

@@ -147,11 +147,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<IrFuncTyArg *> &args);

gen/dibuilder.cpp

@@ -705,9 +705,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);
@@ -718,12 +715,7 @@ DISubroutineType DIBuilder::CreateFunctionType(Type *type) {
   LLMetadata *params = {CreateTypeDescription(retType)};
   auto paramsArray = DBuilder.getOrCreateTypeArray(params);
 
-  // The calling convention has to be recorded to distinguish
+  return DBuilder.createSubroutineType(paramsArray, DIFlags::FlagZero, 0);
-  // 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);
 }
 
 DISubroutineType DIBuilder::CreateEmptyFunctionType() {

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);
 
@@ -791,9 +783,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;
+    IrFuncTyArg *arg = irFty.args[k++];
-    ++k;
-    IrFuncTyArg *arg = irFty.args[llExplicitIdx];
 
     if (!fdecl->parameters || arg->parametersIdx >= fdecl->parameters->length) {
       iarg->setName("unnamed");

gen/tocall.cpp

@@ -155,8 +155,6 @@ static void addExplicitArguments(std::vector<LLValue *> &args, AttrSet &attrs,
   args.resize(implicitLLArgCount + explicitLLArgCount,
               static_cast<llvm::Value *>(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<LLValue *> &args, AttrSet &attrs,
     llvm::Value *llVal = irFty.putArg(*irArg, dval, isLValueExp,
                                       dArgIndex == explicitDArgCount - 1);
 
-    const size_t llArgIdx =
+    const size_t llArgIdx = implicitLLArgCount + i;
-        implicitLLArgCount +
-        (irFty.reverseParams ? explicitLLArgCount - i - 1 : i);
     llvm::Type *const paramType =
         (isVararg ? nullptr : calleeType->getParamType(llArgIdx));
 

gen/uda.cpp

@@ -191,12 +191,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 =
+  const auto llvmSizeIdx = sizeArgIdx + offset;
-      irFunc->irFty.reverseParams ? numUserParams - sizeArgIdx - 1 : sizeArgIdx;
+  const auto llvmNumIdx = numArgIdx + offset;
-  auto llvmNumIdx =
-      irFunc->irFty.reverseParams ? numUserParams - numArgIdx - 1 : numArgIdx;
-  llvmSizeIdx += offset;
-  llvmNumIdx += offset;
 
   llvm::AttrBuilder builder;
   if (numArgIdx >= 0) {

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(idx + k, args[k]->attrs);
-    newAttrs.addToParam(i, args[k]->attrs);
   }
 
   return newAttrs;

ir/irfuncty.h

@@ -105,9 +105,6 @@ struct IrFuncTy {
   using ArgList = std::vector<IrFuncTyArg *>;
   ArgList args;
 
-  // range of normal parameters to reverse
-  bool reverseParams = false;
-
   // reserved for ABI-specific data
   void *tag = nullptr;
 

runtime/druntime

@@ -1 +1 @@
-Subproject commit 9c8cd6e677b2dc94549b33ff4dce1edf3952301f
+Subproject commit 644a04b9bbbfeca0d57d127c99e9027c19068bf1

runtime/jit-rt/d/ldc/dynamic_compile.d

@@ -578,11 +578,10 @@ struct BindPayload(OF, F, int[] Index, Args...)
       static if (InvalidIndex != ind)
       {
         {
-          const ii = ParametersCount - i - 1; // reverse params
+          desc[i].data = &(argStore.args[ind]);
-          desc[ii].data = &(argStore.args[ind]);
+          desc[i].size = (argStore.args[ind]).sizeof;
-          desc[ii].size = (argStore.args[ind]).sizeof;
           alias T = FuncParams[i];
-          desc[ii].type = (isAggregateType!T || isDelegate!T || isStaticArray!T ? ParamType.Aggregate : ParamType.Simple);
+          desc[i].type = (isAggregateType!T || isDelegate!T || isStaticArray!T ? ParamType.Aggregate : ParamType.Simple);
         }
       }
     }

runtime/phobos

@@ -1 +1 @@
-Subproject commit 30162ba06d3eb1f81034009ce108867a8e776acd
+Subproject commit 3d725fce8f0acb78bf6cb984a8462e81e8e1b715

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

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 #[[ATTR2]] ={{.*}} allocsize(0,2)
-// CHECK-DAG: attributes #[[ATTR3]] ={{.*}} allocsize(4,2)
+// CHECK-DAG: attributes #[[ATTR3]] ={{.*}} allocsize(1,3)

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

tests/codegen/gh2131.d

@@ -1,7 +1,7 @@
 // RUN: %ldc -O3 -output-ll -of=%t.ll %s && FileCheck %s < %t.ll
 
 // CHECK:      define {{.*}}zeroext {{.*}}@{{.*}}_D6gh21313foo
-// CHECK-SAME: i1 zeroext %x_arg
+// CHECK-SAME: i1 {{(inreg )?}}zeroext %x_arg
 bool foo(bool x, ref bool o)
 {
     // CHECK-NOT: and i8

tests/debuginfo/args_cdb.d

@@ -79,7 +79,7 @@ int byValue(ubyte ub, ushort us, uint ui, ulong ul,
 
 // check arguments with indirections
 // CDB: ?? c
-// CHECK: cdouble
+// CHECK: > struct cdouble
 // CHECK-NEXT: re : 8
 // CHECK-NEXT: im : 9