diff mbox

LTO streaming of TARGET_OPTIMIZE_NODE

Message ID 20141113040652.GB1984@kam.mff.cuni.cz
State New
Headers show

Commit Message

Jan Hubicka Nov. 13, 2014, 4:06 a.m. UTC
Hi,
this patch adds infrastructure for proper streaming and merging of
TREE_TARGET_OPTION.  The catch is that TREE_TARGET_OPTION is autogenerated
structure.  For x86_64 it looks as follows:

/* Structure to save/restore selected target specific options.  */
struct GTY(()) cl_target_option
{
  HOST_WIDE_INT x_ix86_isa_flags_explicit;
  HOST_WIDE_INT x_ix86_target_flags_explicit;
  const char *x_ix86_arch_string;
  const char *x_ix86_recip_name;
  const char *x_ix86_tune_ctrl_string;
  const char *x_ix86_tune_memcpy_strategy;
  const char *x_ix86_tune_memset_strategy;
  const char *x_ix86_tune_string;
  HOST_WIDE_INT x_ix86_isa_flags;
  enum fpmath_unit x_ix86_fpmath;
  enum asm_dialect x_ix86_asm_dialect;
  ...
  enum stack_protector_guard x_ix86_stack_protector_guard;
  enum stringop_alg x_ix86_stringop_alg;
  enum tls_dialect x_ix86_tls_dialect;
  int x_ix86_branch_cost;
  int x_ix86_dump_tunes;
  ....
  int x_recip_mask;
  int x_target_flags;
  unsigned char arch;
  unsigned char arch_specified;
  unsigned char branch_cost;
  unsigned char schedule;
  unsigned char tune;
  unsigned char tune_defaulted;
};

The problem is that the strings prevents us from streaming the structure as a binary
blob like we do for optimization nodes (and we should not do that to make LTO streaming
stable across attribute tweaks in minor releases).

This patch extends the AWK generators to produce four functions:

/* Compare two target options  */
bool
cl_target_option_eq (struct cl_target_option const *ptr1,
                     struct cl_target_option const *ptr2)
{
  if (ptr1->x_ix86_arch_string != ptr2->x_ix86_arch_string
      && (!ptr1->x_ix86_arch_string || !ptr2->x_ix86_arch_string
          || strcmp (ptr1->x_ix86_arch_string, ptr2->x_ix86_arch_string)))
    return false;
....
    if (ptr1->x_ix86_isa_flags != ptr2->x_ix86_isa_flags)
    return false;
  if (ptr1->x_ix86_fpmath != ptr2->x_ix86_fpmath)
    return false;
  return true;
}

/* Hash target options  */
hashval_t
cl_target_option_hash (struct cl_target_option const *ptr)
{ 
  inchash::hash hstate;
  if (ptr->x_ix86_arch_string)
    hstate.add (ptr->x_ix86_arch_string, strlen (ptr->x_ix86_arch_string));
  else
    hstate.add_int (0);
...
  hstate.add_wide_int (ptr->x_ix86_fpmath);
  return hstate.end ();
}

/* Stream out target options  */
void
cl_target_option_stream_out (struct output_block *ob, struct cl_target_option *ptr)
{ 
  streamer_write_string (ob, ob->main_stream, ptr->x_ix86_arch_string, true);
...
  streamer_write_uhwi (ob, ptr->x_ix86_isa_flags);
  streamer_write_uhwi (ob, ptr->x_ix86_fpmath);
}

/* Stream in target options  */
void
cl_target_option_stream_in (struct lto_input_block *ib, struct data_in *data_in, struct cl_target_option *ptr)
{ 
  ptr->x_ix86_arch_string = strdup (streamer_read_string (data_in, ib));
...
  ptr->x_ix86_fpmath = (enum fpmath_unit ) streamer_read_uhwi (ib);
}

These allows us to properly stream, compare and hash the values.

There is some implementation lazyness - for example I stream all scalars as
unsigned HOST_WIDE_INT that seems to be the largest type used for variables
across the targets.

If other types are used, the type matching (I stole from other part of the awk
scripts) will need to be extended; for now it knows that const char * is a
string and needs special care but nothing else.

I have tested this with firefox and additional change to tree.c to always
initialize TARGET_OPTION_NODE for defined functions (so target units are
peorperly presered across units). This seems to work as intended - i.e. the
functions gets link-time optimized according to -march settings passed at
compile time.

I noticed that memory use is now 3.8GB instead of previous 2.8GB - this may be
tree merging problem caused by mismatching target settings but most probably it
is an unrelated stuff. I will look into it tomorrow.  I also plan to test this
on PPC (that currently fails to bootstrap).

Incrementally I would like to handle optimizations nodes same way and change
streaming format to be a set of assignments field_name=value.
This way we could behave sanely when some field is introduced or removed by
either defaulting it or ignoring it.

Bootstrapped/regtested x86_64-linux, seems sane?

	* optc-save-gen.awk: Output cl_target_option_eq,
	cl_target_option_hash, cl_target_option_stream_out,
	cl_target_option_stream_in functions.
	* opth-gen.awk: Output prototypes for
	cl_target_option_eq and cl_target_option_hash.
	* lto-streamer.h (cl_target_option_stream_out,
	cl_target_option_stream_in): Declare.
	* tree.c (cl_option_hash_hash): Use cl_target_option_hash.
	(cl_option_hash_eq): Use cl_target_option_eq.
	* tree-streamer-in.c (unpack_value_fields): Stream in
	TREE_TARGET_OPTION.
	* lto-streamer-out.c (DFS::DFS_write_tree_body): Follow
	DECL_FUNCTION_SPECIFIC_TARGET.
	(hash_tree): Hash TREE_TARGET_OPTION; visit
	DECL_FUNCTION_SPECIFIC_TARGET.
	* tree-streamer-out.c (streamer_pack_tree_bitfields): Skip
	TS_TARGET_OPTION.
	(streamer_write_tree_body): Output TS_TARGET_OPTION.

	* lto.c (compare_tree_sccs_1): Compare cl_target_option_eq.

Comments

Richard Biener Nov. 13, 2014, 10:48 a.m. UTC | #1
On Thu, 13 Nov 2014, Jan Hubicka wrote:

> Hi,
> this patch adds infrastructure for proper streaming and merging of
> TREE_TARGET_OPTION.  The catch is that TREE_TARGET_OPTION is autogenerated
> structure.  For x86_64 it looks as follows:
> 
> /* Structure to save/restore selected target specific options.  */
> struct GTY(()) cl_target_option
> {
>   HOST_WIDE_INT x_ix86_isa_flags_explicit;
>   HOST_WIDE_INT x_ix86_target_flags_explicit;
>   const char *x_ix86_arch_string;
>   const char *x_ix86_recip_name;
>   const char *x_ix86_tune_ctrl_string;
>   const char *x_ix86_tune_memcpy_strategy;
>   const char *x_ix86_tune_memset_strategy;
>   const char *x_ix86_tune_string;
>   HOST_WIDE_INT x_ix86_isa_flags;
>   enum fpmath_unit x_ix86_fpmath;
>   enum asm_dialect x_ix86_asm_dialect;
>   ...
>   enum stack_protector_guard x_ix86_stack_protector_guard;
>   enum stringop_alg x_ix86_stringop_alg;
>   enum tls_dialect x_ix86_tls_dialect;
>   int x_ix86_branch_cost;
>   int x_ix86_dump_tunes;
>   ....
>   int x_recip_mask;
>   int x_target_flags;
>   unsigned char arch;
>   unsigned char arch_specified;
>   unsigned char branch_cost;
>   unsigned char schedule;
>   unsigned char tune;
>   unsigned char tune_defaulted;
> };
> 
> The problem is that the strings prevents us from streaming the structure as a binary
> blob like we do for optimization nodes (and we should not do that to make LTO streaming
> stable across attribute tweaks in minor releases).
> 
> This patch extends the AWK generators to produce four functions:
> 
> /* Compare two target options  */
> bool
> cl_target_option_eq (struct cl_target_option const *ptr1,
>                      struct cl_target_option const *ptr2)
> {
>   if (ptr1->x_ix86_arch_string != ptr2->x_ix86_arch_string
>       && (!ptr1->x_ix86_arch_string || !ptr2->x_ix86_arch_string
>           || strcmp (ptr1->x_ix86_arch_string, ptr2->x_ix86_arch_string)))
>     return false;
> ....
>     if (ptr1->x_ix86_isa_flags != ptr2->x_ix86_isa_flags)
>     return false;
>   if (ptr1->x_ix86_fpmath != ptr2->x_ix86_fpmath)
>     return false;
>   return true;
> }
> 
> /* Hash target options  */
> hashval_t
> cl_target_option_hash (struct cl_target_option const *ptr)
> { 
>   inchash::hash hstate;
>   if (ptr->x_ix86_arch_string)
>     hstate.add (ptr->x_ix86_arch_string, strlen (ptr->x_ix86_arch_string));
>   else
>     hstate.add_int (0);
> ...
>   hstate.add_wide_int (ptr->x_ix86_fpmath);
>   return hstate.end ();
> }
> 
> /* Stream out target options  */
> void
> cl_target_option_stream_out (struct output_block *ob, struct cl_target_option *ptr)
> { 
>   streamer_write_string (ob, ob->main_stream, ptr->x_ix86_arch_string, true);
> ...
>   streamer_write_uhwi (ob, ptr->x_ix86_isa_flags);
>   streamer_write_uhwi (ob, ptr->x_ix86_fpmath);
> }
> 
> /* Stream in target options  */
> void
> cl_target_option_stream_in (struct lto_input_block *ib, struct data_in *data_in, struct cl_target_option *ptr)
> { 
>   ptr->x_ix86_arch_string = strdup (streamer_read_string (data_in, ib));
> ...
>   ptr->x_ix86_fpmath = (enum fpmath_unit ) streamer_read_uhwi (ib);
> }
> 
> These allows us to properly stream, compare and hash the values.
> 
> There is some implementation lazyness - for example I stream all scalars as
> unsigned HOST_WIDE_INT that seems to be the largest type used for variables
> across the targets.
> 
> If other types are used, the type matching (I stole from other part of the awk
> scripts) will need to be extended; for now it knows that const char * is a
> string and needs special care but nothing else.
> 
> I have tested this with firefox and additional change to tree.c to always
> initialize TARGET_OPTION_NODE for defined functions (so target units are
> peorperly presered across units). This seems to work as intended - i.e. the
> functions gets link-time optimized according to -march settings passed at
> compile time.
> 
> I noticed that memory use is now 3.8GB instead of previous 2.8GB - this may be
> tree merging problem caused by mismatching target settings but most probably it
> is an unrelated stuff. I will look into it tomorrow.  I also plan to test this
> on PPC (that currently fails to bootstrap).
> 
> Incrementally I would like to handle optimizations nodes same way and change
> streaming format to be a set of assignments field_name=value.
> This way we could behave sanely when some field is introduced or removed by
> either defaulting it or ignoring it.

I'm not sure that idea is good ;)  Adding a version / checksum to
detect a mismatch early would be nice though.  Maybe just compute
a hash for the fields in the AWK script?

> Bootstrapped/regtested x86_64-linux, seems sane?

Sounds sane - I'd like to have a AWK capable person have a look
for non-portable stuff (did you check AIX? ;))

Some more comments below

> 	* optc-save-gen.awk: Output cl_target_option_eq,
> 	cl_target_option_hash, cl_target_option_stream_out,
> 	cl_target_option_stream_in functions.
> 	* opth-gen.awk: Output prototypes for
> 	cl_target_option_eq and cl_target_option_hash.
> 	* lto-streamer.h (cl_target_option_stream_out,
> 	cl_target_option_stream_in): Declare.
> 	* tree.c (cl_option_hash_hash): Use cl_target_option_hash.
> 	(cl_option_hash_eq): Use cl_target_option_eq.
> 	* tree-streamer-in.c (unpack_value_fields): Stream in
> 	TREE_TARGET_OPTION.
> 	* lto-streamer-out.c (DFS::DFS_write_tree_body): Follow
> 	DECL_FUNCTION_SPECIFIC_TARGET.
> 	(hash_tree): Hash TREE_TARGET_OPTION; visit
> 	DECL_FUNCTION_SPECIFIC_TARGET.
> 	* tree-streamer-out.c (streamer_pack_tree_bitfields): Skip
> 	TS_TARGET_OPTION.
> 	(streamer_write_tree_body): Output TS_TARGET_OPTION.
> 
> 	* lto.c (compare_tree_sccs_1): Compare cl_target_option_eq.
> 	
> Index: optc-save-gen.awk
> ===================================================================
> --- optc-save-gen.awk	(revision 217448)
> +++ optc-save-gen.awk	(working copy)
> @@ -39,6 +39,16 @@ print "#include " quote "intl.h" quote
>  print ""
>  print "#include " quote "flags.h" quote
>  print "#include " quote "target.h" quote
> +print "#include " quote "inchash.h" quote
> +print "#include " quote "tree.h" quote
> +print "#include " quote "tree-ssa-alias.h" quote
> +print "#include " quote "is-a.h" quote
> +print "#include " quote "predict.h" quote
> +print "#include " quote "function.h" quote
> +print "#include " quote "basic-block.h" quote
> +print "#include " quote "gimple-expr.h" quote
> +print "#include " quote "gimple.h" quote
> +print "#include " quote "data-streamer.h" quote
>  print ""
>  
>  if (n_extra_c_includes > 0) {
> @@ -417,4 +427,120 @@ print "    targetm.target_option.print (
>  
>  print "}";
>  
> +print "";
> +print "/* Compare two target options  */";
> +print "bool";
> +print "cl_target_option_eq (struct cl_target_option const *ptr1,";
> +print "                     struct cl_target_option const *ptr2)";
> +print "{";
> +n_target_val = 0;
> +n_target_str = 0;
> +
> +for (i = 0; i < n_target_save; i++) {
> +	var = target_save_decl[i];
> +	sub (" *=.*", "", var);
> +	name = var;
> +	type = var;
> +	sub("^.*[ *]", "", name)
> +	sub(" *" name "$", "", type)
> +	if (target_save_decl[i] ~ "^const char \\*+[_" alnum "]+$")
> +		var_target_str[n_target_str++] = name;
> +	else {
> +		var_target_val_type[n_target_val] = type;
> +		var_target_val[n_target_val++] = name;
> +	}
> +}
> +if (have_save) {
> +	for (i = 0; i < n_opts; i++) {
> +		if (flag_set_p("Save", flags[i])) {
> +			name = var_name(flags[i])
> +			if(name == "")
> +				name = "target_flags";
> +
> +			if(name in var_list_seen)
> +				continue;
> +
> +			var_list_seen[name]++;
> +			otype = var_type_struct(flags[i])
> +			if (otype ~ "^const char \\**$")
> +				var_target_str[n_target_str++] = "x_" name;
> +			else {
> +				var_target_val_type[n_target_val] = otype;
> +				var_target_val[n_target_val++] = "x_" name;
> +			}
> +		}
> +	}
> +} else {
> +	var_target_val_type[n_target_val] = "int";
> +	var_target_val[n_target_val++] = "x_target_flags";
> +}
> +
> +for (i = 0; i < n_target_str; i++) {
> +	name = var_target_str[i]
> +	print "  if (ptr1->" name" != ptr2->" name;
> +	print "      && (!ptr1->" name" || !ptr2->" name
> +	print "          || strcmp (ptr1->" name", ptr2->" name ")))";
> +	print "    return false;";
> +}
> +for (i = 0; i < n_target_val; i++) {
> +	name = var_target_val[i]
> +	print "  if (ptr1->" name" != ptr2->" name ")";
> +	print "    return false;";
> +}
> +
> +print "  return true;";
> +
> +print "}";
> +
> +print "";
> +print "/* Hash target options  */";
> +print "hashval_t";
> +print "cl_target_option_hash (struct cl_target_option const *ptr)";
> +print "{";
> +print "  inchash::hash hstate;";
> +for (i = 0; i < n_target_str; i++) {
> +	name = var_target_str[i]
> +	print "  if (ptr->" name")";
> +	print "    hstate.add (ptr->" name", strlen (ptr->" name"));";
> +	print "  else";
> +	print "    hstate.add_int (0);";
> +}
> +for (i = 0; i < n_target_val; i++) {
> +	name = var_target_val[i]
> +	print "  hstate.add_wide_int (ptr->" name");";
> +}
> +print "  return hstate.end ();";
> +print "}";
> +
> +print "";
> +print "/* Stream out target options  */";
> +print "void";
> +print "cl_target_option_stream_out (struct output_block *ob, struct cl_target_option *ptr)";
> +print "{";
> +for (i = 0; i < n_target_str; i++) {
> +	name = var_target_str[i]
> +	print "  streamer_write_string (ob, ob->main_stream, ptr->" name", true);";
> +}
> +for (i = 0; i < n_target_val; i++) {
> +	name = var_target_val[i]
> +	print "  streamer_write_uhwi (ob, ptr->" name");";
> +}
> +print "}";
> +
> +print "";
> +print "/* Stream in target options  */";
> +print "void";
> +print "cl_target_option_stream_in (struct lto_input_block *ib, struct data_in *data_in, struct cl_target_option *ptr)";
> +print "{";
> +for (i = 0; i < n_target_str; i++) {
> +	name = var_target_str[i]
> +	print "  ptr->" name" = strdup (streamer_read_string (data_in, ib));";
> +}
> +for (i = 0; i < n_target_val; i++) {
> +	name = var_target_val[i]
> +	print "  ptr->" name" = (" var_target_val_type[i] ") streamer_read_uhwi (ib);";
> +}
> +
> +print "}";
> +
>  }
> Index: tree-streamer-in.c
> ===================================================================
> --- tree-streamer-in.c	(revision 217448)
> +++ tree-streamer-in.c	(working copy)
> @@ -506,9 +506,6 @@ unpack_value_fields (struct data_in *dat
>    if (CODE_CONTAINS_STRUCT (code, TS_TRANSLATION_UNIT_DECL))
>      unpack_ts_translation_unit_decl_value_fields (data_in, bp, expr);
>  
> -  if (CODE_CONTAINS_STRUCT (code, TS_TARGET_OPTION))
> -    gcc_unreachable ();
> -
>    if (CODE_CONTAINS_STRUCT (code, TS_OPTIMIZATION))
>      unpack_ts_optimization (bp, expr);
>  
> @@ -1081,6 +1078,9 @@ streamer_read_tree_body (struct lto_inpu
>    if (CODE_CONTAINS_STRUCT (code, TS_CONSTRUCTOR))
>      lto_input_ts_constructor_tree_pointers (ib, data_in, expr);
>  
> +  if (CODE_CONTAINS_STRUCT (code, TS_TARGET_OPTION))
> +    cl_target_option_stream_in (ib, data_in, TREE_TARGET_OPTION (expr));
> +
>    if (code == OMP_CLAUSE)
>      lto_input_ts_omp_clause_tree_pointers (ib, data_in, expr);

Any reason you don't use bitpacks and stream it from 
[un]pack_value_fields?  We can happily pack strings there as well.
The 'body' streaming is for tree pointers only.

>  }
> Index: lto-streamer.h
> ===================================================================
> --- lto-streamer.h	(revision 217448)
> +++ lto-streamer.h	(working copy)
> @@ -832,6 +832,14 @@ bool reachable_from_this_partition_p (st
>  				      lto_symtab_encoder_t);
>  lto_symtab_encoder_t compute_ltrans_boundary (lto_symtab_encoder_t encoder);
>  
> +/* In options-save.c.  */
> +void cl_target_option_stream_out (struct output_block *,
> +				  struct cl_target_option *);
> +
> +void cl_target_option_stream_in (struct lto_input_block *,
> +				 struct data_in *,
> +				 struct cl_target_option *);
> +
>  
>  /* In lto-symtab.c.  */
>  extern void lto_symtab_merge_decls (void);
> Index: tree.c
> ===================================================================
> --- tree.c	(revision 217448)
> +++ tree.c	(working copy)
> @@ -11484,10 +11495,7 @@ cl_option_hash_hash (const void *x)
>      }
>  
>    else if (TREE_CODE (t) == TARGET_OPTION_NODE)
> -    {
> -      p = (const char *)TREE_TARGET_OPTION (t);
> -      len = sizeof (struct cl_target_option);
> -    }
> +    return cl_target_option_hash (TREE_TARGET_OPTION (t));
>  
>    else
>      gcc_unreachable ();
> @@ -11526,9 +11534,8 @@ cl_option_hash_eq (const void *x, const
>  
>    else if (TREE_CODE (xt) == TARGET_OPTION_NODE)
>      {
> -      xp = (const char *)TREE_TARGET_OPTION (xt);
> -      yp = (const char *)TREE_TARGET_OPTION (yt);
> -      len = sizeof (struct cl_target_option);
> +      return cl_target_option_eq (TREE_TARGET_OPTION (xt),
> +				  TREE_TARGET_OPTION (yt));
>      }
>  
>    else
> Index: opth-gen.awk
> ===================================================================
> --- opth-gen.awk	(revision 217448)
> +++ opth-gen.awk	(working copy)
> @@ -293,6 +293,12 @@ print "";
>  print "/* Print target option variables from a structure.  */";
>  print "extern void cl_target_option_print (FILE *, int, struct cl_target_option *);";
>  print "";
> +print "/* Compare two target option variables from a structure.  */";
> +print "extern bool cl_target_option_eq (const struct cl_target_option *, const struct cl_target_option *);";
> +print "";
> +print "/* Hash option variables from a structure.  */";
> +print "extern hashval_t cl_target_option_hash (const struct cl_target_option *);";
> +print "";
>  print "/* Anything that includes tm.h, does not necessarily need this.  */"
>  print "#if !defined(GCC_TM_H)"
>  print "#include \"input.h\" /* for location_t */"
> Index: lto-streamer-out.c
> ===================================================================
> --- lto-streamer-out.c	(revision 217448)
> +++ lto-streamer-out.c	(working copy)
> @@ -594,7 +594,7 @@ DFS::DFS_write_tree_body (struct output_
>      {
>        DFS_follow_tree_edge (DECL_VINDEX (expr));
>        DFS_follow_tree_edge (DECL_FUNCTION_PERSONALITY (expr));
> -      /* Do not DECL_FUNCTION_SPECIFIC_TARGET.  They will be regenerated.  */
> +      DFS_follow_tree_edge (DECL_FUNCTION_SPECIFIC_TARGET (expr));
>        DFS_follow_tree_edge (DECL_FUNCTION_SPECIFIC_OPTIMIZATION (expr));
>      }
>  
> @@ -945,7 +945,7 @@ hash_tree (struct streamer_tree_cache_d
>  			strlen (TRANSLATION_UNIT_LANGUAGE (t)));
>  
>    if (CODE_CONTAINS_STRUCT (code, TS_TARGET_OPTION))
> -    gcc_unreachable ();
> +    hstate.add_wide_int (cl_target_option_hash (TREE_TARGET_OPTION (t)));
>  
>    if (CODE_CONTAINS_STRUCT (code, TS_OPTIMIZATION))
>      hstate.add (t, sizeof (struct cl_optimization));
> @@ -1028,7 +1028,7 @@ hash_tree (struct streamer_tree_cache_d
>      {
>        visit (DECL_VINDEX (t));
>        visit (DECL_FUNCTION_PERSONALITY (t));
> -      /* Do not follow DECL_FUNCTION_SPECIFIC_TARGET.  */
> +      visit (DECL_FUNCTION_SPECIFIC_TARGET (t));
>        visit (DECL_FUNCTION_SPECIFIC_OPTIMIZATION (t));
>      }
>  
> Index: lto/lto.c
> ===================================================================
> --- lto/lto.c	(revision 217448)
> +++ lto/lto.c	(working copy)
> @@ -1377,7 +1377,8 @@ compare_tree_sccs_1 (tree t1, tree t2, t
>        return false;
>  
>    if (CODE_CONTAINS_STRUCT (code, TS_TARGET_OPTION))
> -    gcc_unreachable ();
> +    if (cl_target_option_eq (TREE_TARGET_OPTION (t1), TREE_TARGET_OPTION (t2)))
> +      return false;

I suppose that's the reason for the memory use - should be
!cl_target_option_eq I think ;)

>  
>    if (CODE_CONTAINS_STRUCT (code, TS_OPTIMIZATION))
>      if (memcmp (TREE_OPTIMIZATION (t1), TREE_OPTIMIZATION (t2),
> Index: tree-streamer-out.c
> ===================================================================
> --- tree-streamer-out.c	(revision 217448)
> +++ tree-streamer-out.c	(working copy)
> @@ -472,9 +472,6 @@ streamer_pack_tree_bitfields (struct out
>    if (CODE_CONTAINS_STRUCT (code, TS_TRANSLATION_UNIT_DECL))
>      pack_ts_translation_unit_decl_value_fields (ob, bp, expr);
>  
> -  if (CODE_CONTAINS_STRUCT (code, TS_TARGET_OPTION))
> -    gcc_unreachable ();
> -
>    if (CODE_CONTAINS_STRUCT (code, TS_OPTIMIZATION))
>      pack_ts_optimization (bp, expr);
>  
> @@ -963,6 +960,9 @@ streamer_write_tree_body (struct output_
>    if (CODE_CONTAINS_STRUCT (code, TS_CONSTRUCTOR))
>      write_ts_constructor_tree_pointers (ob, expr, ref_p);
>  
> +  if (CODE_CONTAINS_STRUCT (code, TS_TARGET_OPTION))
> +    cl_target_option_stream_out (ob, TREE_TARGET_OPTION (expr));
> +
>    if (code == OMP_CLAUSE)
>      write_ts_omp_clause_tree_pointers (ob, expr, ref_p);
>  }

Otherwise looks good.

Thanks,
Richard.
Jan Hubicka Nov. 13, 2014, 3:22 p.m. UTC | #2
> > 
> > Incrementally I would like to handle optimizations nodes same way and change
> > streaming format to be a set of assignments field_name=value.
> > This way we could behave sanely when some field is introduced or removed by
> > either defaulting it or ignoring it.
> 
> I'm not sure that idea is good ;)  Adding a version / checksum to
> detect a mismatch early would be nice though.  Maybe just compute
> a hash for the fields in the AWK script?

OK, lets see how to handle it incrementaly.
> 
> > Bootstrapped/regtested x86_64-linux, seems sane?
> 
> Sounds sane - I'd like to have a AWK capable person have a look
> for non-portable stuff (did you check AIX? ;))

Good idea, will do today :)
Basically all the AWK is cut&paste from the way save loop is generated, no
really new constructs in it.
> > +  if (CODE_CONTAINS_STRUCT (code, TS_TARGET_OPTION))
> > +    cl_target_option_stream_in (ib, data_in, TREE_TARGET_OPTION (expr));
> > +
> >    if (code == OMP_CLAUSE)
> >      lto_input_ts_omp_clause_tree_pointers (ib, data_in, expr);
> 
> Any reason you don't use bitpacks and stream it from 
> [un]pack_value_fields?  We can happily pack strings there as well.
> The 'body' streaming is for tree pointers only.

Hmm, I did not notice we have pack_string.  Will go with bitpacks then.
> > @@ -963,6 +960,9 @@ streamer_write_tree_body (struct output_
> >    if (CODE_CONTAINS_STRUCT (code, TS_CONSTRUCTOR))
> >      write_ts_constructor_tree_pointers (ob, expr, ref_p);
> >  
> > +  if (CODE_CONTAINS_STRUCT (code, TS_TARGET_OPTION))
> > +    cl_target_option_stream_out (ob, TREE_TARGET_OPTION (expr));
> > +
> >    if (code == OMP_CLAUSE)
> >      write_ts_omp_clause_tree_pointers (ob, expr, ref_p);
> >  }
> 
> Otherwise looks good.

OK, I will update the changes above and test AIX today.
Incrementally I will send patch to disable the TARGET_OPTION_NODE regeeneration from attribute
and to stream TARGET_OPTION_NODE for all functions (this is tested on firefox and AIX by now
and works).

For optimization node I think I can follow exactly the same strategy (to avoid writting binary blobs
that are not portable). I plan to add Ipa and Late modifiers to the format to annotate flags that
affects only Ipa or Late codegen/optimization (ltrans).  This way we can add comparers for inliner
ignoring the flags that are not marked Late and streamer that streams only Ipa/Late.

If this plan sounds sane, I can do it today I believe.

Across stage3 early stage3 I can update IPA passes to not check global flags during LTO so
we can support correctly support different LTO flags (like -fno-inline) in the optimization
nodes...

Honza
> 
> Thanks,
> Richard.
diff mbox

Patch

Index: optc-save-gen.awk
===================================================================
--- optc-save-gen.awk	(revision 217448)
+++ optc-save-gen.awk	(working copy)
@@ -39,6 +39,16 @@  print "#include " quote "intl.h" quote
 print ""
 print "#include " quote "flags.h" quote
 print "#include " quote "target.h" quote
+print "#include " quote "inchash.h" quote
+print "#include " quote "tree.h" quote
+print "#include " quote "tree-ssa-alias.h" quote
+print "#include " quote "is-a.h" quote
+print "#include " quote "predict.h" quote
+print "#include " quote "function.h" quote
+print "#include " quote "basic-block.h" quote
+print "#include " quote "gimple-expr.h" quote
+print "#include " quote "gimple.h" quote
+print "#include " quote "data-streamer.h" quote
 print ""
 
 if (n_extra_c_includes > 0) {
@@ -417,4 +427,120 @@  print "    targetm.target_option.print (
 
 print "}";
 
+print "";
+print "/* Compare two target options  */";
+print "bool";
+print "cl_target_option_eq (struct cl_target_option const *ptr1,";
+print "                     struct cl_target_option const *ptr2)";
+print "{";
+n_target_val = 0;
+n_target_str = 0;
+
+for (i = 0; i < n_target_save; i++) {
+	var = target_save_decl[i];
+	sub (" *=.*", "", var);
+	name = var;
+	type = var;
+	sub("^.*[ *]", "", name)
+	sub(" *" name "$", "", type)
+	if (target_save_decl[i] ~ "^const char \\*+[_" alnum "]+$")
+		var_target_str[n_target_str++] = name;
+	else {
+		var_target_val_type[n_target_val] = type;
+		var_target_val[n_target_val++] = name;
+	}
+}
+if (have_save) {
+	for (i = 0; i < n_opts; i++) {
+		if (flag_set_p("Save", flags[i])) {
+			name = var_name(flags[i])
+			if(name == "")
+				name = "target_flags";
+
+			if(name in var_list_seen)
+				continue;
+
+			var_list_seen[name]++;
+			otype = var_type_struct(flags[i])
+			if (otype ~ "^const char \\**$")
+				var_target_str[n_target_str++] = "x_" name;
+			else {
+				var_target_val_type[n_target_val] = otype;
+				var_target_val[n_target_val++] = "x_" name;
+			}
+		}
+	}
+} else {
+	var_target_val_type[n_target_val] = "int";
+	var_target_val[n_target_val++] = "x_target_flags";
+}
+
+for (i = 0; i < n_target_str; i++) {
+	name = var_target_str[i]
+	print "  if (ptr1->" name" != ptr2->" name;
+	print "      && (!ptr1->" name" || !ptr2->" name
+	print "          || strcmp (ptr1->" name", ptr2->" name ")))";
+	print "    return false;";
+}
+for (i = 0; i < n_target_val; i++) {
+	name = var_target_val[i]
+	print "  if (ptr1->" name" != ptr2->" name ")";
+	print "    return false;";
+}
+
+print "  return true;";
+
+print "}";
+
+print "";
+print "/* Hash target options  */";
+print "hashval_t";
+print "cl_target_option_hash (struct cl_target_option const *ptr)";
+print "{";
+print "  inchash::hash hstate;";
+for (i = 0; i < n_target_str; i++) {
+	name = var_target_str[i]
+	print "  if (ptr->" name")";
+	print "    hstate.add (ptr->" name", strlen (ptr->" name"));";
+	print "  else";
+	print "    hstate.add_int (0);";
+}
+for (i = 0; i < n_target_val; i++) {
+	name = var_target_val[i]
+	print "  hstate.add_wide_int (ptr->" name");";
+}
+print "  return hstate.end ();";
+print "}";
+
+print "";
+print "/* Stream out target options  */";
+print "void";
+print "cl_target_option_stream_out (struct output_block *ob, struct cl_target_option *ptr)";
+print "{";
+for (i = 0; i < n_target_str; i++) {
+	name = var_target_str[i]
+	print "  streamer_write_string (ob, ob->main_stream, ptr->" name", true);";
+}
+for (i = 0; i < n_target_val; i++) {
+	name = var_target_val[i]
+	print "  streamer_write_uhwi (ob, ptr->" name");";
+}
+print "}";
+
+print "";
+print "/* Stream in target options  */";
+print "void";
+print "cl_target_option_stream_in (struct lto_input_block *ib, struct data_in *data_in, struct cl_target_option *ptr)";
+print "{";
+for (i = 0; i < n_target_str; i++) {
+	name = var_target_str[i]
+	print "  ptr->" name" = strdup (streamer_read_string (data_in, ib));";
+}
+for (i = 0; i < n_target_val; i++) {
+	name = var_target_val[i]
+	print "  ptr->" name" = (" var_target_val_type[i] ") streamer_read_uhwi (ib);";
+}
+
+print "}";
+
 }
Index: tree-streamer-in.c
===================================================================
--- tree-streamer-in.c	(revision 217448)
+++ tree-streamer-in.c	(working copy)
@@ -506,9 +506,6 @@  unpack_value_fields (struct data_in *dat
   if (CODE_CONTAINS_STRUCT (code, TS_TRANSLATION_UNIT_DECL))
     unpack_ts_translation_unit_decl_value_fields (data_in, bp, expr);
 
-  if (CODE_CONTAINS_STRUCT (code, TS_TARGET_OPTION))
-    gcc_unreachable ();
-
   if (CODE_CONTAINS_STRUCT (code, TS_OPTIMIZATION))
     unpack_ts_optimization (bp, expr);
 
@@ -1081,6 +1078,9 @@  streamer_read_tree_body (struct lto_inpu
   if (CODE_CONTAINS_STRUCT (code, TS_CONSTRUCTOR))
     lto_input_ts_constructor_tree_pointers (ib, data_in, expr);
 
+  if (CODE_CONTAINS_STRUCT (code, TS_TARGET_OPTION))
+    cl_target_option_stream_in (ib, data_in, TREE_TARGET_OPTION (expr));
+
   if (code == OMP_CLAUSE)
     lto_input_ts_omp_clause_tree_pointers (ib, data_in, expr);
 }
Index: lto-streamer.h
===================================================================
--- lto-streamer.h	(revision 217448)
+++ lto-streamer.h	(working copy)
@@ -832,6 +832,14 @@  bool reachable_from_this_partition_p (st
 				      lto_symtab_encoder_t);
 lto_symtab_encoder_t compute_ltrans_boundary (lto_symtab_encoder_t encoder);
 
+/* In options-save.c.  */
+void cl_target_option_stream_out (struct output_block *,
+				  struct cl_target_option *);
+
+void cl_target_option_stream_in (struct lto_input_block *,
+				 struct data_in *,
+				 struct cl_target_option *);
+
 
 /* In lto-symtab.c.  */
 extern void lto_symtab_merge_decls (void);
Index: tree.c
===================================================================
--- tree.c	(revision 217448)
+++ tree.c	(working copy)
@@ -11484,10 +11495,7 @@  cl_option_hash_hash (const void *x)
     }
 
   else if (TREE_CODE (t) == TARGET_OPTION_NODE)
-    {
-      p = (const char *)TREE_TARGET_OPTION (t);
-      len = sizeof (struct cl_target_option);
-    }
+    return cl_target_option_hash (TREE_TARGET_OPTION (t));
 
   else
     gcc_unreachable ();
@@ -11526,9 +11534,8 @@  cl_option_hash_eq (const void *x, const
 
   else if (TREE_CODE (xt) == TARGET_OPTION_NODE)
     {
-      xp = (const char *)TREE_TARGET_OPTION (xt);
-      yp = (const char *)TREE_TARGET_OPTION (yt);
-      len = sizeof (struct cl_target_option);
+      return cl_target_option_eq (TREE_TARGET_OPTION (xt),
+				  TREE_TARGET_OPTION (yt));
     }
 
   else
Index: opth-gen.awk
===================================================================
--- opth-gen.awk	(revision 217448)
+++ opth-gen.awk	(working copy)
@@ -293,6 +293,12 @@  print "";
 print "/* Print target option variables from a structure.  */";
 print "extern void cl_target_option_print (FILE *, int, struct cl_target_option *);";
 print "";
+print "/* Compare two target option variables from a structure.  */";
+print "extern bool cl_target_option_eq (const struct cl_target_option *, const struct cl_target_option *);";
+print "";
+print "/* Hash option variables from a structure.  */";
+print "extern hashval_t cl_target_option_hash (const struct cl_target_option *);";
+print "";
 print "/* Anything that includes tm.h, does not necessarily need this.  */"
 print "#if !defined(GCC_TM_H)"
 print "#include \"input.h\" /* for location_t */"
Index: lto-streamer-out.c
===================================================================
--- lto-streamer-out.c	(revision 217448)
+++ lto-streamer-out.c	(working copy)
@@ -594,7 +594,7 @@  DFS::DFS_write_tree_body (struct output_
     {
       DFS_follow_tree_edge (DECL_VINDEX (expr));
       DFS_follow_tree_edge (DECL_FUNCTION_PERSONALITY (expr));
-      /* Do not DECL_FUNCTION_SPECIFIC_TARGET.  They will be regenerated.  */
+      DFS_follow_tree_edge (DECL_FUNCTION_SPECIFIC_TARGET (expr));
       DFS_follow_tree_edge (DECL_FUNCTION_SPECIFIC_OPTIMIZATION (expr));
     }
 
@@ -945,7 +945,7 @@  hash_tree (struct streamer_tree_cache_d
 			strlen (TRANSLATION_UNIT_LANGUAGE (t)));
 
   if (CODE_CONTAINS_STRUCT (code, TS_TARGET_OPTION))
-    gcc_unreachable ();
+    hstate.add_wide_int (cl_target_option_hash (TREE_TARGET_OPTION (t)));
 
   if (CODE_CONTAINS_STRUCT (code, TS_OPTIMIZATION))
     hstate.add (t, sizeof (struct cl_optimization));
@@ -1028,7 +1028,7 @@  hash_tree (struct streamer_tree_cache_d
     {
       visit (DECL_VINDEX (t));
       visit (DECL_FUNCTION_PERSONALITY (t));
-      /* Do not follow DECL_FUNCTION_SPECIFIC_TARGET.  */
+      visit (DECL_FUNCTION_SPECIFIC_TARGET (t));
       visit (DECL_FUNCTION_SPECIFIC_OPTIMIZATION (t));
     }
 
Index: lto/lto.c
===================================================================
--- lto/lto.c	(revision 217448)
+++ lto/lto.c	(working copy)
@@ -1377,7 +1377,8 @@  compare_tree_sccs_1 (tree t1, tree t2, t
       return false;
 
   if (CODE_CONTAINS_STRUCT (code, TS_TARGET_OPTION))
-    gcc_unreachable ();
+    if (cl_target_option_eq (TREE_TARGET_OPTION (t1), TREE_TARGET_OPTION (t2)))
+      return false;
 
   if (CODE_CONTAINS_STRUCT (code, TS_OPTIMIZATION))
     if (memcmp (TREE_OPTIMIZATION (t1), TREE_OPTIMIZATION (t2),
Index: tree-streamer-out.c
===================================================================
--- tree-streamer-out.c	(revision 217448)
+++ tree-streamer-out.c	(working copy)
@@ -472,9 +472,6 @@  streamer_pack_tree_bitfields (struct out
   if (CODE_CONTAINS_STRUCT (code, TS_TRANSLATION_UNIT_DECL))
     pack_ts_translation_unit_decl_value_fields (ob, bp, expr);
 
-  if (CODE_CONTAINS_STRUCT (code, TS_TARGET_OPTION))
-    gcc_unreachable ();
-
   if (CODE_CONTAINS_STRUCT (code, TS_OPTIMIZATION))
     pack_ts_optimization (bp, expr);
 
@@ -963,6 +960,9 @@  streamer_write_tree_body (struct output_
   if (CODE_CONTAINS_STRUCT (code, TS_CONSTRUCTOR))
     write_ts_constructor_tree_pointers (ob, expr, ref_p);
 
+  if (CODE_CONTAINS_STRUCT (code, TS_TARGET_OPTION))
+    cl_target_option_stream_out (ob, TREE_TARGET_OPTION (expr));
+
   if (code == OMP_CLAUSE)
     write_ts_omp_clause_tree_pointers (ob, expr, ref_p);
 }