From patchwork Wed Aug 26 08:15:20 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Iain Buclaw X-Patchwork-Id: 1351702 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=gcc.gnu.org (client-ip=2620:52:3:1:0:246e:9693:128c; helo=sourceware.org; envelope-from=gcc-patches-bounces@gcc.gnu.org; receiver=) Authentication-Results: ozlabs.org; dmarc=none (p=none dis=none) header.from=gcc.gnu.org Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.a=rsa-sha256 header.s=default header.b=j5WldvBv; dkim-atps=neutral Received: from sourceware.org (server2.sourceware.org [IPv6:2620:52:3:1:0:246e:9693:128c]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 4BbzFL66Jnz9sSJ for ; Wed, 26 Aug 2020 18:15:34 +1000 (AEST) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id A28453850431; Wed, 26 Aug 2020 08:15:32 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org A28453850431 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1598429732; bh=ja3vIfCmSv6YySmN+5Eb8ZoeCw9XErY4GA4gDIWSlu4=; h=To:Subject:Date:List-Id:List-Unsubscribe:List-Archive:List-Post: List-Help:List-Subscribe:From:Reply-To:From; b=j5WldvBvdawmobiLXti7A1UN0fWoKS91YpGgz/wVdNI+OR3zYNDIIiWG3YJ7A3MD2 j3BrMu6udAsgjZ55GgUccgFBZjli5GuN4aXjdkoVWx0Z7bdPZ47PrggLMpednxe4wO XHHH400ve8FIAxMdDxH4brEWEs3Nn5nSX4G+yQFM= X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from mout-p-201.mailbox.org (mout-p-201.mailbox.org [IPv6:2001:67c:2050::465:201]) by sourceware.org (Postfix) with ESMTPS id 4CC6E3850431 for ; Wed, 26 Aug 2020 08:15:28 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 4CC6E3850431 Received: from smtp1.mailbox.org (smtp1.mailbox.org [IPv6:2001:67c:2050:105:465:1:1:0]) (using TLSv1.2 with cipher ECDHE-RSA-CHACHA20-POLY1305 (256/256 bits)) (No client certificate requested) by mout-p-201.mailbox.org (Postfix) with ESMTPS id 4BbzFC2dHvzQlWc; Wed, 26 Aug 2020 10:15:27 +0200 (CEST) X-Virus-Scanned: amavisd-new at heinlein-support.de Received: from smtp1.mailbox.org ([80.241.60.240]) by hefe.heinlein-support.de (hefe.heinlein-support.de [91.198.250.172]) (amavisd-new, port 10030) with ESMTP id tmGplSOTKx4X; Wed, 26 Aug 2020 10:15:22 +0200 (CEST) To: gcc-patches@gcc.gnu.org Subject: [committed] d: Fix no RVO when returning struct literals initialized with constructor. Date: Wed, 26 Aug 2020 10:15:20 +0200 Message-Id: <20200826081520.451295-1-ibuclaw@gdcproject.org> MIME-Version: 1.0 X-MBO-SPAM-Probability: ***** X-Rspamd-Score: 6.28 / 15.00 / 15.00 X-Rspamd-Queue-Id: CF0271373 X-Rspamd-UID: 575875 X-Spam-Status: No, score=-15.1 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, KAM_SHORT, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP, URIBL_ABUSE_SURBL autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-Patchwork-Original-From: Iain Buclaw via Gcc-patches From: Iain Buclaw Reply-To: Iain Buclaw Errors-To: gcc-patches-bounces@gcc.gnu.org Sender: "Gcc-patches" Hi, This patch 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. Regstrapped on x86_64-linux-gnu/-m32/-mx32, committed to mainline. Regards Iain --- 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 | 12 +++++- gcc/d/decl.cc | 25 ++++------- gcc/d/dmd/MERGE | 2 +- gcc/d/dmd/declaration.h | 1 + gcc/d/dmd/func.c | 52 +++++++++++++++++++++-- gcc/d/dmd/statementsem.c | 33 --------------- gcc/d/expr.cc | 14 ++++++- gcc/d/toir.cc | 56 ++++++++++++++++++++++--- gcc/testsuite/gdc.dg/pr96156.d | 33 +++++++++++++++ gcc/testsuite/gdc.test/runnable/sdtor.d | 5 ++- gcc/testsuite/gdc.test/runnable/test8.d | 8 ++-- 11 files changed, 171 insertions(+), 70 deletions(-) create mode 100644 gcc/testsuite/gdc.dg/pr96156.d diff --git a/gcc/d/d-frontend.cc b/gcc/d/d-frontend.cc index 3b9fc1aaf2a..da34e902275 100644 --- a/gcc/d/d-frontend.cc +++ b/gcc/d/d-frontend.cc @@ -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. */ diff --git a/gcc/d/decl.cc b/gcc/d/decl.cc index 295f780170a..008a2bb16ab 100644 --- a/gcc/d/decl.cc +++ b/gcc/d/decl.cc @@ -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); } } diff --git a/gcc/d/dmd/MERGE b/gcc/d/dmd/MERGE index 2dc2ef3d622..276406c7696 100644 --- a/gcc/d/dmd/MERGE +++ b/gcc/d/dmd/MERGE @@ -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. diff --git a/gcc/d/dmd/declaration.h b/gcc/d/dmd/declaration.h index 251c407bf8c..65ac3f75b27 100644 --- a/gcc/d/dmd/declaration.h +++ b/gcc/d/dmd/declaration.h @@ -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; } diff --git a/gcc/d/dmd/func.c b/gcc/d/dmd/func.c index 30ba8dd93be..f8e9601dbec 100644 --- a/gcc/d/dmd/func.c +++ b/gcc/d/dmd/func.c @@ -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"; diff --git a/gcc/d/dmd/statementsem.c b/gcc/d/dmd/statementsem.c index 599c52e754c..90493475fff 100644 --- a/gcc/d/dmd/statementsem.c +++ b/gcc/d/dmd/statementsem.c @@ -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) { diff --git a/gcc/d/expr.cc b/gcc/d/expr.cc index 85407ac7eb0..2b13615521d 100644 --- a/gcc/d/expr.cc +++ b/gcc/d/expr.cc @@ -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; } diff --git a/gcc/d/toir.cc b/gcc/d/toir.cc index fd18b9e6a76..f9bc0958af3 100644 --- a/gcc/d/toir.cc +++ b/gcc/d/toir.cc @@ -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: ( = expr, return ); */ + 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 index 00000000000..d9359b5bcff --- /dev/null +++ b/gcc/testsuite/gdc.dg/pr96156.d @@ -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); +} diff --git a/gcc/testsuite/gdc.test/runnable/sdtor.d b/gcc/testsuite/gdc.test/runnable/sdtor.d index b7766297f14..ed58bcff9c8 100644 --- a/gcc/testsuite/gdc.test/runnable/sdtor.d +++ b/gcc/testsuite/gdc.test/runnable/sdtor.d @@ -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(); diff --git a/gcc/testsuite/gdc.test/runnable/test8.d b/gcc/testsuite/gdc.test/runnable/test8.d index 932a0cf4ca4..f8b800f18bd 100644 --- a/gcc/testsuite/gdc.test/runnable/test8.d +++ b/gcc/testsuite/gdc.test/runnable/test8.d @@ -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);