]> git.ipfire.org Git - thirdparty/gcc.git/commitdiff
d: Fix no RVO when returning struct literals initialized with constructor.
authorIain Buclaw <ibuclaw@gdcproject.org>
Fri, 24 Jul 2020 16:06:51 +0000 (18:06 +0200)
committerIain Buclaw <ibuclaw@gdcproject.org>
Wed, 26 Aug 2020 08:03:56 +0000 (10:03 +0200)
Backports a change from upstream dmd that moves front-end NRVO checking
from ReturnStatement semantic to the end of FuncDeclaration semantic.

In the codegen, retStyle has been partially implemented so that only
structs and static arrays return RETstack.  This isn't accurate, but
don't need to be for the purposes of semantic analysis.

If a function either has TREE_ADDRESSABLE or must return in memory, then
DECL_RESULT is set as the shidden field for the function.  This is used
in the codegen pass for ReturnStatement where it is now detected whether
a function is returning a struct literal or a constructor function, then
the DECL_RESULT is used to directly construct the return value, instead
of doing so via temporaries.

Reviewed-on: https://github.com/dlang/dmd/pull/11622

gcc/d/ChangeLog:

PR d/96156
* d-frontend.cc (retStyle): Only return RETstack for struct and static
array types.
* decl.cc (DeclVisitor::visit (FuncDeclaration *)): Use NRVO return
for all TREE_ADDRESSABLE types.  Set shidden to the RESULT_DECL.
* expr.cc (ExprVisitor::visit (CallExp *)): Force TARGET_EXPR if the
'this' pointer reference is a CONSTRUCTOR.
(ExprVisitor::visit (StructLiteralExp *)): Generate assignment to the
symbol to initialize with literal.
* toir.cc (IRVisitor::visit (ReturnStatement *)): Detect returning
struct literals and write directly into the RESULT_DECL.
* dmd/MERGE: Merge upstream dmd fe5f388d8.

gcc/testsuite/ChangeLog:

PR d/96156
* gdc.dg/pr96156.d: New test.

gcc/d/d-frontend.cc
gcc/d/decl.cc
gcc/d/dmd/MERGE
gcc/d/dmd/declaration.h
gcc/d/dmd/func.c
gcc/d/dmd/statementsem.c
gcc/d/expr.cc
gcc/d/toir.cc
gcc/testsuite/gdc.dg/pr96156.d [new file with mode: 0644]
gcc/testsuite/gdc.test/runnable/sdtor.d
gcc/testsuite/gdc.test/runnable/test8.d

index 3b9fc1aaf2aa1304691fd562e518949e50723188..da34e90227584fd4f54ae0a9fc4914110762e84a 100644 (file)
@@ -143,12 +143,20 @@ Loc::equals (const Loc &loc)
    hidden pointer to the caller's stack.  */
 
 RET
-retStyle (TypeFunction *)
+retStyle (TypeFunction *tf)
 {
   /* Need the backend type to determine this, but this is called from the
      frontend before semantic processing is finished.  An accurate value
      is not currently needed anyway.  */
-  return RETstack;
+  if (tf->isref)
+    return RETregs;
+
+  Type *tn = tf->next->toBasetype ();
+
+  if (tn->ty == Tstruct || tn->ty == Tsarray)
+    return RETstack;
+
+  return RETregs;
 }
 
 /* Determine if function FD is a builtin one that we can evaluate in CTFE.  */
index 295f780170a800e7aac9f056492b9e4264fc5871..008a2bb16ab98215417502e219be30c49c7185cd 100644 (file)
@@ -944,20 +944,11 @@ public:
        Implemented by overriding all the RETURN_EXPRs and replacing all
        occurrences of VAR with the RESULT_DECL for the function.
        This is only worth doing for functions that can return in memory.  */
-    if (d->nrvo_can)
-      {
-       tree restype = TREE_TYPE (DECL_RESULT (fndecl));
-
-       if (!AGGREGATE_TYPE_P (restype))
-         d->nrvo_can = 0;
-       else
-         d->nrvo_can = aggregate_value_p (restype, fndecl);
-      }
+    tree resdecl = DECL_RESULT (fndecl);
 
-    if (d->nrvo_can)
+    if (TREE_ADDRESSABLE (TREE_TYPE (resdecl))
+       || aggregate_value_p (TREE_TYPE (resdecl), fndecl))
       {
-       tree resdecl = DECL_RESULT (fndecl);
-
        /* Return non-trivial structs by invisible reference.  */
        if (TREE_ADDRESSABLE (TREE_TYPE (resdecl)))
          {
@@ -965,9 +956,12 @@ public:
            DECL_BY_REFERENCE (resdecl) = 1;
            TREE_ADDRESSABLE (resdecl) = 0;
            relayout_decl (resdecl);
+           d->shidden = build_deref (resdecl);
          }
+       else
+         d->shidden = resdecl;
 
-       if (d->nrvo_var)
+       if (d->nrvo_can && d->nrvo_var)
          {
            tree var = get_symbol_decl (d->nrvo_var);
 
@@ -976,12 +970,9 @@ public:
            /* Don't forget that we take its address.  */
            TREE_ADDRESSABLE (var) = 1;
 
-           if (DECL_BY_REFERENCE (resdecl))
-             resdecl = build_deref (resdecl);
-
            SET_DECL_VALUE_EXPR (var, resdecl);
            DECL_HAS_VALUE_EXPR_P (var) = 1;
-           SET_DECL_LANG_NRVO (var, resdecl);
+           SET_DECL_LANG_NRVO (var, d->shidden);
          }
       }
 
index 2dc2ef3d6223a5e302ebf43e5784343856cf7424..276406c76961cedb1424f285815d9f71aa770ab4 100644 (file)
@@ -1,4 +1,4 @@
-cb4a96faecb6b521ac4749581bcfa39f61143db0
+fe5f388d8e5d97dccaa4ef1349f931c36a2cbc46
 
 The first line of this file holds the git revision number of the last
 merge done from the dlang/dmd repository.
index 251c407bf8c301897cf4b6aeacbc96ea5745c538..65ac3f75b2780b8d306e9dc6afc3fd527945eb72 100644 (file)
@@ -669,6 +669,7 @@ public:
     static FuncDeclaration *genCfunc(Parameters *args, Type *treturn, const char *name, StorageClass stc=0);
     static FuncDeclaration *genCfunc(Parameters *args, Type *treturn, Identifier *id, StorageClass stc=0);
     void checkDmain();
+    bool checkNrvo();
 
     FuncDeclaration *isFuncDeclaration() { return this; }
 
index 30ba8dd93be95c3c0c05d29256cc823aa9285ea1..f8e9601dbec875553523836bd67e159bb490443c 100644 (file)
@@ -1704,9 +1704,6 @@ void FuncDeclaration::semantic3(Scope *sc)
                 }
             }
 
-            if (!inferRetType && retStyle(f) != RETstack)
-                nrvo_can = 0;
-
             bool inferRef = (f->isref && (storage_class & STCauto));
 
             fbody = ::semantic(fbody, sc2);
@@ -1761,7 +1758,7 @@ void FuncDeclaration::semantic3(Scope *sc)
                 if (storage_class & STCauto)
                     storage_class &= ~STCauto;
             }
-            if (retStyle(f) != RETstack)
+            if (retStyle(f) != RETstack || checkNrvo())
                 nrvo_can = 0;
 
             if (fbody->isErrorStatement())
@@ -4293,6 +4290,53 @@ void FuncDeclaration::checkDmain()
         error("parameters must be main() or main(string[] args)");
 }
 
+/***********************************************
+ * Check all return statements for a function to verify that returning
+ * using NRVO is possible.
+ *
+ * Returns:
+ *      true if the result cannot be returned by hidden reference.
+ */
+bool FuncDeclaration::checkNrvo()
+{
+    if (!nrvo_can)
+        return true;
+
+    if (returns == NULL)
+        return true;
+
+    TypeFunction *tf = type->toTypeFunction();
+    if (tf->isref)
+        return true;
+
+    for (size_t i = 0; i < returns->length; i++)
+    {
+        ReturnStatement *rs = (*returns)[i];
+
+        if (VarExp *ve = rs->exp->isVarExp())
+        {
+            VarDeclaration *v = ve->var->isVarDeclaration();
+            if (!v || v->isOut() || v->isRef())
+                return true;
+            else if (nrvo_var == NULL)
+            {
+                if (!v->isDataseg() && !v->isParameter() && v->toParent2() == this)
+                {
+                    //printf("Setting nrvo to %s\n", v->toChars());
+                    nrvo_var = v;
+                }
+                else
+                    return true;
+            }
+            else if (nrvo_var != v)
+                return true;
+        }
+        else //if (!exp->isLvalue())    // keep NRVO-ability
+            return true;
+    }
+    return false;
+}
+
 const char *FuncDeclaration::kind() const
 {
     return generated ? "generated function" : "function";
index 599c52e754c4fe3c635208a50f20df51169920d3..90493475fffcd6069ee8aa11e37400f1413a34d8 100644 (file)
@@ -2861,42 +2861,9 @@ public:
                  *    return x; return 3;  // ok, x can be a value
                  */
             }
-
-            // handle NRVO
-            if (fd->nrvo_can && rs->exp->op == TOKvar)
-            {
-                VarExp *ve = (VarExp *)rs->exp;
-                VarDeclaration *v = ve->var->isVarDeclaration();
-
-                if (tf->isref)
-                {
-                    // Function returns a reference
-                    if (!inferRef)
-                        fd->nrvo_can = 0;
-                }
-                else if (!v || v->isOut() || v->isRef())
-                    fd->nrvo_can = 0;
-                else if (fd->nrvo_var == NULL)
-                {
-                    if (!v->isDataseg() && !v->isParameter() && v->toParent2() == fd)
-                    {
-                        //printf("Setting nrvo to %s\n", v->toChars());
-                        fd->nrvo_var = v;
-                    }
-                    else
-                        fd->nrvo_can = 0;
-                }
-                else if (fd->nrvo_var != v)
-                    fd->nrvo_can = 0;
-            }
-            else //if (!exp->isLvalue())    // keep NRVO-ability
-                fd->nrvo_can = 0;
         }
         else
         {
-            // handle NRVO
-            fd->nrvo_can = 0;
-
             // infer return type
             if (fd->inferRetType)
             {
index 85407ac7eb05523722e746e1930142a78a5f7934..2b13615521dd0398548dac08786a27170578c60d 100644 (file)
@@ -1782,6 +1782,9 @@ public:
                    thisexp = TREE_OPERAND (thisexp, 1);
                  }
 
+               if (TREE_CODE (thisexp) == CONSTRUCTOR)
+                 thisexp = force_target_expr (thisexp);
+
                /* Want reference to `this' object.  */
                if (!POINTER_TYPE_P (TREE_TYPE (thisexp)))
                  thisexp = build_address (thisexp);
@@ -2886,7 +2889,16 @@ public:
        processing has complete.  Build the static initializer now.  */
     if (e->useStaticInit && !this->constp_)
       {
-       this->result_ = aggregate_initializer_decl (e->sd);
+       tree init = aggregate_initializer_decl (e->sd);
+
+       /* If initializing a symbol, don't forget to set it.  */
+       if (e->sym != NULL)
+         {
+           tree var = build_deref (e->sym);
+           init = compound_expr (modify_expr (var, init), var);
+         }
+
+       this->result_ = init;
        return;
       }
 
index fd18b9e6a763024d8f78835e5c19ae89693600b9..f9bc0958af33023589ccf9fe1a9e53f71378de52 100644 (file)
@@ -1015,19 +1015,63 @@ public:
        && type->toBasetype ()->ty == Tvoid)
       type = Type::tint32;
 
-    if (this->func_->nrvo_can && this->func_->nrvo_var)
+    if (this->func_->shidden)
       {
-       /* Just refer to the DECL_RESULT; this differs from using
-          NULL_TREE in that it indicates that we care about the value
-          of the DECL_RESULT.  */
+       /* Returning by hidden reference, store the result into the retval decl.
+          The result returned then becomes the retval reference itself.  */
        tree decl = DECL_RESULT (get_symbol_decl (this->func_));
+       gcc_assert (!tf->isref);
+
+       /* If returning via NRVO, just refer to the DECL_RESULT; this differs
+          from using NULL_TREE in that it indicates that we care about the
+          value of the DECL_RESULT.  */
+       if (this->func_->nrvo_can && this->func_->nrvo_var)
+         {
+           add_stmt (return_expr (decl));
+           return;
+         }
+
+       /* Detect a call to a constructor function, or if returning a struct
+          literal, write result directly into the return value.  */
+       StructLiteralExp *sle = NULL;
+
+       if (DotVarExp *dve = (s->exp->op == TOKcall
+                             && s->exp->isCallExp ()->e1->op == TOKdotvar
+                             ? s->exp->isCallExp ()->e1->isDotVarExp ()
+                             : NULL))
+         {
+           sle = (dve->var->isCtorDeclaration ()
+                  ? dve->e1->isStructLiteralExp () : NULL);
+         }
+       else
+         sle = s->exp->isStructLiteralExp ();
+
+       if (sle != NULL)
+         {
+           StructDeclaration *sd = type->baseElemOf ()->isTypeStruct ()->sym;
+           sle->sym = build_address (this->func_->shidden);
+
+           /* Fill any alignment holes in the return slot using memset.  */
+           if (!identity_compare_p (sd) || sd->isUnionDeclaration ())
+             add_stmt (build_memset_call (this->func_->shidden));
+
+           add_stmt (build_expr_dtor (s->exp));
+         }
+       else
+         {
+           /* Generate: (<retval> = expr, return <retval>);  */
+           tree expr = build_expr_dtor (s->exp);
+           tree init = stabilize_expr (&expr);
+           expr = build_assign (INIT_EXPR, this->func_->shidden, expr);
+           add_stmt (compound_expr (init, expr));
+         }
+
        add_stmt (return_expr (decl));
       }
     else
       {
        /* Convert for initializing the DECL_RESULT.  */
-       tree expr = build_return_dtor (s->exp, type, tf);
-       add_stmt (expr);
+       add_stmt (build_return_dtor (s->exp, type, tf));
       }
   }
 
diff --git a/gcc/testsuite/gdc.dg/pr96156.d b/gcc/testsuite/gdc.dg/pr96156.d
new file mode 100644 (file)
index 0000000..d9359b5
--- /dev/null
@@ -0,0 +1,33 @@
+// https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96156
+// { dg-do run { target hw } }
+// { dg-skip-if "needs gcc/config.d" { ! d_runtime } }
+
+struct S96156
+{
+    __gshared void* ptr;
+    int x;
+
+    this(int x) { ptr = &this; this.x = x; }
+    @disable this(this);
+}
+
+auto f96156()
+{
+    return g96156();
+}
+
+auto g96156()
+{
+    return h96156();
+}
+
+auto h96156()
+{
+    return S96156(100);
+}
+
+void main()
+{
+    auto s = f96156();
+    assert(&s == S96156.ptr);
+}
index b7766297f144e289aaa2fc6f382f1bbe837af118..ed58bcff9c8614d87cc00fbab9c56004b1fb314d 100644 (file)
@@ -1,4 +1,4 @@
-// PERMUTE_ARGS: -unittest -O -release -inline -g
+// PERMUTE_ARGS: -unittest -O -release -inline -fPIC -g
 
 import core.vararg;
 
@@ -2827,6 +2827,7 @@ struct S17457 {
     this(int seconds) {
         dg17457 = &mfunc;
     }
+    @disable this(this);
     void mfunc() {}
 }
 
@@ -4612,7 +4613,7 @@ int main()
     test9899();
     test9907();
     test9985();
-    //test17457();    // XBUG: NRVO implementation differs
+    test17457();
     test9994();
     test10094();
     test10244();
index 932a0cf4ca4766426eb46067a1879e780343f999..f8b800f18bd1d861578b96b15f01c2467e759e4e 100644 (file)
@@ -746,19 +746,19 @@ int foo42(const(char) *x, ...)
     va_list ap;
 
     va_start!(typeof(x))(ap, x);
-    //printf("&x = %p, ap = %p\n", &x, ap);     // XBUG: static array va_lists (eg: x86_64) cannot be passed as vararg.
+    assert(ap != va_list.init);
 
     int i;
     i = va_arg!(typeof(i))(ap);
-    printf("i = %d\n", i);
+    assert(i == 3);
 
     long l;
     l = va_arg!(typeof(l))(ap);
-    printf("l = %lld\n", l);
+    assert(l == 23);
 
     uint k;
     k = va_arg!(typeof(k))(ap);
-    printf("k = %u\n", k);
+    assert(k == 4);
 
     va_end(ap);