Remap goto_locus on edges during inlining

Message ID 1959532.xT2u3vGzl1@polaris
State New
Headers show
Series
  • Remap goto_locus on edges during inlining
Related show

Commit Message

Eric Botcazou June 26, 2018, 8:54 a.m.
Hi,

this makes sure the goto_locus present (or not) on edges is properly remapped 
during inlining.

Tested on x86-64/Linux, OK for the mainline?


2018-06-26  Eric Botcazou  <ebotcazou@adacore.com>

	* tree-inline.c (remap_location): New function extracted from...
	(copy_edges_for_bb): Add ID parameter.  Remap goto_locus.
	(copy_phis_for_bb): ...here.  Call remap_location.
	(copy_cfg_body): Adjust call to copy_edges_for_bb.

-- 
Eric Botcazou

Comments

Jeff Law June 26, 2018, 8:11 p.m. | #1
On 06/26/2018 02:54 AM, Eric Botcazou wrote:
> Hi,

> 

> this makes sure the goto_locus present (or not) on edges is properly remapped 

> during inlining.

> 

> Tested on x86-64/Linux, OK for the mainline?

> 

> 

> 2018-06-26  Eric Botcazou  <ebotcazou@adacore.com>

> 

> 	* tree-inline.c (remap_location): New function extracted from...

> 	(copy_edges_for_bb): Add ID parameter.  Remap goto_locus.

> 	(copy_phis_for_bb): ...here.  Call remap_location.

> 	(copy_cfg_body): Adjust call to copy_edges_for_bb.

OK.
jeff
Richard Biener June 28, 2018, 9:49 a.m. | #2
On Tue, Jun 26, 2018 at 10:12 PM Jeff Law <law@redhat.com> wrote:
>

> On 06/26/2018 02:54 AM, Eric Botcazou wrote:

> > Hi,

> >

> > this makes sure the goto_locus present (or not) on edges is properly remapped

> > during inlining.

> >

> > Tested on x86-64/Linux, OK for the mainline?

> >

> >

> > 2018-06-26  Eric Botcazou  <ebotcazou@adacore.com>

> >

> >       * tree-inline.c (remap_location): New function extracted from...

> >       (copy_edges_for_bb): Add ID parameter.  Remap goto_locus.

> >       (copy_phis_for_bb): ...here.  Call remap_location.

> >       (copy_cfg_body): Adjust call to copy_edges_for_bb.

> OK.


Related we're also missing to verify_location () on those in
verify_gimple_in_cfg.  Having stale references to GCed BLOCKs
via locations can be difficult to track down...

So can you add the verification bits as well?

Thanks,
Richard.

> jeff
Eric Botcazou June 28, 2018, 10:45 a.m. | #3
> Related we're also missing to verify_location () on those in

> verify_gimple_in_cfg.  Having stale references to GCed BLOCKs

> via locations can be difficult to track down...

> 

> So can you add the verification bits as well?


Like this?


	* tree-cfg.c (verify_gimple_in_cfg): Call verify_location on the
	goto_locus of each outgoing edge of each basic block.

-- 
Eric Botcazou
Index: tree-cfg.c
===================================================================
--- tree-cfg.c	(revision 262207)
+++ tree-cfg.c	(working copy)
@@ -5286,6 +5286,8 @@ verify_gimple_in_cfg (struct function *f
   FOR_EACH_BB_FN (bb, fn)
     {
       gimple_stmt_iterator gsi;
+      edge_iterator ei;
+      edge e;
 
       for (gphi_iterator gpi = gsi_start_phis (bb);
 	   !gsi_end_p (gpi);
@@ -5407,6 +5409,10 @@ verify_gimple_in_cfg (struct function *f
 	    debug_gimple_stmt (stmt);
 	  err |= err2;
 	}
+
+      FOR_EACH_EDGE (e, ei, bb->succs)
+	if (e->goto_locus != UNKNOWN_LOCATION)
+	  err |= verify_location (&blocks, e->goto_locus);
     }
 
   hash_map<gimple *, int> *eh_table = get_eh_throw_stmt_table (cfun);
Richard Biener June 28, 2018, 11:27 a.m. | #4
On Thu, Jun 28, 2018 at 12:46 PM Eric Botcazou <ebotcazou@adacore.com> wrote:
>

> > Related we're also missing to verify_location () on those in

> > verify_gimple_in_cfg.  Having stale references to GCed BLOCKs

> > via locations can be difficult to track down...

> >

> > So can you add the verification bits as well?

>

> Like this?


Yes, OK if it (hopefully!) passes testing.

Richard.

>

>         * tree-cfg.c (verify_gimple_in_cfg): Call verify_location on the

>         goto_locus of each outgoing edge of each basic block.

>

> --

> Eric Botcazou
Eric Botcazou June 28, 2018, 2:49 p.m. | #5
> Yes, OK if it (hopefully!) passes testing.


Yes, it passed bootstrap/regtest on x86-64/Linux so was applied.

-- 
Eric Botcazou

Patch

Index: tree-inline.c
===================================================================
--- tree-inline.c	(revision 262129)
+++ tree-inline.c	(working copy)
@@ -718,6 +718,7 @@  remap_block (tree *block, copy_body_data
 }
 
 /* Copy the whole block tree and root it in id->block.  */
+
 static tree
 remap_blocks (tree block, copy_body_data *id)
 {
@@ -738,6 +739,7 @@  remap_blocks (tree block, copy_body_data
 }
 
 /* Remap the block tree rooted at BLOCK to nothing.  */
+
 static void
 remap_blocks_to_null (tree block, copy_body_data *id)
 {
@@ -747,6 +749,27 @@  remap_blocks_to_null (tree block, copy_b
     remap_blocks_to_null (t, id);
 }
 
+/* Remap the location info pointed to by LOCUS.  */
+
+static location_t
+remap_location (location_t locus, copy_body_data *id)
+{
+  if (LOCATION_BLOCK (locus))
+    {
+      tree *n = id->decl_map->get (LOCATION_BLOCK (locus));
+      gcc_assert (n);
+      if (*n)
+	return set_block (locus, *n);
+    }
+
+  locus = LOCATION_LOCUS (locus);
+
+  if (locus != UNKNOWN_LOCATION && id->block)
+    return set_block (locus, id->block);
+
+  return locus;
+}
+
 static void
 copy_statement_list (tree *tp)
 {
@@ -2145,7 +2168,8 @@  update_ssa_across_abnormal_edges (basic_
 
 static bool
 copy_edges_for_bb (basic_block bb, profile_count num, profile_count den,
-		   basic_block ret_bb, basic_block abnormal_goto_dest)
+		   basic_block ret_bb, basic_block abnormal_goto_dest,
+		   copy_body_data *id)
 {
   basic_block new_bb = (basic_block) bb->aux;
   edge_iterator ei;
@@ -2160,6 +2184,7 @@  copy_edges_for_bb (basic_block bb, profi
       {
 	edge new_edge;
 	int flags = old_edge->flags;
+	location_t locus = old_edge->goto_locus;
 
 	/* Return edges do get a FALLTHRU flag when they get inlined.  */
 	if (old_edge->dest->index == EXIT_BLOCK
@@ -2167,8 +2192,10 @@  copy_edges_for_bb (basic_block bb, profi
 	    && old_edge->dest->aux != EXIT_BLOCK_PTR_FOR_FN (cfun))
 	  flags |= EDGE_FALLTHRU;
 
-	new_edge = make_edge (new_bb, (basic_block) old_edge->dest->aux, flags);
+	new_edge
+	  = make_edge (new_bb, (basic_block) old_edge->dest->aux, flags);
 	new_edge->probability = old_edge->probability;
+	new_edge->goto_locus = remap_location (locus, id);
       }
 
   if (bb->index == ENTRY_BLOCK || bb->index == EXIT_BLOCK)
@@ -2365,16 +2392,7 @@  copy_phis_for_bb (basic_block bb, copy_b
 		      inserted = true;
 		    }
 		  locus = gimple_phi_arg_location_from_edge (phi, old_edge);
-		  if (LOCATION_BLOCK (locus))
-		    {
-		      tree *n;
-		      n = id->decl_map->get (LOCATION_BLOCK (locus));
-		      gcc_assert (n);
-		      locus = set_block (locus, *n);
-		    }
-		  else
-		    locus = LOCATION_LOCUS (locus);
-
+		  locus = remap_location (locus, id);
 		  add_phi_arg (new_phi, new_arg, new_edge, locus);
 		}
 	    }
@@ -2705,7 +2723,7 @@  copy_cfg_body (copy_body_data * id,
     if (!id->blocks_to_copy
 	|| (bb->index > 0 && bitmap_bit_p (id->blocks_to_copy, bb->index)))
       need_debug_cleanup |= copy_edges_for_bb (bb, num, den, exit_block_map,
-					       abnormal_goto_dest);
+					       abnormal_goto_dest, id);
 
   if (new_entry)
     {