public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Iain Sandoe <iain@sandoe.co.uk>
To: GCC Patches <gcc-patches@gcc.gnu.org>
Subject: [PATCH 1/8] coroutines : Use DECL_VALUE_EXPR instead of rewriting  vars.
Date: Wed, 1 Sep 2021 11:52:30 +0100	[thread overview]
Message-ID: <3E415FEA-27D9-4C04-BFEE-002F67EED9F6@sandoe.co.uk> (raw)
In-Reply-To: <4C7B8763-1292-450A-96FE-A56B66926540@sandoe.co.uk>

Hi,

Variables that need to persist over suspension expressions
must be preserved by being copied into the coroutine frame.

The initial implementations do this manually in the transform
code.  However, that has various disadvantages - including
that the debug connections are lost between the original var
and the frame copy.

The revised implementation makes use of DECL_VALUE_EXPRs to
contain the frame offset expressions, so that the original
var names are preserved in the code.

This process is also applied to the function parms which are
always copied to the frame.  In this case the decls need to be
copied since they are used in two different contexts during
the re-write (in the building of the ramp function, and in
the actor function itself).

This will assist in improvement of debugging (PR 99215).

Signed-off-by: Iain Sandoe <iain@sandoe.co.uk>

gcc/cp/ChangeLog:

	* coroutines.cc (transform_local_var_uses): Record
	frame offset expressions as DECL_VALUE_EXPRs instead of
	rewriting them.
---
 gcc/cp/coroutines.cc | 105 +++----------------------------------------
 1 file changed, 5 insertions(+), 100 deletions(-)

diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc
index ceb3d3be75e..2d68098f242 100644
--- a/gcc/cp/coroutines.cc
+++ b/gcc/cp/coroutines.cc
@@ -1974,8 +1974,7 @@ transform_local_var_uses (tree *stmt, int *do_subtree, void *d)
   local_vars_transform *lvd = (local_vars_transform *) d;
 
   /* For each var in this bind expr (that has a frame id, which means it was
-     accessed), build a frame reference for each and then walk the bind expr
-     statements, substituting the frame ref for the original var.  */
+     accessed), build a frame reference and add it as the DECL_VALUE_EXPR.  */
 
   if (TREE_CODE (*stmt) == BIND_EXPR)
     {
@@ -1991,13 +1990,9 @@ transform_local_var_uses (tree *stmt, int *do_subtree, void *d)
 	  /* Re-write the variable's context to be in the actor func.  */
 	  DECL_CONTEXT (lvar) = lvd->context;
 
-	/* For capture proxies, this could include the decl value expr.  */
-	if (local_var.is_lambda_capture || local_var.has_value_expr_p)
-	  {
-	    tree ve = DECL_VALUE_EXPR (lvar);
-	    cp_walk_tree (&ve, transform_local_var_uses, d, NULL);
+	  /* For capture proxies, this could include the decl value expr.  */
+	  if (local_var.is_lambda_capture || local_var.has_value_expr_p)
 	    continue; /* No frame entry for this.  */
-	  }
 
 	  /* TODO: implement selective generation of fields when vars are
 	     known not-used.  */
@@ -2011,103 +2006,13 @@ transform_local_var_uses (tree *stmt, int *do_subtree, void *d)
 	  tree fld_idx = build3_loc (lvd->loc, COMPONENT_REF, TREE_TYPE (lvar),
 				     lvd->actor_frame, fld_ref, NULL_TREE);
 	  local_var.field_idx = fld_idx;
-	}
-      /* FIXME: we should be able to do this in the loop above, but (at least
-	 for range for) there are cases where the DECL_INITIAL contains
-	 forward references.
-	 So, now we've built the revised var in the frame, substitute uses of
-	 it in initializers and the bind expr body.  */
-      for (lvar = BIND_EXPR_VARS (*stmt); lvar != NULL;
-	   lvar = DECL_CHAIN (lvar))
-	{
-	  /* we need to walk some of the decl trees, which might contain
-	     references to vars replaced at a higher level.  */
-	  cp_walk_tree (&DECL_INITIAL (lvar), transform_local_var_uses, d,
-			NULL);
-	  cp_walk_tree (&DECL_SIZE (lvar), transform_local_var_uses, d, NULL);
-	  cp_walk_tree (&DECL_SIZE_UNIT (lvar), transform_local_var_uses, d,
-			NULL);
+	  SET_DECL_VALUE_EXPR (lvar, fld_idx);
+	  DECL_HAS_VALUE_EXPR_P (lvar) = true;
 	}
       cp_walk_tree (&BIND_EXPR_BODY (*stmt), transform_local_var_uses, d, NULL);
-
-      /* Now we have processed and removed references to the original vars,
-	 we can drop those from the bind - leaving capture proxies alone.  */
-      for (tree *pvar = &BIND_EXPR_VARS (*stmt); *pvar != NULL;)
-	{
-	  bool existed;
-	  local_var_info &local_var
-	    = lvd->local_var_uses->get_or_insert (*pvar, &existed);
-	  gcc_checking_assert (existed);
-
-	  /* Leave lambda closure captures alone, we replace the *this
-	     pointer with the frame version and let the normal process
-	     deal with the rest.
-	     Likewise, variables with their value found elsewhere.
-	     Skip past unused ones too.  */
-	  if (local_var.is_lambda_capture
-	     || local_var.has_value_expr_p
-	     || local_var.field_id == NULL_TREE)
-	    {
-	      pvar = &DECL_CHAIN (*pvar);
-	      continue;
-	    }
-
-	  /* Discard this one, we replaced it.  */
-	  *pvar = DECL_CHAIN (*pvar);
-	}
-
       *do_subtree = 0; /* We've done the body already.  */
       return NULL_TREE;
     }
-
-  tree var_decl = *stmt;
-  /* Look inside cleanups, we don't want to wrap a statement list in a
-     cleanup.  */
-  bool needs_cleanup = true;
-  if (TREE_CODE (var_decl) == CLEANUP_POINT_EXPR)
-    var_decl = TREE_OPERAND (var_decl, 0);
-  else
-    needs_cleanup = false;
-
-  /* Look inside the decl_expr for the actual var.  */
-  bool decl_expr_p = TREE_CODE (var_decl) == DECL_EXPR;
-  if (decl_expr_p && TREE_CODE (DECL_EXPR_DECL (var_decl)) == VAR_DECL)
-    var_decl = DECL_EXPR_DECL (var_decl);
-  else if (TREE_CODE (var_decl) != VAR_DECL)
-    return NULL_TREE;
-
-  /* VAR_DECLs that are not recorded can belong to the proxies we've placed
-     for the promise and coroutine handle(s), to global vars or to compiler
-     temporaries.  Skip past these, we will handle them later.  */
-  local_var_info *local_var_i = lvd->local_var_uses->get (var_decl);
-
-  if (local_var_i == NULL)
-    return NULL_TREE;
-
-  if (local_var_i->is_lambda_capture
-      || local_var_i->is_static
-      || local_var_i->has_value_expr_p)
-    return NULL_TREE;
-
-  /* This is our revised 'local' i.e. a frame slot.  */
-  tree revised = local_var_i->field_idx;
-  gcc_checking_assert (DECL_CONTEXT (var_decl) == lvd->context);
-
-  if (decl_expr_p && DECL_INITIAL (var_decl))
-    {
-      location_t loc = DECL_SOURCE_LOCATION (var_decl);
-      tree r
-	= cp_build_modify_expr (loc, revised, INIT_EXPR,
-				DECL_INITIAL (var_decl), tf_warning_or_error);
-      if (needs_cleanup)
-	r = coro_build_cvt_void_expr_stmt (r, EXPR_LOCATION (*stmt));
-      *stmt = r;
-    }
-  else
-    *stmt = revised;
-
-  if (decl_expr_p)
-    *do_subtree = 0; /* We've accounted for the nested use.  */
   return NULL_TREE;
 }
 
-- 


  reply	other threads:[~2021-09-01 10:52 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-09-01 10:51 [PATCH 0/8] coroutines: Use DECL_VALUE_EXPRs to assist in debug [PR99215] Iain Sandoe
2021-09-01 10:52 ` Iain Sandoe [this message]
2021-09-01 10:53   ` [PATCH 2/8] coroutines: Add a helper for creating local vars Iain Sandoe
2021-09-01 10:53     ` PATCH 3/8] coroutines: Support for debugging implementation state Iain Sandoe
2021-09-01 10:54       ` [PATCH 4/8] coroutines: Make some of the artificial names more debugger-friendly Iain Sandoe
2021-09-01 10:54         ` [PATCH 5/8] coroutines: Define and populate accessors for debug state Iain Sandoe
2021-09-01 10:55           ` [PATCH 6/8] coroutines: Convert implementation variables to debug-friendly form Iain Sandoe
2021-09-01 10:56             ` [PATCH 7/8] coroutines: Make proxy vars for the function arg copies Iain Sandoe
2021-09-01 10:56               ` [PATCH 8/8] coroutines: Make the continue handle visible to debug Iain Sandoe
2021-09-03 14:25                 ` Jason Merrill
2021-09-03 14:07               ` [PATCH 7/8] coroutines: Make proxy vars for the function arg copies Jason Merrill
2021-09-03 14:23                 ` Iain Sandoe
2021-09-05 19:50                   ` [PATCH 7/8 v2] " Iain Sandoe
2021-09-07 17:26                     ` Jason Merrill
2021-09-03 13:52             ` [PATCH 6/8] coroutines: Convert implementation variables to debug-friendly form Jason Merrill
2021-09-03 13:56               ` Iain Sandoe
2021-09-03 14:12                 ` Jason Merrill
2021-09-03 14:21                   ` Iain Sandoe
2021-09-05 19:47                     ` Iain Sandoe
2021-09-07 17:23                       ` Jason Merrill
2021-09-03 13:39           ` [PATCH 5/8] coroutines: Define and populate accessors for debug state Jason Merrill
2021-09-03 13:41             ` Iain Sandoe
2021-09-03 13:42             ` Iain Sandoe
2021-09-03 13:44               ` Jason Merrill
2021-09-03 13:35         ` [PATCH 4/8] coroutines: Make some of the artificial names more debugger-friendly Jason Merrill
2021-09-03 13:31       ` PATCH 3/8] coroutines: Support for debugging implementation state Jason Merrill
2021-09-02 21:52     ` [PATCH 2/8] coroutines: Add a helper for creating local vars Jason Merrill
2021-09-02 21:51   ` [PATCH 1/8] coroutines : Use DECL_VALUE_EXPR instead of rewriting vars Jason Merrill

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=3E415FEA-27D9-4C04-BFEE-002F67EED9F6@sandoe.co.uk \
    --to=iain@sandoe.co.uk \
    --cc=gcc-patches@gcc.gnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).