From patchwork Wed Jun 7 01:32:14 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jason Merrill X-Patchwork-Id: 1791491 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@legolas.ozlabs.org Authentication-Results: legolas.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+incoming=patchwork.ozlabs.org@gcc.gnu.org; receiver=) Authentication-Results: legolas.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=LbPolJB/; 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 ECDSA (P-384) server-digest SHA384) (No client certificate requested) by legolas.ozlabs.org (Postfix) with ESMTPS id 4QbVG36xnwz20Q8 for ; Wed, 7 Jun 2023 11:32:43 +1000 (AEST) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id B445D3857705 for ; Wed, 7 Jun 2023 01:32:41 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org B445D3857705 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1686101561; bh=UoF0YvsjQh4GGf6/RGcGEb7dHF2AUOqVIVHBdxbaAkU=; h=To:Subject:Date:List-Id:List-Unsubscribe:List-Archive:List-Post: List-Help:List-Subscribe:From:Reply-To:From; b=LbPolJB/hnIWyw3pWQLNcsgyJmUYQYu/qgVQXJhGXDwvZG4qv90UHKHKoS3t5+iTc I0s6Es18NCKTMXEyKdK1mBBEkWnqVQoeTUgOL353GP1IPsK5D33p5K4CgU5yO+3qNf dmiPkWlnCkbtteLSE0TqsYsSuz2HMqDcvnPYLRiA= X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by sourceware.org (Postfix) with ESMTPS id C6BC63858C54 for ; Wed, 7 Jun 2023 01:32:20 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org C6BC63858C54 Received: from mail-qt1-f200.google.com (mail-qt1-f200.google.com [209.85.160.200]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-203--TVOug-1OqiwwXQWeYsjJA-1; Tue, 06 Jun 2023 21:32:18 -0400 X-MC-Unique: -TVOug-1OqiwwXQWeYsjJA-1 Received: by mail-qt1-f200.google.com with SMTP id d75a77b69052e-3f99bbd0d9dso30868381cf.0 for ; Tue, 06 Jun 2023 18:32:18 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1686101538; x=1688693538; h=content-transfer-encoding:mime-version:message-id:date:subject:to :from:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=UoF0YvsjQh4GGf6/RGcGEb7dHF2AUOqVIVHBdxbaAkU=; b=S/AG8tjmB1bb1lX1ygmsqdpzb+frQVb745d8kRGoqVzj81AneRodHSUokl5Zifi5vb s/aH+Kax33GdvQELZscLHZLP/iilwcBSurjKUIbw2lNoVsYWBFATXPRhPfeD+X2o0Dsm kDOp4/fDTAr4dTMllgz1GUD0cK6naINY3ZCWNb6hLIlAZB1OEbEJIEpVFUpiPVCADU6d GmLdLfohUFmqbN71mx64fDo+4Mr9CDV47KowLULArb97mayDYYCeZxTvdpuisaXDzuwN sgEbYAmhPK0R6LJ4vMeAKGcmpp7dD/RCqXHlvRnRJQ4yqm6VVhmUj21gIjYrAQj+BHGC 7WrA== X-Gm-Message-State: AC+VfDzks6s8xWulL2ag1Pqv8XtBf2ij5KNFNMWKT1XJJxWCXjROClgv H0Rvd9W0mSkb3TFNO36j/Ns6MojMaJMQr+ucUJMJfUzcHOEEnGH7EHn9hpEHojrty20mCpOo9L2 0v7FSWw18LBw7u7F/IyCXqJ52sx5hWly6D8rAfoW3EJkCuatRgGJv0bHlV5aj/o0iKsw51xr4jw == X-Received: by 2002:ac8:7d12:0:b0:3f9:bbb6:b566 with SMTP id g18-20020ac87d12000000b003f9bbb6b566mr847512qtb.1.1686101538087; Tue, 06 Jun 2023 18:32:18 -0700 (PDT) X-Google-Smtp-Source: ACHHUZ43s+nBafgVH/OMmc+RTQQuwHU491f6QBsHAqzAAdeAx/vAo4MmCvL4YFloFo9n/bHpQGGYRw== X-Received: by 2002:ac8:7d12:0:b0:3f9:bbb6:b566 with SMTP id g18-20020ac87d12000000b003f9bbb6b566mr847489qtb.1.1686101537600; Tue, 06 Jun 2023 18:32:17 -0700 (PDT) Received: from jason.com (130-44-146-16.s12558.c3-0.arl-cbr1.sbo-arl.ma.cable.rcncustomer.com. [130.44.146.16]) by smtp.gmail.com with ESMTPSA id b15-20020ac85bcf000000b003f6b8a6fd18sm5993361qtb.96.2023.06.06.18.32.16 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 06 Jun 2023 18:32:16 -0700 (PDT) To: gcc-patches@gcc.gnu.org Subject: [pushed] c++: enable NRVO from inner block [PR51571] Date: Tue, 6 Jun 2023 21:32:14 -0400 Message-Id: <20230607013214.2771266-1-jason@redhat.com> X-Mailer: git-send-email 2.31.1 MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com X-Spam-Status: No, score=-25.0 required=5.0 tests=BAYES_00, DKIM_INVALID, DKIM_SIGNED, GIT_PATCH_0, KAM_DMARC_NONE, KAM_DMARC_STATUS, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_NONE, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) 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: Jason Merrill via Gcc-patches From: Jason Merrill Reply-To: Jason Merrill Errors-To: gcc-patches-bounces+incoming=patchwork.ozlabs.org@gcc.gnu.org Sender: "Gcc-patches" Tested x86_64-pc-linux-gnu, applying to trunk. -- 8< -- Our implementation of the named return value optimization has been limited to variables declared in the outermost block of the function, to avoid needing to handle the case where the variable needs to be destroyed due to going out of scope. PR92407 pointed out a case we were missing, where the variable goes out of scope due to a goto and we were failing to destroy it. It occurred to me that this problem is the flip side of PR33799, where we need to be sure to destroy the return value if a cleanup throws on return; here we want to avoid destroying the return value when exiting the variable's scope on return. We can use the same flag to indicate to both cleanups that we're returning. This implements the guaranteed copy elision specified by P2025 (which is not yet part of the draft standard). PR c++/51571 PR c++/92407 gcc/cp/ChangeLog: * decl.cc (finish_function): Simplify NRV handling. * except.cc (maybe_set_retval_sentinel): Also set if NRV. (maybe_splice_retval_cleanup): Don't add the cleanup region if we don't need it. * semantics.cc (nrv_data): Add simple field. (finalize_nrv): Set it. (finalize_nrv_r): Check it and retval sentinel. * cp-tree.h (finalize_nrv): Adjust declaration. * typeck.cc (check_return_expr): Remove named_labels check. gcc/testsuite/ChangeLog: * g++.dg/opt/nrv23.C: New test. --- gcc/cp/cp-tree.h | 2 +- gcc/cp/decl.cc | 19 +++---------------- gcc/cp/except.cc | 12 ++++++++++-- gcc/cp/semantics.cc | 31 ++++++++++++++++++++++++++----- gcc/cp/typeck.cc | 3 --- gcc/testsuite/g++.dg/opt/nrv23.C | 23 +++++++++++++++++++++++ 6 files changed, 63 insertions(+), 27 deletions(-) create mode 100644 gcc/testsuite/g++.dg/opt/nrv23.C base-commit: 29c82c6ca929e0f5eccfe038dea71177d814c6b7 diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h index 834fdd12ef8..87572e3574d 100644 --- a/gcc/cp/cp-tree.h +++ b/gcc/cp/cp-tree.h @@ -7714,7 +7714,7 @@ extern bool check_accessibility_of_qualified_id (tree, tree, tree, tsubst_flags_ extern tree finish_qualified_id_expr (tree, tree, bool, bool, bool, bool, tsubst_flags_t); extern void simplify_aggr_init_expr (tree *); -extern void finalize_nrv (tree *, tree, tree); +extern void finalize_nrv (tree, tree); extern tree omp_reduction_id (enum tree_code, tree, tree); extern tree cp_remove_omp_priv_cleanup_stmt (tree *, int *, void *); extern bool cp_check_omp_declare_reduction (tree); diff --git a/gcc/cp/decl.cc b/gcc/cp/decl.cc index 3985c6d2d1f..c07a4a8d58d 100644 --- a/gcc/cp/decl.cc +++ b/gcc/cp/decl.cc @@ -18236,23 +18236,10 @@ finish_function (bool inline_p) /* Set up the named return value optimization, if we can. Candidate variables are selected in check_return_expr. */ - if (current_function_return_value) + if (tree r = current_function_return_value) { - tree r = current_function_return_value; - tree outer; - - if (r != error_mark_node - /* This is only worth doing for fns that return in memory--and - simpler, since we don't have to worry about promoted modes. */ - && aggregate_value_p (TREE_TYPE (TREE_TYPE (fndecl)), fndecl) - /* Only allow this for variables declared in the outer scope of - the function so we know that their lifetime always ends with a - return; see g++.dg/opt/nrv6.C. We could be more flexible if - we were to do this optimization in tree-ssa. */ - && (outer = outer_curly_brace_block (fndecl)) - && chain_member (r, BLOCK_VARS (outer))) - finalize_nrv (&DECL_SAVED_TREE (fndecl), r, DECL_RESULT (fndecl)); - + if (r != error_mark_node) + finalize_nrv (fndecl, r); current_function_return_value = NULL_TREE; } diff --git a/gcc/cp/except.cc b/gcc/cp/except.cc index 28106dadf1e..6c0f0815424 100644 --- a/gcc/cp/except.cc +++ b/gcc/cp/except.cc @@ -1280,7 +1280,9 @@ build_noexcept_spec (tree expr, tsubst_flags_t complain) /* If the current function has a cleanup that might throw, and the return value has a non-trivial destructor, return a MODIFY_EXPR to set current_retval_sentinel so that we know that the return value needs to be - destroyed on throw. Otherwise, returns NULL_TREE. */ + destroyed on throw. Do the same if the current function might use the + named return value optimization, so we don't destroy it on return. + Otherwise, returns NULL_TREE. */ tree maybe_set_retval_sentinel () @@ -1290,7 +1292,9 @@ maybe_set_retval_sentinel () tree retval = DECL_RESULT (current_function_decl); if (!TYPE_HAS_NONTRIVIAL_DESTRUCTOR (TREE_TYPE (retval))) return NULL_TREE; - if (!cp_function_chain->throwing_cleanup) + if (!cp_function_chain->throwing_cleanup + && (current_function_return_value == error_mark_node + || current_function_return_value == NULL_TREE)) return NULL_TREE; if (!current_retval_sentinel) @@ -1338,6 +1342,10 @@ maybe_splice_retval_cleanup (tree compound_stmt, bool is_try) tsi_link_before (&iter, decl_expr, TSI_SAME_STMT); } + if (!cp_function_chain->throwing_cleanup) + /* We're only using the sentinel for an NRV. */ + return; + /* Skip past other decls, they can't contain a return. */ while (TREE_CODE (tsi_stmt (iter)) == DECL_EXPR) tsi_next (&iter); diff --git a/gcc/cp/semantics.cc b/gcc/cp/semantics.cc index a13c16f34a3..1d397b6f257 100644 --- a/gcc/cp/semantics.cc +++ b/gcc/cp/semantics.cc @@ -4938,6 +4938,7 @@ public: tree var; tree result; hash_table > visited; + bool simple; }; /* Helper function for walk_tree, used by finalize_nrv below. */ @@ -4952,6 +4953,9 @@ finalize_nrv_r (tree* tp, int* walk_subtrees, void* data) non-statements, except that we have to consider STMT_EXPRs. */ if (TYPE_P (*tp)) *walk_subtrees = 0; + /* If there's a label, we might need to destroy the NRV on goto (92407). */ + else if (TREE_CODE (*tp) == LABEL_EXPR) + dp->simple = false; /* Change all returns to just refer to the RESULT_DECL; this is a nop, but differs from using NULL_TREE in that it indicates that we care about the value of the RESULT_DECL. But preserve anything appended @@ -4965,11 +4969,20 @@ finalize_nrv_r (tree* tp, int* walk_subtrees, void* data) && TREE_OPERAND (*p, 0) == dp->result); *p = dp->result; } - /* Change all cleanups for the NRV to only run when an exception is - thrown. */ + /* Change all cleanups for the NRV to only run when not returning. */ else if (TREE_CODE (*tp) == CLEANUP_STMT && CLEANUP_DECL (*tp) == dp->var) - CLEANUP_EH_ONLY (*tp) = 1; + { + if (dp->simple) + CLEANUP_EH_ONLY (*tp) = true; + else + { + tree cond = build3 (COND_EXPR, void_type_node, + current_retval_sentinel, + void_node, CLEANUP_EXPR (*tp)); + CLEANUP_EXPR (*tp) = cond; + } + } /* Replace the DECL_EXPR for the NRV with an initialization of the RESULT_DECL, if needed. */ else if (TREE_CODE (*tp) == DECL_EXPR @@ -5009,9 +5022,10 @@ finalize_nrv_r (tree* tp, int* walk_subtrees, void* data) RESULT_DECL for the function. */ void -finalize_nrv (tree *tp, tree var, tree result) +finalize_nrv (tree fndecl, tree var) { class nrv_data data; + tree result = DECL_RESULT (fndecl); /* Copy name from VAR to RESULT. */ DECL_NAME (result) = DECL_NAME (var); @@ -5025,7 +5039,14 @@ finalize_nrv (tree *tp, tree var, tree result) data.var = var; data.result = result; - cp_walk_tree (tp, finalize_nrv_r, &data, 0); + + /* This is simpler for variables declared in the outer scope of + the function so we know that their lifetime always ends with a + return; see g++.dg/opt/nrv6.C. */ + tree outer = outer_curly_brace_block (fndecl); + data.simple = chain_member (var, BLOCK_VARS (outer)); + + cp_walk_tree (&DECL_SAVED_TREE (fndecl), finalize_nrv_r, &data, 0); } /* Create CP_OMP_CLAUSE_INFO for clause C. Returns true if it is invalid. */ diff --git a/gcc/cp/typeck.cc b/gcc/cp/typeck.cc index 6618c6a2021..11fcc7fcd3b 100644 --- a/gcc/cp/typeck.cc +++ b/gcc/cp/typeck.cc @@ -11155,9 +11155,6 @@ check_return_expr (tree retval, bool *no_warning) if (fn_returns_value_p && flag_elide_constructors) { if (named_return_value_okay_p - /* The current NRV implementation breaks if a backward goto needs to - destroy the object (PR92407). */ - && !cp_function_chain->x_named_labels && (current_function_return_value == NULL_TREE || current_function_return_value == bare_retval)) current_function_return_value = bare_retval; diff --git a/gcc/testsuite/g++.dg/opt/nrv23.C b/gcc/testsuite/g++.dg/opt/nrv23.C new file mode 100644 index 00000000000..9e1253cd830 --- /dev/null +++ b/gcc/testsuite/g++.dg/opt/nrv23.C @@ -0,0 +1,23 @@ +// PR c++/51571 +// { dg-do link } + +int copies; + +struct A { + int i; + A(int i) : i(i) { } + A(A const &); // not defined +}; + +A h() +{ + { + A a(0); + return a; + } +} + +int main() +{ + A c = h(); +}