[gcc r14-1593] c++: enable NRVO from inner block [PR51571]

[gcc r14-1593] c++: enable NRVO from inner block [PR51571]

[Jason Merrill]

https://gcc.gnu.org/g:7e0b65b239c3a0d68ce94896b236b03de666ffd6

commit r14-1593-g7e0b65b239c3a0d68ce94896b236b03de666ffd6
Author: Jason Merrill <jason@redhat.com>
Date:   Sun Jun 4 12:09:11 2023 -0400

    c++: enable NRVO from inner block [PR51571]
    
    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.

Diff:
---
 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(-)

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<nofree_ptr_hash <tree_node> > 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);
 }
 \f
 /* 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();
+}