Merge pull request #11828 from kinke/preview_in

Revise ABI-specific part of -preview=in
merged-on-behalf-of: Mathias LANG <pro.mathias.lang@gmail.com>

Author: dlang-bot

src/dmd/frontend.h

@@ -6541,7 +6541,7 @@ struct Target
     TypeTuple* toArgTypes(Type* t);
     bool isReturnOnStack(TypeFunction* tf, bool needsThis);
     uint64_t parameterSize(const Loc& loc, Type* t);
-    void applyInRefParams(TypeFunction* tf);
+    bool preferPassByRef(Type* t);
 private:
     enum class TargetInfoKeys
     {

src/dmd/mtype.d

@@ -6951,8 +6951,8 @@ extern (C++) final class Parameter : ASTNode
      * Params:
      *  returnByRef = true if the function returns by ref
      *  p = Parameter to compare with
-     *  previewIn = Whether `-previewIn` is being used, and thus if
-     *              `in` means `scope`.
+     *  previewIn = Whether `-preview=in` is being used, and thus if
+     *              `in` means `scope [ref]`.
      *
      * Returns:
      *  true = `this` can be used in place of `p`
@@ -6972,8 +6972,8 @@ extern (C++) final class Parameter : ASTNode
                 otherSTC |= STC.scope_;
         }
 
-        enum stc = STC.ref_ | STC.out_ | STC.lazy_;
-        if ((thisSTC & stc) != (otherSTC & stc))
+        const mask = STC.ref_ | STC.out_ | STC.lazy_ | (previewIn ? STC.in_ : 0);
+        if ((thisSTC & mask) != (otherSTC & mask))
             return false;
         return isCovariantScope(returnByRef, thisSTC, otherSTC);
     }

src/dmd/mtype.h

@@ -558,7 +558,7 @@ class Parameter : public ASTNode
     static size_t dim(Parameters *parameters);
     static Parameter *getNth(Parameters *parameters, d_size_t nth);
     const char *toChars() const;
-    bool isCovariant(bool returnByRef, const Parameter *p) const;
+    bool isCovariant(bool returnByRef, const Parameter *p, bool previewIn) const;
 };
 
 struct ParameterList

src/dmd/target.d

@@ -776,60 +776,61 @@ extern (C++) struct Target
     }
 
     /**
-     * Determine which `in` parameters needs to be passed by `ref`
-     *
-     * Called from `TypeFunction` semantic with the full function type.
-     * This routine must iterate over parameters, and may set `STC.ref_`
-     * for any parameter which already have `STC.in_`.
-     * This hook must never set `STC.ref_` if the parameter is not `STC.in_`,
-     * nor should it ever change anything else.
-     *
-     * This hook will not be called when `-preview=in` wasn't passed to the
-     * frontend, hence it needs not care about `params.previewIn`.
-     *
+     * Decides whether an `in` parameter of the specified POD type is to be
+     * passed by reference or by value. To be used with `-preview=in` only!
      * Params:
-     *   tf    = Type of the function to inspect. The type will have its
-     *           parameter types semantically resolved, however other attributes
-     *           (return type, `@safe` / `nothrow`, etc...) must not be used.
+     *  t = type of the `in` parameter, must be a POD
+     * Returns:
+     *  `true` if the `in` parameter is to be passed by reference
      */
-    extern(C++) void applyInRefParams (TypeFunction tf)
+    extern (C++) bool preferPassByRef(Type t)
     {
-        foreach (_idx, p; tf.parameterList)
+        const size = t.size();
+        if (global.params.is64bit)
         {
-            // Ignore non-`in` or already-`ref` parameters
-            if ((p.storageClass & (STC.in_ | STC.ref_)) != STC.in_)
-                continue;
-
-            assert(p.type !is null);
-            // If it has a copy constructor / destructor / postblit,
-            // it is always by ref
-            if (p.type.needsDestruction() || p.type.needsCopyOrPostblit())
-                p.storageClass |= STC.ref_;
-            // If the type can't be copied, always by `ref`
-            else if (!p.type.isCopyable())
-                p.storageClass |= STC.ref_;
-            // The Win64 ABI requires x87 real to be passed by ref
-            else if (params.isWindows && params.is64bit &&
-                     p.type.ty == Tfloat80)
-                p.storageClass |= STC.ref_;
-
-            // If it's a dynamic array, use the value type as it
-            // allows covariance between `in char[]` and `scope const(char)[]`
-            // The same reasoning applies to pointers and classes,
-            // but that is handled by the `(sz > 8)` below.
-            else if (p.type.ty == Tarray)
-                continue;
-            // Pass delegates by value to allow covariance
-            // Function pointers are a single pointers and handled below.
-            else if (p.type.ty == Tdelegate)
-                continue;
-            else
+            if (global.params.isWindows)
+            {
+                // Win64 special case: by-value for slices and delegates due to
+                // high number of usages in druntime/Phobos (compiled without
+                // -preview=in but supposed to link against -preview=in code)
+                const ty = t.toBasetype().ty;
+                if (ty == Tarray || ty == Tdelegate)
+                    return false;
+
+                // If size is larger than 8 or not a power-of-2, the Win64 ABI
+                // would require a hidden reference anyway.
+                return size > 8
+                    || (size > 0 && (size & (size - 1)) != 0);
+            }
+            else // SysV x86_64 ABI
             {
-                const sz = p.type.size();
-                if (params.is64bit ? (sz > 16) : (sz > 8))
-                    p.storageClass |= STC.ref_;
+                // Prefer a ref if the POD cannot be passed in registers, i.e.,
+                // would be passed on the stack, *and* the size is > 16.
+                if (size <= 16)
+                    return false;
+
+                TypeTuple getArgTypes()
+                {
+                    import dmd.aggregate : Sizeok;
+                    if (auto ts = t.toBasetype().isTypeStruct())
+                    {
+                        auto sd = ts.sym;
+                        assert(sd.sizeok == Sizeok.done);
+                        return sd.argTypes;
+                    }
+                    return toArgTypes(t);
+                }
+
+                TypeTuple argTypes = getArgTypes();
+                assert(argTypes !is null, "size == 0 should already be handled");
+                return argTypes.arguments.length == 0; // cannot be passed in registers
             }
         }
+        else // 32-bit x86 ABI
+        {
+            // Prefer a ref if the size is > 2 machine words.
+            return size > 8;
+        }
     }
 
     // this guarantees `getTargetInfo` and `allTargetInfos` remain in sync

src/dmd/target.h

@@ -109,7 +109,7 @@ struct Target
     TypeTuple *toArgTypes(Type *t);
     bool isReturnOnStack(TypeFunction *tf, bool needsThis);
     d_uns64 parameterSize(const Loc& loc, Type *t);
-    void applyInRefParams(TypeFunction *tf);
+    bool preferPassByRef(Type *t);
     Expression *getTargetInfo(const char* name, const Loc& loc);
 };
 

src/dmd/typesem.d

@@ -1491,12 +1491,16 @@ extern(C++) Type typeSemantic(Type type, const ref Loc loc, Scope* sc)
 
                 // Remove redundant storage classes for type, they are already applied
                 fparam.storageClass &= ~(STC.TYPECTOR);
-            }
 
-            // Now that we're done processing the types of parameters,
-            // apply `STC.ref` where necessary
-            if (global.params.previewIn)
-                target.applyInRefParams(tf);
+                // -preview=in: add `ref` storage class to suited `in` params
+                if (global.params.previewIn && (fparam.storageClass & (STC.in_ | STC.ref_)) == STC.in_)
+                {
+                    auto ts = t.baseElemOf().isTypeStruct();
+                    const isPOD = !ts || ts.sym.isPOD();
+                    if (!isPOD || target.preferPassByRef(t))
+                        fparam.storageClass |= STC.ref_;
+                }
+            }
 
             // Now that we completed semantic for the argument types,
             // run semantic on their default values,

test/compilable/previewin.d

@@ -1,4 +1,4 @@
-/* REQUIRED_ARGS: -preview=dip1000 -preview=in
+/* REQUIRED_ARGS: -preview=dip1000 -preview=in -mcpu=native
  */
 
 import core.stdc.time;
@@ -24,14 +24,13 @@ struct FooBar
     string toString() const
     {
         string result;
+        // Type is const
+        this.toString((in char[] buf) {
+            static assert(is(typeof(buf) == const(char[])));
+            result ~= buf;
+        });
         // Type inference works
-        this.toString((buf) { result ~= buf; });
-        // Specifying the STC too
         this.toString((in buf) { result ~= buf; });
-        // Some covariance
-        this.toString((const scope buf) { result ~= buf; });
-        this.toString((scope const(char)[] buf) { result ~= buf; });
-        this.toString((scope const(char[]) buf) { result ~= buf; });
         return result;
     }
 
@@ -64,3 +63,46 @@ void checkTemporary()
 LError:
     assert(0);
 }
+
+
+// Some ABI-specific tests:
+
+version (Win64)
+{
+    void checkReal(in real p)
+    {
+        // ref for x87 real, value for double-precision real
+        static assert(__traits(isRef, p) == (real.sizeof > 8));
+    }
+
+    struct RGB { ubyte r, g, b; }
+    void checkNonPowerOf2(in RGB p)
+    {
+        static assert(__traits(isRef, p));
+    }
+}
+else version (X86_64) // Posix x86_64
+{
+    struct Empty {} // 1 dummy byte passed on the stack
+    void checkEmptyStruct(in Empty p)
+    {
+        static assert(!__traits(isRef, p));
+    }
+
+    static if (is(__vector(double[4])))
+    {
+        struct AvxVectorWrapper { __vector(double[4]) a; } // 256 bits
+        void checkAvxVector(in AvxVectorWrapper p)
+        {
+            static assert(!__traits(isRef, p));
+        }
+    }
+}
+else version (AArch64)
+{
+    alias HVA = __vector(float[4])[4]; // can be passed in 4 vector registers
+    void checkHVA(in HVA p)
+    {
+        static assert(!__traits(isRef, p));
+    }
+}

test/fail_compilation/diagin.d

@@ -2,8 +2,8 @@
 PERMUTE_ARGS: -preview=in
 TEST_OUTPUT:
 ---
-fail_compilation/diagin.d(14): Error: function `diagin.foo(in string)` is not callable using argument types `()`
-fail_compilation/diagin.d(14):        missing argument for parameter #1: `in string`
+fail_compilation/diagin.d(14): Error: function `diagin.foo(in int)` is not callable using argument types `()`
+fail_compilation/diagin.d(14):        missing argument for parameter #1: `in int`
 fail_compilation/diagin.d(16): Error: template `diagin.foo1` cannot deduce function from argument types `!()(bool[])`, candidates are:
 fail_compilation/diagin.d(20):        `foo1(T)(in T v, string)`
 ---
@@ -16,10 +16,10 @@ void main ()
     foo1(lvalue);
 }
 
-void foo(in string) {}
+void foo(in int) {}
 void foo1(T)(in T v, string) {}
 
 // Ensure that `in` has a unique mangling
-static assert(foo.mangleof       == `_D6diagin3fooFIAyaZv`);
+static assert(foo.mangleof       == `_D6diagin3fooFIiZv`);
 static assert(foo1!int.mangleof  == `_D6diagin__T4foo1TiZQiFNaNbNiNfIiAyaZv`);
 static assert(foo1!char.mangleof == `_D6diagin__T4foo1TaZQiFNaNbNiNfIaAyaZv`);

test/fail_compilation/previewin.d

@@ -2,59 +2,33 @@
 REQUIRED_ARGS: -preview=in -preview=dip1000
 TEST_OUTPUT:
 ---
-fail_compilation/previewin.d(3): Error: function `previewin.func1(void function(ulong[8]) dg)` is not callable using argument types `(void function(in ulong[8]))`
-fail_compilation/previewin.d(3):        cannot pass argument `& func_byRef` of type `void function(in ulong[8])` to parameter `void function(ulong[8]) dg`
-fail_compilation/previewin.d(4): Error: function `previewin.func2(void function(ref ulong[8]) dg)` is not callable using argument types `(void function(in ulong[8]))`
-fail_compilation/previewin.d(4):        cannot pass argument `& func_byRef` of type `void function(in ulong[8])` to parameter `void function(ref ulong[8]) dg`
-fail_compilation/previewin.d(7): Error: function `previewin.func4(void function(ref uint) dg)` is not callable using argument types `(void function(in uint))`
-fail_compilation/previewin.d(7):        cannot pass argument `& func_byValue` of type `void function(in uint)` to parameter `void function(ref uint) dg`
-fail_compilation/previewin.d(41): Error: scope variable `arg` assigned to non-scope `myGlobal`
-fail_compilation/previewin.d(42): Error: scope variable `arg` assigned to non-scope `myGlobal`
-fail_compilation/previewin.d(43): Error: scope variable `arg` may not be returned
-fail_compilation/previewin.d(44): Error: scope variable `arg` assigned to `escape` with longer lifetime
-fail_compilation/previewin.d(48): Error: returning `arg` escapes a reference to parameter `arg`
-fail_compilation/previewin.d(48):        perhaps annotate the parameter with `return`
+fail_compilation/previewin.d(4): Error: function `previewin.takeFunction(void function(in real) f)` is not callable using argument types `(void function(real x) pure nothrow @nogc @safe)`
+fail_compilation/previewin.d(4):        cannot pass argument `__lambda1` of type `void function(real x) pure nothrow @nogc @safe` to parameter `void function(in real) f`
+fail_compilation/previewin.d(5): Error: function `previewin.takeFunction(void function(in real) f)` is not callable using argument types `(void function(const(real) x) pure nothrow @nogc @safe)`
+fail_compilation/previewin.d(5):        cannot pass argument `__lambda2` of type `void function(const(real) x) pure nothrow @nogc @safe` to parameter `void function(in real) f`
+fail_compilation/previewin.d(6): Error: function `previewin.takeFunction(void function(in real) f)` is not callable using argument types `(void function(ref const(real) x) pure nothrow @nogc @safe)`
+fail_compilation/previewin.d(6):        cannot pass argument `__lambda3` of type `void function(ref const(real) x) pure nothrow @nogc @safe` to parameter `void function(in real) f`
+fail_compilation/previewin.d(15): Error: scope variable `arg` assigned to non-scope `myGlobal`
+fail_compilation/previewin.d(16): Error: scope variable `arg` assigned to non-scope `myGlobal`
+fail_compilation/previewin.d(17): Error: scope variable `arg` may not be returned
+fail_compilation/previewin.d(18): Error: scope variable `arg` assigned to `escape` with longer lifetime
+fail_compilation/previewin.d(22): Error: returning `arg` escapes a reference to parameter `arg`
+fail_compilation/previewin.d(22):        perhaps annotate the parameter with `return`
 ---
  */
 
 #line 1
 void main ()
 {
-    func1(&func_byRef); // No
-    func2(&func_byRef); // No
-    func3(&func_byRef); // Could be Yes, but currently No
-
-    func4(&func_byValue); // No
-    func5(&func_byValue); // Yes
-
-    func6(&func_byValue2); // Yes
-    func7(&func_byValue3); // Yes
+    // No covariance without explicit `in`
+    takeFunction((real x) {});
+    takeFunction((const scope real x) {});
+    takeFunction((const scope ref real x) {});
 
     tryEscape("Hello World"); // Yes by `tryEscape` is NG
 }
 
-// Takes by `scope ref const`
-void func_byRef(in ulong[8]) {}
-// Takes by `scope const`
-void func_byValue(in uint) {}
-
-// Error: `ulong[8]` is passed by `ref`
-void func1(void function(scope ulong[8]) dg) {}
-// Error: Missing `scope` on a `ref`
-void func2(void function(ref ulong[8]) dg) {}
-// Works: `scope ref`
-void func3(void function(scope const ref ulong[8]) dg) {}
-
-// Error: `uint` is passed by value
-void func4(void function(ref uint) dg) {}
-// Works: By value `scope const`
-void func5(void function(scope const uint) dg) {}
-
-// This works for arrays:
-void func_byValue2(in char[]) {}
-void func6(void function(char[]) dg) {}
-void func_byValue3(scope const(char)[]) {}
-void func7(void function(in char[]) dg) {}
+void takeFunction(void function(in real) f);
 
 // Make sure things cannot be escaped (`scope` is applied)
 const(char)[] myGlobal;

test/runnable/previewin.d

@@ -13,11 +13,10 @@ void testWithAllAttributes() @safe pure nothrow @nogc
 
     // rvalues
     testin1(42);
-    testin2([42, ulong.max, ulong.min, 42UL]);
+    testin2((ulong[64]).init);
     testin3(ValueT(42));
     testin3(RefT(42));
-    testin4([ValueT(0), ValueT(1), ValueT(2), ValueT(3),
-             ValueT(4), ValueT(5), ValueT(6), ValueT(7)]);
+    testin4((ValueT[64]).init);
     testin4([RefT(42), RefT(84), RefT(126), RefT(4)]);
     testin5(NonCopyable(true));
     testin6(WithPostblit(true));
@@ -27,14 +26,14 @@ void testWithAllAttributes() @safe pure nothrow @nogc
     isTestOver = false;
 
     // lvalues
-    uint      a1;
-    ulong[4]  a2;
-    ValueT    a3;
-    ValueT[8] a4;
-    RefT      a5;
-    RefT[4]   a6;
+    uint       a1;
+    ulong[64]  a2;
+    ValueT     a3;
+    ValueT[64] a4;
+    RefT       a5;
+    RefT[4]    a6;
     NonCopyable  a7 = NonCopyable(true);
-    WithPostblit  a8;
+    WithPostblit a8;
     WithCopyCtor a9;
     WithDtor     a10 = WithDtor(&isTestOver);
 
@@ -118,7 +117,7 @@ void testForeach() @safe pure
 }
 
 struct ValueT { int value; }
-struct RefT   { ubyte[64] value; }
+struct RefT   { ulong[64] value; }
 
 struct NonCopyable
 {
@@ -155,25 +154,27 @@ struct WithDtor
 @safe pure nothrow @nogc:
 
 // By value
-void testin1(in uint p) {}
+void testin1(in uint p) { static assert(!__traits(isRef, p)); }
 // By ref because of size
-void testin2(in ulong[4] p) {}
+void testin2(in ulong[64] p) { static assert(__traits(isRef, p)); }
 // By value or ref depending on size
-void testin3(in ValueT p) {}
-void testin3(in RefT p) {}
+void testin3(in ValueT p) { static assert(!__traits(isRef, p)); }
+void testin3(in RefT p) { static assert(__traits(isRef, p)); }
 // By ref because of size
-void testin4(in ValueT[8] p) {}
-void testin4(in RefT[4] p) {}
+void testin4(in ValueT[64] p) { static assert(__traits(isRef, p)); }
+void testin4(in RefT[4] p) { static assert(__traits(isRef, p)); }
 
 // By ref because of non-copyability
-void testin5(in NonCopyable noncopy) {}
+void testin5(in NonCopyable noncopy) { static assert(__traits(isRef, noncopy)); }
+static assert(testin5.mangleof == "_D9previewin7testin5FNaNbNiNfIKSQBe11NonCopyableZv"); // incl. `ref`
 //  By ref because of postblit
-void testin6(in WithPostblit withposblit) {}
+void testin6(in WithPostblit withpostblit) { static assert(__traits(isRef, withpostblit)); }
 //  By ref because of copy ctor
-void testin7(in WithCopyCtor withcopy) {}
+void testin7(in WithCopyCtor withcopy) { static assert(__traits(isRef, withcopy)); }
 //  By ref because of dtor
 void testin8(in WithDtor withdtor, scope bool* isTestOver)
 {
+    static assert(__traits(isRef, withdtor));
     if (isTestOver)
         *isTestOver = true;
 }