[PATCH] lto: Stream edge goto_locus [PR94235]

[PATCH] lto: Stream edge goto_locus [PR94235]

[Jakub Jelinek]

Hi!

The following patch adds streaming of edge goto_locus (both LOCATION_LOCUS
and LOCATION_BLOCK from it), the PR shows a testcase (inappropriate for
gcc testsuite) where the lack of streaming of goto_locus results in worse
debug info.
Earlier version of the patch (without the output_function changes) failed
miserably, because on the order mismatch - input_function would
first input_cfg, then input_eh_regions and then input_bb (all of which now
have locations), while output_function used output_eh_regions, then output_bb
and then output_cfg.  *_cfg went to a separate stream...
Now, is there a reason why the order is different?
Bootstrap/regtest on x86_64-linux and i686-linux went fine after changing that,
including lto bootstrap on x86_64-linux.

If the intent is that the cfg could be read separately from the rest of
function or vice versa, alternatively we'd need to clear_line_info ();
before output_eh_regions and before/after output_cfg to make them
independent.

2020-09-05  Jakub Jelinek  <jakub@redhat.com>

	PR debug/94235
	* lto-streamer-out.c (output_cfg): Also stream goto_locus for edges.
	Use bp_pack_var_len_unsigned instead of streamer_write_uhwi to stream
	e->dest->index and e->flags.
	(output_function): Call output_cfg before output_ssa_name, rather than
	after streaming all bbs.
	* lto-streamer-in.c (input_cfg): Stream in goto_locus for edges.
	Use bp_unpack_var_len_unsigned instead of streamer_read_uhwi to stream
	in dest_index and edge_flags.

--- gcc/lto-streamer-out.c.jj	2020-09-04 11:53:22.673647471 +0200
+++ gcc/lto-streamer-out.c	2020-09-04 22:42:14.595657649 +0200
@@ -2100,9 +2100,11 @@ output_cfg (struct output_block *ob, str
       streamer_write_uhwi (ob, EDGE_COUNT (bb->succs));
       FOR_EACH_EDGE (e, ei, bb->succs)
 	{
-	  streamer_write_uhwi (ob, e->dest->index);
+	  bitpack_d bp = bitpack_create (ob->main_stream);
+	  bp_pack_var_len_unsigned (&bp, e->dest->index);
+	  bp_pack_var_len_unsigned (&bp, e->flags);
+	  stream_output_location_and_block (ob, &bp, e->goto_locus);
 	  e->probability.stream_out (ob);
-	  streamer_write_uhwi (ob, e->flags);
 	}
     }
 
@@ -2418,6 +2420,8 @@ output_function (struct cgraph_node *nod
       streamer_write_uhwi (ob, 1);
       output_struct_function_base (ob, fn);
 
+      output_cfg (ob, fn);
+
       /* Output all the SSA names used in the function.  */
       output_ssa_names (ob, fn);
 
@@ -2430,8 +2434,6 @@ output_function (struct cgraph_node *nod
 
       /* The terminator for this function.  */
       streamer_write_record_start (ob, LTO_null);
-
-      output_cfg (ob, fn);
    }
   else
     streamer_write_uhwi (ob, 0);
--- gcc/lto-streamer-in.c.jj	2020-09-04 11:55:08.069114502 +0200
+++ gcc/lto-streamer-in.c	2020-09-04 13:26:04.439987053 +0200
@@ -780,23 +780,19 @@ input_cfg (class lto_input_block *ib, cl
       /* Connect up the CFG.  */
       for (i = 0; i < edge_count; i++)
 	{
-	  unsigned int dest_index;
-	  unsigned int edge_flags;
-	  basic_block dest;
-	  profile_probability probability;
-	  edge e;
-
-	  dest_index = streamer_read_uhwi (ib);
-	  probability = profile_probability::stream_in (ib);
-	  edge_flags = streamer_read_uhwi (ib);
-
-	  dest = BASIC_BLOCK_FOR_FN (fn, dest_index);
+	  bitpack_d bp = streamer_read_bitpack (ib);
+	  unsigned int dest_index = bp_unpack_var_len_unsigned (&bp);
+	  unsigned int edge_flags = bp_unpack_var_len_unsigned (&bp);
+	  basic_block dest = BASIC_BLOCK_FOR_FN (fn, dest_index);
 
 	  if (dest == NULL)
 	    dest = make_new_block (fn, dest_index);
 
-	  e = make_edge (bb, dest, edge_flags);
-	  e->probability = probability;
+	  edge e = make_edge (bb, dest, edge_flags);
+	  data_in->location_cache.input_location_and_block (&e->goto_locus,
+							    &bp, ib, data_in);
+	  e->probability = profile_probability::stream_in (ib);
+
 	}
 
       index = streamer_read_hwi (ib);

	Jakub

Re: [PATCH] lto: Stream edge goto_locus [PR94235]

[Richard Biener]

On Sat, 5 Sep 2020, Jakub Jelinek wrote:

> Hi!
> 
> The following patch adds streaming of edge goto_locus (both LOCATION_LOCUS
> and LOCATION_BLOCK from it), the PR shows a testcase (inappropriate for
> gcc testsuite) where the lack of streaming of goto_locus results in worse
> debug info.
> Earlier version of the patch (without the output_function changes) failed
> miserably, because on the order mismatch - input_function would
> first input_cfg, then input_eh_regions and then input_bb (all of which now
> have locations), while output_function used output_eh_regions, then output_bb
> and then output_cfg.  *_cfg went to a separate stream...
> Now, is there a reason why the order is different?

It's probably simply an oversight.

> Bootstrap/regtest on x86_64-linux and i686-linux went fine after changing that,
> including lto bootstrap on x86_64-linux.

OK.

Thanks,
Richard.

> If the intent is that the cfg could be read separately from the rest of
> function or vice versa, alternatively we'd need to clear_line_info ();
> before output_eh_regions and before/after output_cfg to make them
> independent.
> 
> 2020-09-05  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR debug/94235
> 	* lto-streamer-out.c (output_cfg): Also stream goto_locus for edges.
> 	Use bp_pack_var_len_unsigned instead of streamer_write_uhwi to stream
> 	e->dest->index and e->flags.
> 	(output_function): Call output_cfg before output_ssa_name, rather than
> 	after streaming all bbs.
> 	* lto-streamer-in.c (input_cfg): Stream in goto_locus for edges.
> 	Use bp_unpack_var_len_unsigned instead of streamer_read_uhwi to stream
> 	in dest_index and edge_flags.
> 
> --- gcc/lto-streamer-out.c.jj	2020-09-04 11:53:22.673647471 +0200
> +++ gcc/lto-streamer-out.c	2020-09-04 22:42:14.595657649 +0200
> @@ -2100,9 +2100,11 @@ output_cfg (struct output_block *ob, str
>        streamer_write_uhwi (ob, EDGE_COUNT (bb->succs));
>        FOR_EACH_EDGE (e, ei, bb->succs)
>  	{
> -	  streamer_write_uhwi (ob, e->dest->index);
> +	  bitpack_d bp = bitpack_create (ob->main_stream);
> +	  bp_pack_var_len_unsigned (&bp, e->dest->index);
> +	  bp_pack_var_len_unsigned (&bp, e->flags);
> +	  stream_output_location_and_block (ob, &bp, e->goto_locus);
>  	  e->probability.stream_out (ob);
> -	  streamer_write_uhwi (ob, e->flags);
>  	}
>      }
>  
> @@ -2418,6 +2420,8 @@ output_function (struct cgraph_node *nod
>        streamer_write_uhwi (ob, 1);
>        output_struct_function_base (ob, fn);
>  
> +      output_cfg (ob, fn);
> +
>        /* Output all the SSA names used in the function.  */
>        output_ssa_names (ob, fn);
>  
> @@ -2430,8 +2434,6 @@ output_function (struct cgraph_node *nod
>  
>        /* The terminator for this function.  */
>        streamer_write_record_start (ob, LTO_null);
> -
> -      output_cfg (ob, fn);
>     }
>    else
>      streamer_write_uhwi (ob, 0);
> --- gcc/lto-streamer-in.c.jj	2020-09-04 11:55:08.069114502 +0200
> +++ gcc/lto-streamer-in.c	2020-09-04 13:26:04.439987053 +0200
> @@ -780,23 +780,19 @@ input_cfg (class lto_input_block *ib, cl
>        /* Connect up the CFG.  */
>        for (i = 0; i < edge_count; i++)
>  	{
> -	  unsigned int dest_index;
> -	  unsigned int edge_flags;
> -	  basic_block dest;
> -	  profile_probability probability;
> -	  edge e;
> -
> -	  dest_index = streamer_read_uhwi (ib);
> -	  probability = profile_probability::stream_in (ib);
> -	  edge_flags = streamer_read_uhwi (ib);
> -
> -	  dest = BASIC_BLOCK_FOR_FN (fn, dest_index);
> +	  bitpack_d bp = streamer_read_bitpack (ib);
> +	  unsigned int dest_index = bp_unpack_var_len_unsigned (&bp);
> +	  unsigned int edge_flags = bp_unpack_var_len_unsigned (&bp);
> +	  basic_block dest = BASIC_BLOCK_FOR_FN (fn, dest_index);
>  
>  	  if (dest == NULL)
>  	    dest = make_new_block (fn, dest_index);
>  
> -	  e = make_edge (bb, dest, edge_flags);
> -	  e->probability = probability;
> +	  edge e = make_edge (bb, dest, edge_flags);
> +	  data_in->location_cache.input_location_and_block (&e->goto_locus,
> +							    &bp, ib, data_in);
> +	  e->probability = profile_probability::stream_in (ib);
> +
>  	}
>  
>        index = streamer_read_hwi (ib);
> 
> 	Jakub
> 
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
Germany; GF: Felix Imendörffer; HRB 36809 (AG Nuernberg)