coroutines: Update to n4849 allocation/deallocation.

Message ID CE758DFA-319E-4E11-8BAB-0654DA4DE002@sandoe.co.uk
State New
Headers show
Series
  • coroutines: Update to n4849 allocation/deallocation.
Related show

Commit Message

Iain Sandoe Feb. 7, 2020, 12:06 p.m.
Hi,

This is the first of the (small number of) anticipated changes to bring
the implementation into line with the latest published C++20 draft (this
is now expected to be very close to the final, although some wording
might still be adjusted).

The allocation for the coroutine state frame is quite flexible - which means
in reality a number of permutations to cater for and to test for.

Partial or complete adoption of p2014r0 would be additive to this, but it
seems better to me to separate changes for that to a future patch.

Because of the number of permutations, the error messages are somewhat
long-winded in an effort to point the user to the specific problem.

OK for trunk?
thanks
Iain

-- 
2.24.1

Comments

Nathan Sidwell Feb. 11, 2020, 7:31 a.m. | #1
On 2/7/20 1:06 PM, Iain Sandoe wrote:
> Hi,

> 

> This is the first of the (small number of) anticipated changes to bring

> the implementation into line with the latest published C++20 draft (this

> is now expected to be very close to the final, although some wording

> might still be adjusted).

> 

> The allocation for the coroutine state frame is quite flexible - which means

> in reality a number of permutations to cater for and to test for.

> 

> Partial or complete adoption of p2014r0 would be additive to this, but it

> seems better to me to separate changes for that to a future patch.

> 

> Because of the number of permutations, the error messages are somewhat

> long-winded in an effort to point the user to the specific problem.

> 

> OK for trunk?

> thanks

> Iain

> 

> ========

> 

> This updates the coroutine frame allocation and deallocation usage to

> match n4849.

> 

> [dcl.fct.def.coroutine] /9, /10, /12.

> 

> 9 An implementation may need to allocate additional storage for a coroutine.

>    This storage is known as the coroutine state and is obtained by calling a

>    non-array allocation function.  The allocation function’s name is looked up

>    in the scope of the promise type.  If this lookup fails, the allocation

>    function’s name is looked up in the global scope.  If the lookup finds an

>    allocation function in the scope of the promise type, overload resolution

>    is performed on a function call created by assembling an argument list.

>    The first argument is the amount of space requested, and has type

>    std::size_t.  The lvalues p1 . . . pn are the succeeding [user's function]

>    arguments. If no viable function is found, overload resolution is performed

>    again on a function call created by passing just the amount of space required

>    as an argument of type std::size_t.

> 

> 10 The unqualified-id get_return_object_on_allocation_failure is looked up in

>    the scope of the promise type by class member access lookup. If any

>    declarations are found, then the result of a call to an allocation function

>    used to obtain storage for the coroutine state is assumed to return nullptr

>    if it fails to obtain storage, and if a global allocation function is

>    selected, the ::operator new(size_t, nothrow_t) form is used. The allocation

>    function used in this case shall have a non-throwing noexcept-specification.

>    If the allocation function returns nullptr, the coroutine returns control to

>    the caller of the coroutine and the return value is obtained by a call to

>    T::get_return_object_on_allocation_failure(), where T is the promise type.

> 

> 12 The deallocation function’s name is looked up in the scope of the promise

>    type. If this lookup fails, the deallocation function’s name is looked up in

>    the global scope.  If deallocation function lookup finds both a usual

>    deallocation function with only a pointer parameter and a usual deallocation

>    function with both a pointer parameter and a size parameter, then the

>    selected deallocation function shall be the one with two parameters.

>    Otherwise, the selected deallocation function shall be the function with one

>    parameter. If no usual deallocation function is found, the program is ill-

>    formed.  The selected deallocation function shall be called with the address

>    of the block of storage to be reclaimed as its first argument.  If a

>    deallocation function with a parameter of type std::size_t is used, the size

>    of the block is passed as the corresponding argument.

> 

> gcc/cp/ChangeLog:

> 

> 2020-02-07  Iain Sandoe  <iain@sandoe.co.uk>

> 

> 	* coroutines.cc (build_actor_fn): Implement deallocation function

> 	selection per n4849, dcl.fct.def.coroutine bullet 12.

> 	(morph_fn_to_coro): Implement allocation function selection per

> 	n4849, dcl.fct.def.coroutine bullets 9 and 10.

> 

> gcc/testsuite/ChangeLog:

> 


This is ok.  I was having a dumb wrt to the type_size thing we talked about.


nathan


-- 
Nathan Sidwell

Patch

========

This updates the coroutine frame allocation and deallocation usage to
match n4849.

[dcl.fct.def.coroutine] /9, /10, /12.

9 An implementation may need to allocate additional storage for a coroutine.
  This storage is known as the coroutine state and is obtained by calling a
  non-array allocation function.  The allocation function’s name is looked up
  in the scope of the promise type.  If this lookup fails, the allocation
  function’s name is looked up in the global scope.  If the lookup finds an
  allocation function in the scope of the promise type, overload resolution
  is performed on a function call created by assembling an argument list.
  The first argument is the amount of space requested, and has type
  std::size_t.  The lvalues p1 . . . pn are the succeeding [user's function]
  arguments. If no viable function is found, overload resolution is performed
  again on a function call created by passing just the amount of space required
  as an argument of type std::size_t.

10 The unqualified-id get_return_object_on_allocation_failure is looked up in
  the scope of the promise type by class member access lookup. If any
  declarations are found, then the result of a call to an allocation function
  used to obtain storage for the coroutine state is assumed to return nullptr
  if it fails to obtain storage, and if a global allocation function is
  selected, the ::operator new(size_t, nothrow_t) form is used. The allocation
  function used in this case shall have a non-throwing noexcept-specification.
  If the allocation function returns nullptr, the coroutine returns control to
  the caller of the coroutine and the return value is obtained by a call to
  T::get_return_object_on_allocation_failure(), where T is the promise type.

12 The deallocation function’s name is looked up in the scope of the promise
  type. If this lookup fails, the deallocation function’s name is looked up in
  the global scope.  If deallocation function lookup finds both a usual
  deallocation function with only a pointer parameter and a usual deallocation
  function with both a pointer parameter and a size parameter, then the
  selected deallocation function shall be the one with two parameters.
  Otherwise, the selected deallocation function shall be the function with one
  parameter. If no usual deallocation function is found, the program is ill-
  formed.  The selected deallocation function shall be called with the address
  of the block of storage to be reclaimed as its first argument.  If a
  deallocation function with a parameter of type std::size_t is used, the size
  of the block is passed as the corresponding argument.

gcc/cp/ChangeLog:

2020-02-07  Iain Sandoe  <iain@sandoe.co.uk>

	* coroutines.cc (build_actor_fn): Implement deallocation function
	selection per n4849, dcl.fct.def.coroutine bullet 12.
	(morph_fn_to_coro): Implement allocation function selection per
	n4849, dcl.fct.def.coroutine bullets 9 and 10.

gcc/testsuite/ChangeLog:

2020-02-07  Iain Sandoe  <iain@sandoe.co.uk>

	* g++.dg/coroutines/coro1-allocators.h: New.
	* g++.dg/coroutines/coro-bad-alloc-00-bad-op-new.C: New test.
	* g++.dg/coroutines/coro-bad-alloc-01-bad-op-del.C: New test.
	* g++.dg/coroutines/coro-bad-alloc-02-no-op-new-nt.C: New test.
	* g++.dg/coroutines/torture/alloc-00-gro-on-alloc-fail.C: Use new
	coro1-allocators.h header.
	* g++.dg/coroutines/torture/alloc-01-overload-newdel.C: Likewise.
	* g++.dg/coroutines/torture/alloc-02-fail-new-grooaf-check.C: New.
	* g++.dg/coroutines/torture/alloc-03-overload-new-1.C: New test.
	* g++.dg/coroutines/torture/alloc-04-overload-del-use-two-args.C:New.
---
 gcc/cp/coroutines.cc                          | 248 ++++++++++++------
 .../coroutines/coro-bad-alloc-00-bad-op-new.C |  12 +
 .../coroutines/coro-bad-alloc-01-bad-op-del.C |  13 +
 .../coro-bad-alloc-02-no-op-new-nt.C          |  15 ++
 .../g++.dg/coroutines/coro1-allocators.h      | 184 +++++++++++++
 .../torture/alloc-00-gro-on-alloc-fail.C      |  98 +------
 .../torture/alloc-01-overload-newdel.C        |  81 +-----
 .../torture/alloc-02-fail-new-grooaf-check.C  |  41 +++
 .../torture/alloc-03-overload-new-1.C         |  55 ++++
 .../alloc-04-overload-del-use-two-args.C      |  60 +++++
 10 files changed, 567 insertions(+), 240 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/coroutines/coro-bad-alloc-00-bad-op-new.C
 create mode 100644 gcc/testsuite/g++.dg/coroutines/coro-bad-alloc-01-bad-op-del.C
 create mode 100644 gcc/testsuite/g++.dg/coroutines/coro-bad-alloc-02-no-op-new-nt.C
 create mode 100644 gcc/testsuite/g++.dg/coroutines/coro1-allocators.h
 create mode 100644 gcc/testsuite/g++.dg/coroutines/torture/alloc-02-fail-new-grooaf-check.C
 create mode 100644 gcc/testsuite/g++.dg/coroutines/torture/alloc-03-overload-new-1.C
 create mode 100644 gcc/testsuite/g++.dg/coroutines/torture/alloc-04-overload-del-use-two-args.C

diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc
index 0a8a531d6d7..524d4872804 100644
--- a/gcc/cp/coroutines.cc
+++ b/gcc/cp/coroutines.cc
@@ -1879,7 +1879,7 @@  build_actor_fn (location_t loc, tree coro_frame_type, tree actor, tree fnbody,
 		tree orig, hash_map<tree, param_info> *param_uses,
 		hash_map<tree, local_var_info> *local_var_uses,
 		vec<tree, va_gc> *param_dtor_list, tree initial_await,
-		tree final_await, unsigned body_count)
+		tree final_await, unsigned body_count, tree frame_size)
 {
   verify_stmt_tree (fnbody);
   /* Some things we inherit from the original function.  */
@@ -2198,33 +2198,64 @@  build_actor_fn (location_t loc, tree coro_frame_type, tree actor, tree fnbody,
 	}
     }
 
-  tree delname = ovl_op_identifier (false, DELETE_EXPR);
-  tree arg = build1 (CONVERT_EXPR, ptr_type_node, actor_fp);
-  vec<tree, va_gc> *arglist = make_tree_vector_single (arg);
+  /* n4849 [dcl.fct.def.coroutine] / 12
+     The deallocation function’s name is looked up in the scope of the promise
+     type.  If this lookup fails, the deallocation function’s name is looked up
+     in the global scope.  If deallocation function lookup finds both a usual
+     deallocation function with only a pointer parameter and a usual
+     deallocation function with both a pointer parameter and a size parameter,
+     then the selected deallocation function shall be the one with two
+     parameters.  Otherwise, the selected deallocation function shall be the
+     function with one parameter.  If no usual deallocation function is found
+     the program is ill-formed.  The selected deallocation function shall be
+     called with the address of the block of storage to be reclaimed as its
+     first argument.  If a deallocation function with a parameter of type
+     std::size_t is used, the size of the block is passed as the corresponding
+     argument.  */
 
-  /* The user can (optionally) provide a delete function in the promise
-      type, it's not a failure for it to be absent.  */
-  tree fns = lookup_promise_method (orig, delname, loc, false);
   tree del_coro_fr = NULL_TREE;
-  if (fns && fns != error_mark_node)
+  tree frame_arg = build1 (CONVERT_EXPR, ptr_type_node, actor_fp);
+
+  tree delname = ovl_op_identifier (false, DELETE_EXPR);
+  tree fns = lookup_promise_method (orig, delname, loc, /*musthave=*/false);
+  if (fns && BASELINK_P (fns))
     {
-      del_coro_fr = lookup_arg_dependent (delname, fns, arglist);
-      if (OVL_P (del_coro_fr))
-	del_coro_fr = OVL_FIRST (del_coro_fr);
-      else
-	del_coro_fr = BASELINK_FUNCTIONS (del_coro_fr);
+      /* Look for sized version first, since this takes precedence.  */
+      vec<tree, va_gc> *args = make_tree_vector ();
+      vec_safe_push (args, frame_arg);
+      vec_safe_push (args, frame_size);
+      tree dummy_promise = build_dummy_object (promise_type);
 
-      gcc_checking_assert (DECL_STATIC_FUNCTION_P (del_coro_fr));
-      TREE_USED (del_coro_fr) = 1;
-      del_coro_fr = build_call_expr_loc_vec (loc, del_coro_fr, arglist);
-    }
+      /* It's OK to fail for this one... */
+      del_coro_fr = build_new_method_call (dummy_promise, fns, &args,
+					   NULL_TREE, LOOKUP_NORMAL, NULL,
+					   tf_none);
 
-  /* If that fails, then fall back to the global delete operator.  */
-  if (del_coro_fr == NULL_TREE || del_coro_fr == error_mark_node)
+      if (!del_coro_fr || del_coro_fr == error_mark_node)
+	{
+	  release_tree_vector (args);
+	  args = make_tree_vector_single (frame_arg);
+	  del_coro_fr = build_new_method_call (dummy_promise, fns, &args,
+					       NULL_TREE, LOOKUP_NORMAL, NULL,
+					       tf_none);
+	}
+
+      /* But one of them must succeed, or the program is ill-formed.  */
+      if (!del_coro_fr || del_coro_fr == error_mark_node)
+	{
+	  error_at (loc, "%qE is provided by %qT but is not usable with"
+		  " the function signature %qD", delname, promise_type, orig);
+	  del_coro_fr = error_mark_node;
+	}
+    }
+  else
     {
-      fns =lookup_name_real (delname, 0, 1, /*block_p=*/true, 0, 0);
-      del_coro_fr = lookup_arg_dependent (del_coro_fr, fns, arglist);
-      del_coro_fr = build_new_function_call (del_coro_fr, &arglist, true);
+      del_coro_fr = build_op_delete_call (DELETE_EXPR, frame_arg, frame_size,
+					  /*global_p=*/true, /*placement=*/NULL,
+					  /*alloc_fn=*/NULL,
+					  tf_warning_or_error);
+      if (!del_coro_fr || del_coro_fr == error_mark_node)
+	del_coro_fr = error_mark_node;
     }
 
   del_coro_fr = coro_build_cvt_void_expr_stmt (del_coro_fr, loc);
@@ -3236,74 +3267,136 @@  morph_fn_to_coro (tree orig, tree *resumer, tree *destroyer)
   r = coro_build_cvt_void_expr_stmt (r, fn_start);
   add_stmt (r);
 
-  /* We are going to copy the behavior of clang w.r.t to failed allocation
-     of the coroutine frame.
-     1. If the promise has a 'get_return_object_on_allocation_failure()'
-	method, then we use a nothrow new and check the return value, calling
-	the method on failure to initialize an early return.
-     2. Otherwise, we call new and the ramp is expected to terminate with an
-	unhandled exception in the case of failure to allocate.
-
-     The get_return_object_on_allocation_failure() must be a static method.  */
-
-  tree grooaf_meth
-    = lookup_promise_method (orig, coro_gro_on_allocation_fail_identifier,
-			     fn_start, /*musthave=*/false);
-
   /* The CO_FRAME internal function is a mechanism to allow the middle end
      to adjust the allocation in response to optimisations.  We provide the
      current conservative estimate of the frame size (as per the current)
      computed layout.  */
+  tree frame_size = TYPE_SIZE_UNIT (coro_frame_type);
   tree resizeable
     = build_call_expr_internal_loc (fn_start, IFN_CO_FRAME, size_type_node, 2,
-				    TYPE_SIZE_UNIT (coro_frame_type), coro_fp);
+				    frame_size, coro_fp);
+
+  /* n4849 [dcl.fct.def.coroutine] / 10 (part1)
+    The unqualified-id get_return_object_on_allocation_failure is looked up
+    in the scope of the promise type by class member access lookup.  */
+
+  tree grooaf_meth
+    = lookup_promise_method (orig, coro_gro_on_allocation_fail_identifier,
+			     fn_start, /*musthave=*/false);
 
-  /* We need to adjust the operator new call as per the description above when
-     there is a return on allocation fail function provided in the promise.  */
   tree grooaf = NULL_TREE;
-  vec<tree, va_gc> *arglist;
-  vec_alloc (arglist, 2);
-  arglist->quick_push (resizeable);
+  tree dummy_promise = build_dummy_object (get_coroutine_promise_type (orig));
+
+  /* We don't require this, so lookup_promise_method can return NULL...  */
   if (grooaf_meth && BASELINK_P (grooaf_meth))
     {
-      tree fn = BASELINK_FUNCTIONS (grooaf_meth);
-      if (TREE_CODE (fn) == FUNCTION_DECL && DECL_STATIC_FUNCTION_P (fn))
-	{
-	  grooaf = build_call_expr_loc (fn_start, fn, 0);
-	  TREE_USED (fn) = 1;
-	}
-      tree nth_ns = lookup_qualified_name (std_node, get_identifier ("nothrow"),
-					   0, /*complain=*/true, false);
-      arglist->quick_push (nth_ns);
+      /* ... but, if the lookup succeeds, then the function must be
+	 usable.
+	 build_new_method_call () wants a valid pointer to (an empty)  args
+	 list in this case.  */
+      vec<tree, va_gc> *args = make_tree_vector ();
+      grooaf = build_new_method_call (dummy_promise, grooaf_meth, &args,
+				      NULL_TREE, LOOKUP_NORMAL, NULL,
+				      tf_warning_or_error);
+      release_tree_vector (args);
     }
 
-  /* Allocate the frame.  */
-
+  /* Allocate the frame, this has several possibilities:
+     n4849 [dcl.fct.def.coroutine] / 9 (part 1)
+     The allocation function’s name is looked up in the scope of the promise
+     type.  It's not a failure for it to be absent see part 4, below.  */
   tree nwname = ovl_op_identifier (false, NEW_EXPR);
-  /* The user can (optionally) provide an allocation function in the promise
-      type, it's not a failure for it to be absent.  */
   tree fns = lookup_promise_method (orig, nwname, fn_start,
 				    /*musthave=*/false);
   tree new_fn = NULL_TREE;
-  if (fns && fns != error_mark_node)
-    {
-      new_fn = lookup_arg_dependent (nwname, fns, arglist);
-      if (OVL_P (new_fn))
-	new_fn = OVL_FIRST (new_fn);
-      else
-	new_fn = BASELINK_FUNCTIONS (new_fn);
+  if (fns && BASELINK_P (fns))
+    {
+      /* n4849 [dcl.fct.def.coroutine] / 9 (part 2)
+	If the lookup finds an allocation function in the scope of the promise
+	type, overload resolution is performed on a function call created by
+	assembling an argument list.  The first argument is the amount of space
+	requested, and has type std::size_t.  The succeeding arguments are
+	those of the original function.  */
+      vec<tree, va_gc> *args = make_tree_vector ();
+      vec_safe_push (args, resizeable); /* Space needed.  */
+      for (tree arg = DECL_ARGUMENTS (orig); arg != NULL;
+	   arg = DECL_CHAIN (arg))
+	vec_safe_push (args, arg);
 
-	gcc_checking_assert (DECL_STATIC_FUNCTION_P (new_fn));
-	TREE_USED (new_fn) = 1;
-	new_fn = build_call_expr_loc_vec (fn_start, new_fn, arglist);
-    }
+      /* We might need to check that the provided function is nothrow.  */
+      tree func;
+      /* Failure is OK for the first attempt.  */
+      new_fn = build_new_method_call (dummy_promise, fns, &args, NULL,
+				      LOOKUP_NORMAL, &func, tf_none);
+      release_tree_vector (args);
+
+      if (!new_fn || new_fn == error_mark_node)
+	{
+	  /* n4849 [dcl.fct.def.coroutine] / 9 (part 3)
+	    If no viable function is found, overload resolution is performed
+	    again on a function call created by passing just the amount of
+	    space required as an argument of type std::size_t.  */
+	  args = make_tree_vector ();
+	  vec_safe_push (args, resizeable); /* Space needed.  */
+	  new_fn = build_new_method_call (dummy_promise, fns, &args,
+					  NULL_TREE, LOOKUP_NORMAL, &func,
+					  tf_none);
+	  release_tree_vector (args);
+	}
 
-  /* If that fails, then fall back to the global operator new.  */
-  if (new_fn == NULL_TREE || new_fn == error_mark_node)
+     /* However, if the initial lookup succeeded, then one of these two
+	options must be available.  */
+    if (!new_fn || new_fn == error_mark_node)
+      {
+	error_at (fn_start, "%qE is provided by %qT but is not usable with"
+		  " the function signature %qD", nwname, promise_type, orig);
+	new_fn = error_mark_node;
+      }
+    else if (grooaf && !TYPE_NOTHROW_P (TREE_TYPE (func)))
+      error_at (fn_start, "%qE is provided by %qT but %qE is not marked"
+		" %<throw()%> or %<noexcept%>", grooaf, promise_type, nwname);
+    }
+  else
     {
-      fns =lookup_name_real (nwname, 0, 1, /*block_p=*/true, 0, 0);
-      new_fn = lookup_arg_dependent (nwname, fns, arglist);
-      new_fn = build_new_function_call (new_fn, &arglist, /*complain=*/true);
+      /* n4849 [dcl.fct.def.coroutine] / 9 (part 4)
+	 If this lookup fails, the allocation function’s name is looked up in
+	 the global scope.  */
+
+      vec<tree, va_gc> *args;
+      /* build_operator_new_call () will insert size needed as element 0 of
+	 this, and we might need to append the std::nothrow constant.  */
+      vec_alloc (args, 2);
+
+      if (grooaf)
+	{
+	  /* n4849 [dcl.fct.def.coroutine] / 10 (part 2)
+	   If any declarations (of the get return on allocation fail) are
+	   found, then the result of a call to an allocation function used
+	   to obtain storage for the coroutine state is assumed to return
+	   nullptr if it fails to obtain storage and, if a global allocation
+	   function is selected, the ::operator new(size_t, nothrow_t) form
+	   is used.  The allocation function used in this case shall have a
+	   non-throwing noexcept-specification.  So we need std::nothrow.  */
+	  tree std_nt = lookup_qualified_name (std_node,
+					       get_identifier ("nothrow"),
+					       0, /*complain=*/true, false);
+	  vec_safe_push (args, std_nt);
+	}
+
+      /* If we get to this point, we must succeed in looking up the global
+	 operator new for the params provided.  Extract a simplified version
+	 of the machinery from build_operator_new_call.  This can update the
+	 frame size.  */
+      tree cookie = NULL;
+      new_fn = build_operator_new_call (nwname, &args, &frame_size, &cookie,
+					/*align_arg=*/NULL,
+					/*size_check=*/NULL, /*fn=*/NULL,
+					tf_warning_or_error);
+      resizeable = build_call_expr_internal_loc
+	(fn_start, IFN_CO_FRAME, size_type_node, 2, frame_size, coro_fp);
+      CALL_EXPR_ARG (new_fn, 0) = resizeable;
+
+      release_tree_vector (args);
     }
 
   tree allocated = build1 (CONVERT_EXPR, coro_frame_ptr, new_fn);
@@ -3317,7 +3410,12 @@  morph_fn_to_coro (tree orig, tree *resumer, tree *destroyer)
 
   if (grooaf)
     {
-      tree cfra_label
+      /* n4849 [dcl.fct.def.coroutine] / 10 (part 3)
+	 If the allocation function returns nullptr,the coroutine returns
+	 control to the caller of the coroutine and the return value is
+	 obtained by a call to T::get_return_object_on_allocation_failure(),
+	 where T is the promise type.  */
+       tree cfra_label
 	= create_named_label_with_ctx (fn_start, "coro.frame.active",
 				       current_scope ());
       tree early_ret_list = NULL;
@@ -3349,8 +3447,8 @@  morph_fn_to_coro (tree orig, tree *resumer, tree *destroyer)
   /* deref the frame pointer, to use in member access code.  */
   tree deref_fp = build_x_arrow (fn_start, coro_fp, tf_warning_or_error);
 
-  /* For now, we always assume that this needs destruction, there's no impl.
-     for frame allocation elision.  */
+  /* For now, once allocation has succeeded we always assume that this needs
+     destruction, there's no impl. for frame allocation elision.  */
   tree fnf_m
     = lookup_member (coro_frame_type, fnf_name, 1, 0, tf_warning_or_error);
   tree fnf_x = build_class_member_access_expr (deref_fp, fnf_m, NULL_TREE,
@@ -3724,7 +3822,7 @@  morph_fn_to_coro (tree orig, tree *resumer, tree *destroyer)
   /* Actor ...  */
   build_actor_fn (fn_start, coro_frame_type, actor, fnbody, orig, param_uses,
 		  &local_var_uses, param_dtor_list, initial_await, final_await,
-		  body_aw_points.count);
+		  body_aw_points.count, frame_size);
 
   /* Destroyer ... */
   build_destroy_fn (fn_start, coro_frame_type, destroy, actor);
diff --git a/gcc/testsuite/g++.dg/coroutines/coro-bad-alloc-00-bad-op-new.C b/gcc/testsuite/g++.dg/coroutines/coro-bad-alloc-00-bad-op-new.C
new file mode 100644
index 00000000000..1fdce1dad1d
--- /dev/null
+++ b/gcc/testsuite/g++.dg/coroutines/coro-bad-alloc-00-bad-op-new.C
@@ -0,0 +1,12 @@ 
+//  { dg-additional-options "-fsyntax-only -w" }
+
+// check error for a bad overload of operator new.
+
+#define BOGUS_OPNEW_CASE1
+#include "coro1-allocators.h"
+
+struct coro1
+f ()  /* { dg-error {'operator new' is provided by 'std::__n4835::coroutine_traits<coro1>::promise_type' \{aka 'coro1::promise_type'\} but is not usable with the function signature 'coro1 f\(\)'} } */
+{
+  co_return;
+}
diff --git a/gcc/testsuite/g++.dg/coroutines/coro-bad-alloc-01-bad-op-del.C b/gcc/testsuite/g++.dg/coroutines/coro-bad-alloc-01-bad-op-del.C
new file mode 100644
index 00000000000..be804796835
--- /dev/null
+++ b/gcc/testsuite/g++.dg/coroutines/coro-bad-alloc-01-bad-op-del.C
@@ -0,0 +1,13 @@ 
+//  { dg-additional-options "-fsyntax-only -w" }
+
+// check error for a bad overload of operator delete.
+
+#define BOGUS_OPDEL_CASE1
+#include "coro1-allocators.h"
+
+struct coro1
+f ()  /* { dg-error {'operator delete' is provided by 'std::__n4835::coroutine_traits<coro1>::promise_type' \{aka 'coro1::promise_type'\} but is not usable with the function signature 'coro1 f\(\)'} } */
+{
+  co_return;
+}
+
diff --git a/gcc/testsuite/g++.dg/coroutines/coro-bad-alloc-02-no-op-new-nt.C b/gcc/testsuite/g++.dg/coroutines/coro-bad-alloc-02-no-op-new-nt.C
new file mode 100644
index 00000000000..eae2dada911
--- /dev/null
+++ b/gcc/testsuite/g++.dg/coroutines/coro-bad-alloc-02-no-op-new-nt.C
@@ -0,0 +1,15 @@ 
+//  { dg-additional-options "-fsyntax-only -w" }
+
+// check error for missing new (size, nothrow).
+
+#define PROVIDE_NEW_SZT
+#define PROVIDE_DEL_VP
+#define PROVIDE_GROOAF
+
+#include "coro1-allocators.h"
+
+struct coro1
+f () /* { dg-error {'coro1::promise_type::get_return_object_on_allocation_failure\(\)\(\)' is provided by 'std::__n4835::coroutine_traits<coro1>::promise_type' \{aka 'coro1::promise_type'\} but 'operator new' is not marked 'throw\(\)' or 'noexcept'} } */
+{
+  co_return;
+}
diff --git a/gcc/testsuite/g++.dg/coroutines/coro1-allocators.h b/gcc/testsuite/g++.dg/coroutines/coro1-allocators.h
new file mode 100644
index 00000000000..3d869106006
--- /dev/null
+++ b/gcc/testsuite/g++.dg/coroutines/coro1-allocators.h
@@ -0,0 +1,184 @@ 
+/* Include <coroutine> or the equivalent.  */
+#include "coro.h"
+
+/* Allow for stand-alone testing with no headers installed.  */
+#if __has_include(<new>)
+#  include <new>
+#else
+
+/* Required when get_return_object_on_allocation_failure() is defined by
+  the promise.  We need a no-throw new, and new etc.  build the relevant
+  pieces here to avoid needing the headers in the test.  */
+
+namespace std {
+  struct nothrow_t {};
+  constexpr nothrow_t nothrow = {};
+  typedef __SIZE_TYPE__ size_t;
+} // end namespace std
+
+void* operator new(std::size_t, const std::nothrow_t&) noexcept;
+void  operator delete(void* __p, const std::nothrow_t&) noexcept;
+
+#endif
+
+/* Flags and counters so that we can test that the methods we expected
+   to be called, were called (and the correct number of times).  */
+
+#ifdef USE_FAILING_OP_NEW
+extern int used_failing_new;
+#endif
+
+#if defined (PROVIDE_NEW_SZT) || defined (PROVIDE_NEW_SZT_INT)
+extern int used_ovl_new;
+#endif
+
+#ifdef PROVIDE_NEW_SZT_NT
+extern void *malloc (size_t);
+extern int used_ovl_new_nt;
+#endif
+
+#ifdef PROVIDE_DEL_VP
+extern int used_ovl_del;
+#endif
+
+#ifdef PROVIDE_DEL_VP_SZT
+extern int used_ovl_del_2arg;
+#endif
+
+#ifdef PROVIDE_GROOAF
+extern int used_grooaf;
+#endif
+
+struct coro1 {
+  struct promise_type;
+  using handle_type = coro::coroutine_handle<coro1::promise_type>;
+  handle_type handle;
+  coro1 () noexcept : handle(0) {}
+  coro1 (handle_type _handle) noexcept
+    : handle(_handle)  {
+        PRINT("Created coro1 object from handle");
+  }
+  coro1 (const coro1 &) = delete; // no copying
+  coro1 (coro1 &&s) noexcept : handle(s.handle)  {
+	s.handle = nullptr;
+	PRINT("coro1 mv ctor ");
+  }
+  coro1 &operator = (coro1 &&s) noexcept {
+	handle = s.handle;
+	s.handle = nullptr;
+	PRINT("coro1 op=  ");
+	return *this;
+  }
+  ~coro1() noexcept {
+        PRINT("Destroyed coro1");
+        if ( handle )
+          handle.destroy();
+  }
+
+  struct suspend_never_prt {
+  bool await_ready() const noexcept { return true; }
+  void await_suspend(handle_type) const noexcept { PRINT ("susp-never-susp");}
+  void await_resume() const noexcept { PRINT ("susp-never-resume");}
+  ~suspend_never_prt() {};
+  };
+
+  struct  suspend_always_prt {
+  bool await_ready() const noexcept { return false; }
+  void await_suspend(handle_type) const noexcept { PRINT ("susp-always-susp");}
+  void await_resume() const noexcept { PRINT ("susp-always-resume");}
+  };
+
+  struct promise_type {
+  promise_type() {  PRINT ("Created Promise"); }
+  ~promise_type() { PRINT ("Destroyed Promise"); }
+
+  auto get_return_object () {
+    PRINT ("get_return_object: handle from promise");
+    return handle_type::from_promise (*this);
+  }
+  auto initial_suspend () {
+    PRINT ("get initial_suspend (always)");
+    return suspend_always_prt{};
+  }
+  auto final_suspend () {
+    PRINT ("get final_suspend (always)");
+    return suspend_always_prt{};
+  }
+  void return_void () {
+    PRINT ("return_void ()");
+  }
+  void unhandled_exception() { PRINT ("** unhandled exception"); }
+
+#ifdef USE_FAILING_OP_NEW
+  /* Provide an operator, that always fails.  */
+  void *operator new (std::size_t sz) noexcept {
+    PRINT ("promise_type: used failing op new");
+    used_failing_new++;
+    return nullptr;
+  }
+#endif
+
+#ifdef PROVIDE_NEW_SZT
+  void *operator new (std::size_t sz) {
+    PRINT ("promise_type: used overloaded operator new");
+    used_ovl_new++;
+    return ::operator new(sz);
+  }
+#endif
+
+#ifdef PROVIDE_NEW_SZT_NT
+  void *operator new (std::size_t sz, const std::nothrow_t&) noexcept {
+    PRINT ("promise_type: used overloaded operator new NT");
+    return malloc (sz);
+  }
+#endif
+
+#ifdef PROVIDE_NEW_SZT_INT
+  void *operator new (std::size_t sz, int x) {
+    PRINT ("promise_type: used overloaded operator new with int arg");
+    used_ovl_new += x;
+    return ::operator new(sz);
+  }
+#endif
+
+#ifdef PROVIDE_DEL_VP
+  void operator delete (void *p) {
+    PRINT ("promise_type: used overloaded operator delete 1 arg");
+    used_ovl_del++;
+    return ::operator delete(p);
+  }
+#endif
+
+#ifdef PROVIDE_DEL_VP_SZT
+  void operator delete (void *p, std::size_t sz) {
+    PRINT ("promise_type: used overloaded operator delete 2 args");
+    used_ovl_del_2arg++;
+    return ::operator delete(p);
+  }
+#endif
+
+#ifdef BOGUS_OPNEW_CASE1
+  /* Provide an operator, but it doesn't match on overload.  */
+  void *operator new (std::size_t sz, char *f) noexcept {
+    PRINT ("promise_type: used bogus op new");
+    return nullptr;
+  }
+#endif
+
+#ifdef BOGUS_OPDEL_CASE1
+  /* Provide an operator, but it doesn't match on overload.  */
+  void operator delete (void *p, char *f) {
+    PRINT ("promise_type: used bogus overloaded operator delete");
+  }
+#endif
+
+#ifdef PROVIDE_GROOAF
+  static coro1 get_return_object_on_allocation_failure () noexcept {
+    PRINT ("alloc fail return");
+    used_grooaf++;
+    return coro1 (nullptr);
+  }
+#endif
+
+  }; // promise
+}; // coro1
diff --git a/gcc/testsuite/g++.dg/coroutines/torture/alloc-00-gro-on-alloc-fail.C b/gcc/testsuite/g++.dg/coroutines/torture/alloc-00-gro-on-alloc-fail.C
index 8430d053c65..ca07a3a03d0 100644
--- a/gcc/testsuite/g++.dg/coroutines/torture/alloc-00-gro-on-alloc-fail.C
+++ b/gcc/testsuite/g++.dg/coroutines/torture/alloc-00-gro-on-alloc-fail.C
@@ -1,96 +1,13 @@ 
 //  { dg-do run }
 
-// check the code-gen for the failed alloc return.
+/* check the code-gen for the failed alloc return.
+   Here we use an allocator that doesn't fail so that the code
+   is generated, but the regular path is run.  */
 
-#include "../coro.h"
+#define PROVIDE_GROOAF
+#include "../coro1-allocators.h"
 
-#if __has_include(<new>)
-#  include <new>
-#else
-
-// Required when get_return_object_on_allocation_failure() is defined by
-// the promise.
-// we need a no-throw new, and new etc.  build the relevant pieces here to
-// avoid needing the headers in the test.
-
-namespace std {
-  struct nothrow_t {};
-  constexpr nothrow_t nothrow = {};
-  typedef __SIZE_TYPE__ size_t;
-} // end namespace std
-
-void* operator new(std::size_t, const std::nothrow_t&) noexcept;
-void  operator delete(void* __p, const std::nothrow_t&) noexcept;
-#endif
-
-struct coro1 {
-  struct promise_type;
-  using handle_type = coro::coroutine_handle<coro1::promise_type>;
-  handle_type handle;
-  coro1 () noexcept : handle(0) {}
-  coro1 (handle_type _handle) noexcept
-    : handle(_handle)  {
-        PRINT("Created coro1 object from handle");
-  }
-  coro1 (const coro1 &) = delete; // no copying
-  coro1 (coro1 &&s) noexcept : handle(s.handle)  {
-	s.handle = nullptr;
-	PRINT("coro1 mv ctor ");
-  }
-  coro1 &operator = (coro1 &&s) noexcept {
-	handle = s.handle;
-	s.handle = nullptr;
-	PRINT("coro1 op=  ");
-	return *this;
-  }
-  ~coro1() noexcept {
-        PRINT("Destroyed coro1");
-        if ( handle )
-          handle.destroy();
-  }
-
-  struct suspend_never_prt {
-  bool await_ready() const noexcept { return true; }
-  void await_suspend(handle_type) const noexcept { PRINT ("susp-never-susp");}
-  void await_resume() const noexcept { PRINT ("susp-never-resume");}
-  ~suspend_never_prt() {};
-  };
-
-  struct  suspend_always_prt {
-  bool await_ready() const noexcept { return false; }
-  void await_suspend(handle_type) const noexcept { PRINT ("susp-always-susp");}
-  void await_resume() const noexcept { PRINT ("susp-always-resume");}
-  };
-
-  struct promise_type {
-  promise_type() {  PRINT ("Created Promise"); }
-  ~promise_type() { PRINT ("Destroyed Promise"); }
-
-  auto get_return_object () {
-    PRINT ("get_return_object: handle from promise");
-    return handle_type::from_promise (*this);
-  }
-  auto initial_suspend () {
-    PRINT ("get initial_suspend (always)");
-    return suspend_always_prt{};
-  }
-  auto final_suspend () {
-    PRINT ("get final_suspend (always)");
-    return suspend_always_prt{};
-  }
-  void return_void () {
-    PRINT ("return_void ()");
-  }
-  void unhandled_exception() { PRINT ("** unhandled exception"); }
-  static coro1 get_return_object_on_allocation_failure () noexcept;
-  }; // promise
-}; // coro1
-
-coro1 coro1::promise_type::
-get_return_object_on_allocation_failure () noexcept {
-  PRINT ("alloc fail return");
-  return coro1 (nullptr);
-}
+int used_grooaf = 0;
 
 struct coro1
 f () noexcept
@@ -103,6 +20,9 @@  int main ()
 {
   PRINT ("main: create coro1");
   struct coro1 x = f ();
+  if (used_grooaf)
+    abort ();
+
   PRINT ("main: got coro1 - resuming");
   if (x.handle.done())
     abort();
diff --git a/gcc/testsuite/g++.dg/coroutines/torture/alloc-01-overload-newdel.C b/gcc/testsuite/g++.dg/coroutines/torture/alloc-01-overload-newdel.C
index f779f6e4863..98babcaf4f4 100644
--- a/gcc/testsuite/g++.dg/coroutines/torture/alloc-01-overload-newdel.C
+++ b/gcc/testsuite/g++.dg/coroutines/torture/alloc-01-overload-newdel.C
@@ -1,86 +1,15 @@ 
 //  { dg-do run }
 
-// check codegen for overloaded operator new/delete.
+// check codegen for overloaded simple operator new/delete.
 
-#include "../coro.h"
+#define PROVIDE_NEW_SZT
+#define PROVIDE_DEL_VP
+
+#include "../coro1-allocators.h"
 
 int used_ovl_new = 0;
 int used_ovl_del = 0;
 
-struct coro1 {
-  struct promise_type;
-  using handle_type = coro::coroutine_handle<coro1::promise_type>;
-  handle_type handle;
-  coro1 () noexcept : handle(0) {}
-  coro1 (handle_type _handle) noexcept
-    : handle(_handle)  {
-        PRINT("Created coro1 object from handle");
-  }
-  coro1 (const coro1 &) = delete; // no copying
-  coro1 (coro1 &&s) noexcept : handle(s.handle)  {
-	s.handle = nullptr;
-	PRINT("coro1 mv ctor ");
-  }
-  coro1 &operator = (coro1 &&s) noexcept {
-	handle = s.handle;
-	s.handle = nullptr;
-	PRINT("coro1 op=  ");
-	return *this;
-  }
-  ~coro1() noexcept {
-        PRINT("Destroyed coro1");
-        if ( handle )
-          handle.destroy();
-  }
-
-  struct suspend_never_prt {
-  bool await_ready() const noexcept { return true; }
-  void await_suspend(handle_type) const noexcept { PRINT ("susp-never-susp");}
-  void await_resume() const noexcept { PRINT ("susp-never-resume");}
-  ~suspend_never_prt() {};
-  };
-
-  struct  suspend_always_prt {
-  bool await_ready() const noexcept { return false; }
-  void await_suspend(handle_type) const noexcept { PRINT ("susp-always-susp");}
-  void await_resume() const noexcept { PRINT ("susp-always-resume");}
-  };
-
-  struct promise_type {
-  promise_type() {  PRINT ("Created Promise"); }
-  ~promise_type() { PRINT ("Destroyed Promise"); }
-
-  void *operator new (std::size_t sz) {
-    PRINT ("promise_type: used overloaded operator new");
-    used_ovl_new++;
-    return ::operator new(sz);
-  }
-
-  void operator delete (void *p)  {
-    PRINT ("promise_type: used overloaded operator delete");
-    used_ovl_del++;
-    return ::operator delete(p);
-  }
-
-  auto get_return_object () {
-    PRINT ("get_return_object: handle from promise");
-    return handle_type::from_promise (*this);
-  }
-  auto initial_suspend () {
-    PRINT ("get initial_suspend (always)");
-    return suspend_always_prt{};
-  }
-  auto final_suspend () {
-    PRINT ("get final_suspend (always)");
-    return suspend_always_prt{};
-  }
-  void return_void () {
-    PRINT ("return_void ()");
-  }
-  void unhandled_exception() { PRINT ("** unhandled exception"); }
-  }; // promise
-}; // coro1
-
 struct coro1
 f () noexcept
 {
diff --git a/gcc/testsuite/g++.dg/coroutines/torture/alloc-02-fail-new-grooaf-check.C b/gcc/testsuite/g++.dg/coroutines/torture/alloc-02-fail-new-grooaf-check.C
new file mode 100644
index 00000000000..7911cc8c43d
--- /dev/null
+++ b/gcc/testsuite/g++.dg/coroutines/torture/alloc-02-fail-new-grooaf-check.C
@@ -0,0 +1,41 @@ 
+//  { dg-do run }
+
+/* check the code-gen for the failed alloc return.
+   In this case, we use an operator new that always fails.
+   So the g-r-o-o-a-f should fire.  */
+
+#define PROVIDE_GROOAF
+#define USE_FAILING_OP_NEW
+#include "../coro1-allocators.h"
+
+int used_grooaf = 0;
+int used_failing_new = 0;
+
+struct coro1
+f () noexcept
+{
+  PRINT ("coro1: about to return");
+  co_return;
+}
+
+int main ()
+{
+  /* nest a scope so that we can check the counts.  */
+  {
+    PRINT ("main: create coro1");
+    struct coro1 x = f ();
+    /* we don't expect to be able to do anything.  */
+    if (used_failing_new != 1)
+      {
+	PRINT ("main: we should have used the failing op new");
+        abort ();
+      }
+    if (used_grooaf != 1)
+      {
+	PRINT ("main: we should have used the GROOAF");
+        abort ();
+      }
+  }
+  PRINT ("main: returning");
+  return 0;
+}
diff --git a/gcc/testsuite/g++.dg/coroutines/torture/alloc-03-overload-new-1.C b/gcc/testsuite/g++.dg/coroutines/torture/alloc-03-overload-new-1.C
new file mode 100644
index 00000000000..b1d6743fd91
--- /dev/null
+++ b/gcc/testsuite/g++.dg/coroutines/torture/alloc-03-overload-new-1.C
@@ -0,0 +1,55 @@ 
+//  { dg-do run }
+
+/* check codegen for overloaded simple operator new/delete.
+   here check that we prefer the overload that accounts the function
+   args.  */
+
+#define PROVIDE_NEW_SZT
+#define PROVIDE_NEW_SZT_INT
+#define PROVIDE_DEL_VP
+
+#include "../coro1-allocators.h"
+
+int used_ovl_new = 0;
+int used_ovl_del = 0;
+
+struct coro1
+f (int v) noexcept
+{
+  PRINT ("coro1: about to return");
+  co_return;
+}
+
+int main ()
+{
+  // Nest a scope so that we can inspect the flags after the DTORs run.
+  {
+  PRINT ("main: create coro1");
+  struct coro1 x = f (5);
+
+  if (used_ovl_new != 5)
+    {
+      PRINT ("main: apparently used the wrong op new...");
+      abort ();
+    }
+
+  PRINT ("main: got coro1 - resuming");
+  if (x.handle.done())
+    abort();
+  x.handle.resume();
+  PRINT ("main: after resume");
+  if (!x.handle.done())
+    {
+      PRINT ("main: apparently not done...");
+      abort ();
+    }
+  }
+
+  if (used_ovl_del != 1)
+    {
+      PRINT ("main: failed to call overloaded operator delete");
+      abort ();
+    }
+  PRINT ("main: returning");
+  return 0;
+}
diff --git a/gcc/testsuite/g++.dg/coroutines/torture/alloc-04-overload-del-use-two-args.C b/gcc/testsuite/g++.dg/coroutines/torture/alloc-04-overload-del-use-two-args.C
new file mode 100644
index 00000000000..2987c6ef116
--- /dev/null
+++ b/gcc/testsuite/g++.dg/coroutines/torture/alloc-04-overload-del-use-two-args.C
@@ -0,0 +1,60 @@ 
+//  { dg-do run }
+
+/* check that we use the deallocation function with two args when both
+   are available.  */
+
+#define PROVIDE_NEW_SZT
+#define PROVIDE_DEL_VP
+#define PROVIDE_DEL_VP_SZT
+
+#include "../coro1-allocators.h"
+
+int used_ovl_new = 0;
+int used_ovl_del = 0;
+int used_ovl_del_2arg = 0;
+
+struct coro1
+f (int v) noexcept
+{
+  PRINT ("coro1: about to return");
+  co_return;
+}
+
+int main ()
+{
+  // Nest a scope so that we can inspect the flags after the DTORs run.
+  {
+  PRINT ("main: create coro1");
+  struct coro1 x = f (5);
+
+  if (used_ovl_new != 1)
+    {
+      PRINT ("main: apparently used the wrong op new...");
+      abort ();
+    }
+
+  PRINT ("main: got coro1 - resuming");
+  if (x.handle.done())
+    abort();
+  x.handle.resume();
+  PRINT ("main: after resume");
+  if (!x.handle.done())
+    {
+      PRINT ("main: apparently not done...");
+      abort ();
+    }
+  }
+  if (used_ovl_del != 0)
+    {
+      PRINT ("main: wrong call to overloaded operator delete 1 arg");
+      abort ();
+    }
+
+  if (used_ovl_del_2arg != 1)
+    {
+      PRINT ("main: failed to call overloaded operator delete 2 args");
+      abort ();
+    }
+  PRINT ("main: returning");
+  return 0;
+}