diff mbox

LTO streaming of TARGET_OPTIMIZE_NODE

Message ID 20150109114503.GW1405@tucnak.redhat.com
State New
Headers show

Commit Message

Jakub Jelinek Jan. 9, 2015, 11:45 a.m. UTC 
On Fri, Jan 09, 2015 at 12:07:26PM +0100, Thomas Schwinge wrote:
> On Thu, 8 Jan 2015 15:11:49 +0100, Jakub Jelinek <jakub@redhat.com> wrote:
> > On Thu, Nov 20, 2014 at 01:27:08PM +0100, Bernd Schmidt wrote:
> > > On 11/13/2014 05:06 AM, Jan Hubicka wrote:
> > > >this patch adds infrastructure for proper streaming and merging of
> > > >TREE_TARGET_OPTION.
> > > 
> > > This breaks the offloading path via LTO since it introduces an
> > > incompatibility in LTO format between host and offload machine.
> > > 
> > > A very quick patch to fix it is below - the OpenACC testcase I was using
> > > seems to be working again with this. Thoughts, suggestions?
> > 
> > I actually think
> 
> Thanks for picking up this issue!

Richard said on IRC he doesn't like the string comparisons, so here is
untested modification of the patch.  If it looks good, I'll test it today:

2015-01-09  Bernd Schmidt  <bernds@codesourcery.com>
	    Jakub Jelinek  <jakub@redhat.com>

	PR middle-end/64412
	* lto-streamer.h (lto_stream_offload_p): New declaration.
	* lto-streamer.c (lto_stream_offload_p): New variable.
	* cgraphunit.c (ipa_passes): Set lto_stream_offload_p
	at the same time as section_name_prefix.
	* lto-streamer-out.c (hash_tree): Don't hash TREE_TARGET_OPTION
	if lto_stream_offload_p.
	* tree-streamer-out.c (streamer_pack_tree_bitfields): Don't
	stream TREE_TARGET_OPTION if lto_stream_offload_p.
	(write_ts_function_decl_tree_pointers): Don't
	stream DECL_FUNCTION_SPECIFIC_TARGET if lto_stream_offload_p.
	* tree-streamer-in.c (unpack_value_fields): Don't stream
	TREE_TARGET_OPTION in if ACCEL_COMPILER.
	(lto_input_ts_function_decl_tree_pointers): Don't stream
	DECL_FUNCTION_SPECIFIC_TARGET in if ACCEL_COMPILER.
	* lto-opts.c (lto_write_options): Use lto_stream_offload_p
	instead of section_name_prefix string comparisons.
lto/
	* lto.c (read_cgraph_and_symbols): Set lto_stream_offload_p
	if ACCEL_COMPILER.


	Jakub

Comments

Richard Biener Jan. 9, 2015, 11:43 a.m. UTC | #1 
On Fri, 9 Jan 2015, Jakub Jelinek wrote:

> On Fri, Jan 09, 2015 at 12:07:26PM +0100, Thomas Schwinge wrote:
> > On Thu, 8 Jan 2015 15:11:49 +0100, Jakub Jelinek <jakub@redhat.com> wrote:
> > > On Thu, Nov 20, 2014 at 01:27:08PM +0100, Bernd Schmidt wrote:
> > > > On 11/13/2014 05:06 AM, Jan Hubicka wrote:
> > > > >this patch adds infrastructure for proper streaming and merging of
> > > > >TREE_TARGET_OPTION.
> > > > 
> > > > This breaks the offloading path via LTO since it introduces an
> > > > incompatibility in LTO format between host and offload machine.
> > > > 
> > > > A very quick patch to fix it is below - the OpenACC testcase I was using
> > > > seems to be working again with this. Thoughts, suggestions?
> > > 
> > > I actually think
> > 
> > Thanks for picking up this issue!
> 
> Richard said on IRC he doesn't like the string comparisons, so here is
> untested modification of the patch.  If it looks good, I'll test it today:

Looks good to me.

Richard.

> 2015-01-09  Bernd Schmidt  <bernds@codesourcery.com>
> 	    Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR middle-end/64412
> 	* lto-streamer.h (lto_stream_offload_p): New declaration.
> 	* lto-streamer.c (lto_stream_offload_p): New variable.
> 	* cgraphunit.c (ipa_passes): Set lto_stream_offload_p
> 	at the same time as section_name_prefix.
> 	* lto-streamer-out.c (hash_tree): Don't hash TREE_TARGET_OPTION
> 	if lto_stream_offload_p.
> 	* tree-streamer-out.c (streamer_pack_tree_bitfields): Don't
> 	stream TREE_TARGET_OPTION if lto_stream_offload_p.
> 	(write_ts_function_decl_tree_pointers): Don't
> 	stream DECL_FUNCTION_SPECIFIC_TARGET if lto_stream_offload_p.
> 	* tree-streamer-in.c (unpack_value_fields): Don't stream
> 	TREE_TARGET_OPTION in if ACCEL_COMPILER.
> 	(lto_input_ts_function_decl_tree_pointers): Don't stream
> 	DECL_FUNCTION_SPECIFIC_TARGET in if ACCEL_COMPILER.
> 	* lto-opts.c (lto_write_options): Use lto_stream_offload_p
> 	instead of section_name_prefix string comparisons.
> lto/
> 	* lto.c (read_cgraph_and_symbols): Set lto_stream_offload_p
> 	if ACCEL_COMPILER.
> 
> --- gcc/lto-streamer.h.jj	2015-01-05 13:07:13.000000000 +0100
> +++ gcc/lto-streamer.h	2015-01-09 12:18:26.199842482 +0100
> @@ -744,6 +744,10 @@ extern void lto_append_block (struct lto
>  
>  
>  /* In lto-streamer.c.  */
> +
> +/* Set when streaming LTO for offloading compiler.  */
> +extern bool lto_stream_offload_p;
> +
>  extern const char *lto_tag_name (enum LTO_tags);
>  extern bitmap lto_bitmap_alloc (void);
>  extern void lto_bitmap_free (bitmap);
> --- gcc/lto-streamer.c.jj	2015-01-05 13:07:13.000000000 +0100
> +++ gcc/lto-streamer.c	2015-01-09 12:16:04.909269917 +0100
> @@ -61,6 +61,8 @@ static bitmap_obstack lto_obstack;
>  static bool lto_obstack_initialized;
>  
>  const char *section_name_prefix = LTO_SECTION_NAME_PREFIX;
> +/* Set when streaming LTO for offloading compiler.  */
> +bool lto_stream_offload_p;
>  
>  /* Return a string representing LTO tag TAG.  */
>  
> --- gcc/cgraphunit.c.jj	2015-01-09 12:01:33.000000000 +0100
> +++ gcc/cgraphunit.c	2015-01-09 12:22:27.742692667 +0100
> @@ -2108,11 +2108,14 @@ ipa_passes (void)
>        if (g->have_offload)
>  	{
>  	  section_name_prefix = OFFLOAD_SECTION_NAME_PREFIX;
> +	  lto_stream_offload_p = true;
>  	  ipa_write_summaries (true);
> +	  lto_stream_offload_p = false;
>  	}
>        if (flag_lto)
>  	{
>  	  section_name_prefix = LTO_SECTION_NAME_PREFIX;
> +	  lto_stream_offload_p = false;
>  	  ipa_write_summaries (false);
>  	}
>      }
> --- gcc/lto-streamer-out.c.jj	2015-01-08 18:10:23.633598629 +0100
> +++ gcc/lto-streamer-out.c	2015-01-09 12:14:41.017711211 +0100
> @@ -944,7 +944,9 @@ hash_tree (struct streamer_tree_cache_d
>      hstate.add (TRANSLATION_UNIT_LANGUAGE (t),
>  			strlen (TRANSLATION_UNIT_LANGUAGE (t)));
>  
> -  if (CODE_CONTAINS_STRUCT (code, TS_TARGET_OPTION))
> +  if (CODE_CONTAINS_STRUCT (code, TS_TARGET_OPTION)
> +      /* We don't stream these when passing things to a different target.  */
> +      && !lto_stream_offload_p)
>      hstate.add_wide_int (cl_target_option_hash (TREE_TARGET_OPTION (t)));
>  
>    if (CODE_CONTAINS_STRUCT (code, TS_OPTIMIZATION))
> --- gcc/tree-streamer-out.c.jj	2015-01-08 18:10:23.631598663 +0100
> +++ gcc/tree-streamer-out.c	2015-01-09 12:14:41.018711194 +0100
> @@ -472,7 +472,9 @@ streamer_pack_tree_bitfields (struct out
>    if (CODE_CONTAINS_STRUCT (code, TS_CONSTRUCTOR))
>      bp_pack_var_len_unsigned (bp, CONSTRUCTOR_NELTS (expr));
>  
> -  if (CODE_CONTAINS_STRUCT (code, TS_TARGET_OPTION))
> +  if (CODE_CONTAINS_STRUCT (code, TS_TARGET_OPTION)
> +      /* Don't stream these when passing things to a different target.  */
> +      && !lto_stream_offload_p)
>      cl_target_option_stream_out (ob, bp, TREE_TARGET_OPTION (expr));
>  
>    if (code == OMP_CLAUSE)
> @@ -687,7 +689,9 @@ write_ts_function_decl_tree_pointers (st
>    stream_write_tree (ob, DECL_VINDEX (expr), ref_p);
>    /* DECL_STRUCT_FUNCTION is handled by lto_output_function.  */
>    stream_write_tree (ob, DECL_FUNCTION_PERSONALITY (expr), ref_p);
> -  stream_write_tree (ob, DECL_FUNCTION_SPECIFIC_TARGET (expr), ref_p);
> +  /* Don't stream these when passing things to a different target.  */
> +  if (!lto_stream_offload_p)
> +    stream_write_tree (ob, DECL_FUNCTION_SPECIFIC_TARGET (expr), ref_p);
>    stream_write_tree (ob, DECL_FUNCTION_SPECIFIC_OPTIMIZATION (expr), ref_p);
>  }
>  
> --- gcc/tree-streamer-in.c.jj	2015-01-08 18:10:23.561599843 +0100
> +++ gcc/tree-streamer-in.c	2015-01-09 12:14:41.017711211 +0100
> @@ -520,8 +520,10 @@ unpack_value_fields (struct data_in *dat
>  	vec_safe_grow (CONSTRUCTOR_ELTS (expr), length);
>      }
>  
> +#ifndef ACCEL_COMPILER
>    if (CODE_CONTAINS_STRUCT (code, TS_TARGET_OPTION))
>      cl_target_option_stream_in (data_in, bp, TREE_TARGET_OPTION (expr));
> +#endif
>  
>    if (code == OMP_CLAUSE)
>      unpack_ts_omp_clause_value_fields (data_in, bp, expr);
> @@ -785,7 +787,9 @@ lto_input_ts_function_decl_tree_pointers
>    DECL_VINDEX (expr) = stream_read_tree (ib, data_in);
>    /* DECL_STRUCT_FUNCTION is loaded on demand by cgraph_get_body.  */
>    DECL_FUNCTION_PERSONALITY (expr) = stream_read_tree (ib, data_in);
> +#ifndef ACCEL_COMPILER
>    DECL_FUNCTION_SPECIFIC_TARGET (expr) = stream_read_tree (ib, data_in);
> +#endif
>    DECL_FUNCTION_SPECIFIC_OPTIMIZATION (expr) = stream_read_tree (ib, data_in);
>  
>    /* If the file contains a function with an EH personality set,
> --- gcc/lto-opts.c.jj	2015-01-05 13:07:12.000000000 +0100
> +++ gcc/lto-opts.c	2015-01-09 12:21:04.203127914 +0100
> @@ -160,7 +160,7 @@ lto_write_options (void)
>  			       "-fno-strict-overflow");
>  
>    /* Append options from target hook and store them to offload_lto section.  */
> -  if (strcmp (section_name_prefix, OFFLOAD_SECTION_NAME_PREFIX) == 0)
> +  if (lto_stream_offload_p)
>      {
>        char *offload_opts = targetm.offload_options ();
>        char *offload_ptr = offload_opts;
> @@ -201,7 +201,7 @@ lto_write_options (void)
>  
>        /* Do not store target-specific options in offload_lto section.  */
>        if ((cl_options[option->opt_index].flags & CL_TARGET)
> -	 && strcmp (section_name_prefix, OFFLOAD_SECTION_NAME_PREFIX) == 0)
> +	  && lto_stream_offload_p)
>         continue;
>  
>        /* Drop options created from the gcc driver that will be rejected
> @@ -214,8 +214,7 @@ lto_write_options (void)
>  	 We do not need those.  The only exception is -foffload option, if we
>  	 write it in offload_lto section.  Also drop all diagnostic options.  */
>        if ((cl_options[option->opt_index].flags & (CL_DRIVER|CL_WARNING))
> -	  && (strcmp (section_name_prefix, OFFLOAD_SECTION_NAME_PREFIX) != 0
> -	      || option->opt_index != OPT_foffload_))
> +	  && (!lto_stream_offload_p || option->opt_index != OPT_foffload_))
>  	continue;
>  
>        for (j = 0; j < option->canonical_option_num_elements; ++j)
> --- gcc/lto/lto.c.jj	2015-01-05 13:07:19.000000000 +0100
> +++ gcc/lto/lto.c	2015-01-09 12:21:50.479332868 +0100
> @@ -2900,7 +2900,8 @@ read_cgraph_and_symbols (unsigned nfiles
>    timevar_push (TV_IPA_LTO_DECL_IN);
>  
>  #ifdef ACCEL_COMPILER
> -    section_name_prefix = OFFLOAD_SECTION_NAME_PREFIX;
> +  section_name_prefix = OFFLOAD_SECTION_NAME_PREFIX;
> +  lto_stream_offload_p = true;
>  #endif
>  
>    real_file_decl_data
> 
> 	Jakub
> 
>
Jan Hubicka Jan. 11, 2015, 3:56 p.m. UTC | #2 
> On Fri, Jan 09, 2015 at 12:07:26PM +0100, Thomas Schwinge wrote:
> > On Thu, 8 Jan 2015 15:11:49 +0100, Jakub Jelinek <jakub@redhat.com> wrote:
> > > On Thu, Nov 20, 2014 at 01:27:08PM +0100, Bernd Schmidt wrote:
> > > > On 11/13/2014 05:06 AM, Jan Hubicka wrote:
> > > > >this patch adds infrastructure for proper streaming and merging of
> > > > >TREE_TARGET_OPTION.
> > > > 
> > > > This breaks the offloading path via LTO since it introduces an
> > > > incompatibility in LTO format between host and offload machine.
> > > > 
> > > > A very quick patch to fix it is below - the OpenACC testcase I was using
> > > > seems to be working again with this. Thoughts, suggestions?
> > > 
> > > I actually think
> > 
> > Thanks for picking up this issue!
> 
> Richard said on IRC he doesn't like the string comparisons, so here is
> untested modification of the patch.  If it looks good, I'll test it today:
> 
> 2015-01-09  Bernd Schmidt  <bernds@codesourcery.com>
> 	    Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR middle-end/64412
> 	* lto-streamer.h (lto_stream_offload_p): New declaration.
> 	* lto-streamer.c (lto_stream_offload_p): New variable.
> 	* cgraphunit.c (ipa_passes): Set lto_stream_offload_p
> 	at the same time as section_name_prefix.
> 	* lto-streamer-out.c (hash_tree): Don't hash TREE_TARGET_OPTION
> 	if lto_stream_offload_p.
> 	* tree-streamer-out.c (streamer_pack_tree_bitfields): Don't
> 	stream TREE_TARGET_OPTION if lto_stream_offload_p.
> 	(write_ts_function_decl_tree_pointers): Don't
> 	stream DECL_FUNCTION_SPECIFIC_TARGET if lto_stream_offload_p.
> 	* tree-streamer-in.c (unpack_value_fields): Don't stream
> 	TREE_TARGET_OPTION in if ACCEL_COMPILER.
> 	(lto_input_ts_function_decl_tree_pointers): Don't stream
> 	DECL_FUNCTION_SPECIFIC_TARGET in if ACCEL_COMPILER.
> 	* lto-opts.c (lto_write_options): Use lto_stream_offload_p
> 	instead of section_name_prefix string comparisons.
> lto/
> 	* lto.c (read_cgraph_and_symbols): Set lto_stream_offload_p
> 	if ACCEL_COMPILER.

This looks fine to me. Thanks for looking into this issue (that was mine,
but I was quite busy last month)

Honza
diff mbox

Patch

--- gcc/lto-streamer.h.jj	2015-01-05 13:07:13.000000000 +0100
+++ gcc/lto-streamer.h	2015-01-09 12:18:26.199842482 +0100
@@ -744,6 +744,10 @@  extern void lto_append_block (struct lto
 
 
 /* In lto-streamer.c.  */
+
+/* Set when streaming LTO for offloading compiler.  */
+extern bool lto_stream_offload_p;
+
 extern const char *lto_tag_name (enum LTO_tags);
 extern bitmap lto_bitmap_alloc (void);
 extern void lto_bitmap_free (bitmap);
--- gcc/lto-streamer.c.jj	2015-01-05 13:07:13.000000000 +0100
+++ gcc/lto-streamer.c	2015-01-09 12:16:04.909269917 +0100
@@ -61,6 +61,8 @@  static bitmap_obstack lto_obstack;
 static bool lto_obstack_initialized;
 
 const char *section_name_prefix = LTO_SECTION_NAME_PREFIX;
+/* Set when streaming LTO for offloading compiler.  */
+bool lto_stream_offload_p;
 
 /* Return a string representing LTO tag TAG.  */
 
--- gcc/cgraphunit.c.jj	2015-01-09 12:01:33.000000000 +0100
+++ gcc/cgraphunit.c	2015-01-09 12:22:27.742692667 +0100
@@ -2108,11 +2108,14 @@  ipa_passes (void)
       if (g->have_offload)
 	{
 	  section_name_prefix = OFFLOAD_SECTION_NAME_PREFIX;
+	  lto_stream_offload_p = true;
 	  ipa_write_summaries (true);
+	  lto_stream_offload_p = false;
 	}
       if (flag_lto)
 	{
 	  section_name_prefix = LTO_SECTION_NAME_PREFIX;
+	  lto_stream_offload_p = false;
 	  ipa_write_summaries (false);
 	}
     }
--- gcc/lto-streamer-out.c.jj	2015-01-08 18:10:23.633598629 +0100
+++ gcc/lto-streamer-out.c	2015-01-09 12:14:41.017711211 +0100
@@ -944,7 +944,9 @@  hash_tree (struct streamer_tree_cache_d
     hstate.add (TRANSLATION_UNIT_LANGUAGE (t),
 			strlen (TRANSLATION_UNIT_LANGUAGE (t)));
 
-  if (CODE_CONTAINS_STRUCT (code, TS_TARGET_OPTION))
+  if (CODE_CONTAINS_STRUCT (code, TS_TARGET_OPTION)
+      /* We don't stream these when passing things to a different target.  */
+      && !lto_stream_offload_p)
     hstate.add_wide_int (cl_target_option_hash (TREE_TARGET_OPTION (t)));
 
   if (CODE_CONTAINS_STRUCT (code, TS_OPTIMIZATION))
--- gcc/tree-streamer-out.c.jj	2015-01-08 18:10:23.631598663 +0100
+++ gcc/tree-streamer-out.c	2015-01-09 12:14:41.018711194 +0100
@@ -472,7 +472,9 @@  streamer_pack_tree_bitfields (struct out
   if (CODE_CONTAINS_STRUCT (code, TS_CONSTRUCTOR))
     bp_pack_var_len_unsigned (bp, CONSTRUCTOR_NELTS (expr));
 
-  if (CODE_CONTAINS_STRUCT (code, TS_TARGET_OPTION))
+  if (CODE_CONTAINS_STRUCT (code, TS_TARGET_OPTION)
+      /* Don't stream these when passing things to a different target.  */
+      && !lto_stream_offload_p)
     cl_target_option_stream_out (ob, bp, TREE_TARGET_OPTION (expr));
 
   if (code == OMP_CLAUSE)
@@ -687,7 +689,9 @@  write_ts_function_decl_tree_pointers (st
   stream_write_tree (ob, DECL_VINDEX (expr), ref_p);
   /* DECL_STRUCT_FUNCTION is handled by lto_output_function.  */
   stream_write_tree (ob, DECL_FUNCTION_PERSONALITY (expr), ref_p);
-  stream_write_tree (ob, DECL_FUNCTION_SPECIFIC_TARGET (expr), ref_p);
+  /* Don't stream these when passing things to a different target.  */
+  if (!lto_stream_offload_p)
+    stream_write_tree (ob, DECL_FUNCTION_SPECIFIC_TARGET (expr), ref_p);
   stream_write_tree (ob, DECL_FUNCTION_SPECIFIC_OPTIMIZATION (expr), ref_p);
 }
 
--- gcc/tree-streamer-in.c.jj	2015-01-08 18:10:23.561599843 +0100
+++ gcc/tree-streamer-in.c	2015-01-09 12:14:41.017711211 +0100
@@ -520,8 +520,10 @@  unpack_value_fields (struct data_in *dat
 	vec_safe_grow (CONSTRUCTOR_ELTS (expr), length);
     }
 
+#ifndef ACCEL_COMPILER
   if (CODE_CONTAINS_STRUCT (code, TS_TARGET_OPTION))
     cl_target_option_stream_in (data_in, bp, TREE_TARGET_OPTION (expr));
+#endif
 
   if (code == OMP_CLAUSE)
     unpack_ts_omp_clause_value_fields (data_in, bp, expr);
@@ -785,7 +787,9 @@  lto_input_ts_function_decl_tree_pointers
   DECL_VINDEX (expr) = stream_read_tree (ib, data_in);
   /* DECL_STRUCT_FUNCTION is loaded on demand by cgraph_get_body.  */
   DECL_FUNCTION_PERSONALITY (expr) = stream_read_tree (ib, data_in);
+#ifndef ACCEL_COMPILER
   DECL_FUNCTION_SPECIFIC_TARGET (expr) = stream_read_tree (ib, data_in);
+#endif
   DECL_FUNCTION_SPECIFIC_OPTIMIZATION (expr) = stream_read_tree (ib, data_in);
 
   /* If the file contains a function with an EH personality set,
--- gcc/lto-opts.c.jj	2015-01-05 13:07:12.000000000 +0100
+++ gcc/lto-opts.c	2015-01-09 12:21:04.203127914 +0100
@@ -160,7 +160,7 @@  lto_write_options (void)
 			       "-fno-strict-overflow");
 
   /* Append options from target hook and store them to offload_lto section.  */
-  if (strcmp (section_name_prefix, OFFLOAD_SECTION_NAME_PREFIX) == 0)
+  if (lto_stream_offload_p)
     {
       char *offload_opts = targetm.offload_options ();
       char *offload_ptr = offload_opts;
@@ -201,7 +201,7 @@  lto_write_options (void)
 
       /* Do not store target-specific options in offload_lto section.  */
       if ((cl_options[option->opt_index].flags & CL_TARGET)
-	 && strcmp (section_name_prefix, OFFLOAD_SECTION_NAME_PREFIX) == 0)
+	  && lto_stream_offload_p)
        continue;
 
       /* Drop options created from the gcc driver that will be rejected
@@ -214,8 +214,7 @@  lto_write_options (void)
 	 We do not need those.  The only exception is -foffload option, if we
 	 write it in offload_lto section.  Also drop all diagnostic options.  */
       if ((cl_options[option->opt_index].flags & (CL_DRIVER|CL_WARNING))
-	  && (strcmp (section_name_prefix, OFFLOAD_SECTION_NAME_PREFIX) != 0
-	      || option->opt_index != OPT_foffload_))
+	  && (!lto_stream_offload_p || option->opt_index != OPT_foffload_))
 	continue;
 
       for (j = 0; j < option->canonical_option_num_elements; ++j)
--- gcc/lto/lto.c.jj	2015-01-05 13:07:19.000000000 +0100
+++ gcc/lto/lto.c	2015-01-09 12:21:50.479332868 +0100
@@ -2900,7 +2900,8 @@  read_cgraph_and_symbols (unsigned nfiles
   timevar_push (TV_IPA_LTO_DECL_IN);
 
 #ifdef ACCEL_COMPILER
-    section_name_prefix = OFFLOAD_SECTION_NAME_PREFIX;
+  section_name_prefix = OFFLOAD_SECTION_NAME_PREFIX;
+  lto_stream_offload_p = true;
 #endif
 
   real_file_decl_data