Make IPA-CP propagate alignment information of pointers

Hi,

thanks for the comments.

On Mon, Dec 01, 2014 at 11:40:59PM +0100, Jan Hubicka wrote:
> > > 2014-11-19  Martin Jambor  <mjambor@suse.cz>
> > > 
> > > 	* ipa-prop.h (ipa_alignment): New type.
> > > 	(ipa_jump_func): New field alignment.
> > > 	(ipcp_transformation_summary) New type.
> > > 	(ipcp_grow_transformations_if_necessary): Declare.
> > > 	(ipa_node_agg_replacements): Removed.
> > > 	(ipcp_transformations): Declare.
> > > 	(ipcp_get_transformation_summary): New function.
> > > 	(ipa_get_agg_replacements_for_node): Use it.
> > > 	* ipa-cp.c (ipcp_param_lattices): New field alignment.
> > > 	(print_all_lattices): Also print alignment.
> > > 	(alignment_bottom_p): New function.
> > > 	(set_alignment_to_bottom): Likewise.
> > > 	(set_all_contains_variable): Also set alignment to bottom.
> > > 	(initialize_node_lattices): Likewise.
> > > 	(propagate_alignment_accross_jump_function): New function.
> > > 	(propagate_constants_accross_call): Call it.
> > > 	(ipcp_store_alignment_results): New function.
> > > 	(ipcp_driver): Call it.
> > > 	* ipa-prop.c (ipa_node_agg_replacements): Removed.
> > > 	(ipcp_transformations): New.
> > > 	(ipa_print_node_jump_functions_for_edge): Also print alignment.
> > > 	(ipa_set_jf_unknown): New function.
> > > 	(detect_type_change_from_memory_writes): Use ipa_set_jf_unknown.
> > > 	(ipa_compute_jump_functions_for_edge): Also calculate alignment.
> > > 	(update_jump_functions_after_inlining): Use ipa_set_jf_unknown.
> > > 	(ipcp_grow_transformations_if_necessary): New function.
> > > 	(ipa_set_node_agg_value_chain): Use ipcp_transformations.
> > > 	(ipa_node_removal_hook): Likewise.
> > > 	(ipa_node_duplication_hook): Also duplicate alignment results.
> > > 	(ipa_write_jump_function): Also stream alignments.
> > > 	(ipa_read_jump_function): Use ipa_set_jf_unknown, also stream
> > > 	alignments.
> > > 	(write_agg_replacement_chain): Renamed to
> > > 	write_ipcp_transformation_info, also stream alignments.
> > > 	(read_agg_replacement_chain): Renamed to
> > > 	read_ipcp_transformation_info, also stream alignments.
> > > 	(ipa_prop_write_all_agg_replacement): Renamed to
> > > 	ipcp_write_transformation_summaries. Stream always.
> > > 	(ipa_prop_read_all_agg_replacement): Renamed to
> > > 	ipcp_read_transformation_summaries.
> > > 	(ipcp_update_alignments): New function.
> > > 	(ipcp_transform_function): Call it, free also alignments.
> 
> In longer term I think we should just propagate value range and known to be zero/nonzero
> bits.  This is stronger and more universal than inventing propagation for subproblems
> (like non-NULL would be useful).
> I think it would be also good excuse to do (simplified) early VRP to populate the vlaue
> ranges locally.  This seems to make quite some difference on C++.

Well, it would essentially need some sort of early VRP.  So yes,
hopefully in the next major version we can do that.

> 
> But for 5.0 it seems like resonable thing to do.
> 
> Since ipa-prop itself is built as propaation engine for multiple types of values
> (scalars, aggregates, contexts and now value ranges).

I believe that if we start propagating value ranges, this would make
propagating scalars redundant?  Also it would need some rethinking of
the ipa-cp propagation heuristics.

> I wonder if the code can not be
> organized by type of propagated value and perhaps split into multiple file to make
> it more readable.

Splitting off of some part to extra files might help but first it is
necessary to figure out how to reorganize the contents of the file in
general.  Jump functions should probably be re-thought a little bit,
especially once we add information like reference mapping from my
aggregate propagation patches.

It would also be beneficial to do some analysis together with
generating the inline summaries and tweak ipa-analysis summary
conditions so that they can tell which value ranges would lead to the
same results.  And so on and so forth.

> 
> I think the way we split polymorphic-call-context lattice operations into separate
> file quite works but more of code separation would be nice.
> 
> Well, a bit like df.c infrastructure is done.
> > > +  else if (jfunc->type == IPA_JF_PASS_THROUGH
> > > +	   || jfunc->type == IPA_JF_ANCESTOR)
> > > +    {
> > > +      struct ipa_node_params *caller_info = IPA_NODE_REF (cs->caller);
> > > +      struct ipcp_param_lattices *src_lats;
> > > +      HOST_WIDE_INT offset = 0;
> > > +      int src_idx;
> > > +
> 
> We probably chould avoid new places that rely on tree_fits_shwi_p and use wide_int
> instead.

On IRC we agreed that this making change here alone is of very limited
benefit, unlike using wide_int more in IPA data structures overall,
something which should be done in 6.0.

> > > +      if (POINTER_TYPE_P (TREE_TYPE(arg)))
> > > +	{
> > > +	  unsigned HOST_WIDE_INT hwi_bitpos;
> > > +	  unsigned align;
> > > +
> > > +	  if (get_pointer_alignment_1 (arg, &align, &hwi_bitpos)
> > > +	      && align > BITS_PER_UNIT)
> 
> OK, so here we get the nonzero/zero bits propagation from CCP used, right?
> 
> > > +  ipcp_transformation_summary *ts = ipcp_get_transformation_summary (node);
> > > +  if (ts && vec_safe_length (ts->alignments) > 0)
> > > +    {
> > > +      count = ts->alignments->length ();
> > > +
> > > +      streamer_write_uhwi (ob, count);
> > > +      for (unsigned i = 0; i < count; ++i)
> > > +	{
> > > +	  ipa_alignment *parm_al = &(*ts->alignments)[i];
> > > +
> > > +	  struct bitpack_d bp;
> > > +	  bp = bitpack_create (ob->main_stream);
> > > +	  bp_pack_value (&bp, parm_al->known, 1);
> > > +	  streamer_write_bitpack (&bp);
> > > +	  if (parm_al->known)
> > > +	    {
> > > +	      streamer_write_uhwi (ob, parm_al->align);
> > > +	      streamer_write_uhwi (ob, parm_al->misalign);
> 
> What about using streamer_write_hwi_in_range for misalign (as it is in range 0...align)
> to get tiny bit of extra sanity check & code size saving?
> We probably also get sane range for ->align. 

I have changed this, which lead to the following patch which I intend
to commit after a final round of bootstrap and testing.

Thanks,

Martin


2014-12-02  Martin Jambor  <mjambor@suse.cz>

	* ipa-prop.h (ipa_alignment): New type.
	(ipa_jump_func): New field alignment.
	(ipcp_transformation_summary) New type.
	(ipcp_grow_transformations_if_necessary): Declare.
	(ipa_node_agg_replacements): Removed.
	(ipcp_transformations): Declare.
	(ipcp_get_transformation_summary): New function.
	(ipa_get_agg_replacements_for_node): Use it.
	* ipa-cp.c (ipcp_param_lattices): New field alignment.
	(print_all_lattices): Also print alignment.
	(alignment_bottom_p): New function.
	(set_alignment_to_bottom): Likewise.
	(set_all_contains_variable): Also set alignment to bottom.
	(initialize_node_lattices): Likewise.
	(propagate_alignment_accross_jump_function): New function.
	(propagate_constants_accross_call): Call it.
	(ipcp_store_alignment_results): New function.
	(ipcp_driver): Call it.
	* ipa-prop.c (ipa_node_agg_replacements): Removed.
	(ipcp_transformations): New.
	(ipa_print_node_jump_functions_for_edge): Also print alignment.
	(ipa_set_jf_unknown): New function.
	(detect_type_change_from_memory_writes): Use ipa_set_jf_unknown.
	(ipa_compute_jump_functions_for_edge): Also calculate alignment.
	(update_jump_functions_after_inlining): Use ipa_set_jf_unknown.
	(ipcp_grow_transformations_if_necessary): New function.
	(ipa_set_node_agg_value_chain): Use ipcp_transformations.
	(ipa_node_removal_hook): Likewise.
	(ipa_node_duplication_hook): Also duplicate alignment results.
	(ipa_write_jump_function): Also stream alignments.
	(ipa_read_jump_function): Use ipa_set_jf_unknown, also stream
	alignments.
	(write_agg_replacement_chain): Renamed to
	write_ipcp_transformation_info, also stream alignments.
	(read_agg_replacement_chain): Renamed to
	read_ipcp_transformation_info, also stream alignments.
	(ipa_prop_write_all_agg_replacement): Renamed to
	ipcp_write_transformation_summaries. Stream always.
	(ipa_prop_read_all_agg_replacement): Renamed to
	ipcp_read_transformation_summaries.
	(ipcp_update_alignments): New function.
	(ipcp_transform_function): Call it, free also alignments.

testsuite/
	* gcc.dg/ipa/propalign-1.c: New test.
	* gcc.dg/ipa/propalign-2.c: Likewise.

> Index: src/gcc/ipa-cp.c
> ===================================================================
> --- src.orig/gcc/ipa-cp.c
> +++ src/gcc/ipa-cp.c
> @@ -262,6 +262,9 @@ public:
>    ipcp_lattice<ipa_polymorphic_call_context> ctxlat;
>    /* Lattices describing aggregate parts.  */
>    ipcp_agg_lattice *aggs;
> +  /* Alignment information.  Very basic one value lattice where !known means
> +     TOP and zero alignment bottom.  */
> +  ipa_alignment alignment;
>    /* Number of aggregate lattices */
>    int aggs_count;
>    /* True if aggregate data were passed by reference (as opposed to by
> @@ -444,6 +447,13 @@ print_all_lattices (FILE * f, bool dump_
>           plats->itself.print (f, dump_sources, dump_benefits);
>           fprintf (f, "         ctxs: ");
>           plats->ctxlat.print (f, dump_sources, dump_benefits);
> +         if (plats->alignment.known && plats->alignment.align > 0)
> +           fprintf (f, "         Alignment %u, misaglignment %u\n",

"misalignment"

> +                    plats->alignment.align, plats->alignment.misalign);
> +         else if (plats->alignment.known)
> +           fprintf (f, "         Alignment unusable\n");
> +         else
> +           fprintf (f, "         Alignment unknown\n");
>           if (plats->virt_call)
>             fprintf (f, "        virt_call flag set\n");
>
> @@ -761,6 +771,27 @@ set_agg_lats_contain_variable (struct ip
>    return ret;
>  }
>
> +/* Return true if alignemnt informatin in PLATS is known to be unusable.  */

"alignment", "information"

> +
> +static inline bool
> +alignment_bottom_p (ipcp_param_lattices *plats)
> +{
> +  return plats->alignment.known && (plats->alignment.align == 0);
> +}
> +
> +/* Set alignment information in PLATS to unusable.  Return true if it
> +   previously was usable or unknown.  */
> +
> +static inline bool
> +set_alignment_to_bottom (ipcp_param_lattices *plats)
> +{
> +  if (alignment_bottom_p (plats))
> +    return false;
> +  plats->alignment.known = true;
> +  plats->alignment.align = 0;
> +  return true;
> +}
> +
>  /* Mark bot aggregate and scalar lattices as containing an unknown variable,
>     return true is any of them has not been marked as such so far.  */
>
> @@ -771,6 +802,7 @@ set_all_contains_variable (struct ipcp_p
>    ret = plats->itself.set_contains_variable ();
>    ret |= plats->ctxlat.set_contains_variable ();
>    ret |= set_agg_lats_contain_variable (plats);
> +  ret |= set_alignment_to_bottom (plats);
>    return ret;
>  }
>
> @@ -807,6 +839,7 @@ initialize_node_lattices (struct cgraph_
>               plats->itself.set_to_bottom ();
>               plats->ctxlat.set_to_bottom ();
>               set_agg_lats_to_bottom (plats);
> +             set_alignment_to_bottom (plats);
>             }
>           else
>             set_all_contains_variable (plats);
> @@ -1369,6 +1402,77 @@ propagate_context_accross_jump_function
>    return ret;
>  }
>
> +/* Propagate alignments accross jump function JFUNC that is associated with

"across"

> +   edge CS and update DEST_LAT accordingly.  */
> +
> +static bool
> +propagate_alignment_accross_jump_function (struct cgraph_edge *cs,
> +                                          struct ipa_jump_func *jfunc,
> +                                          struct ipcp_param_lattices *dest_lat)
> +{
> +  if (alignment_bottom_p (dest_lat))
> +    return false;
> +
> +  ipa_alignment cur;
> +  cur.known = false;
> +  if (jfunc->alignment.known)
> +    cur = jfunc->alignment;
> +  else if (jfunc->type == IPA_JF_PASS_THROUGH
> +          || jfunc->type == IPA_JF_ANCESTOR)
> +    {
> +      struct ipa_node_params *caller_info = IPA_NODE_REF (cs->caller);
> +      struct ipcp_param_lattices *src_lats;
> +      HOST_WIDE_INT offset = 0;
> +      int src_idx;
> +
> +      if (jfunc->type == IPA_JF_PASS_THROUGH)
> +       {
> +         enum tree_code op = ipa_get_jf_pass_through_operation (jfunc);
> +         if (op != NOP_EXPR)
> +           {
> +             if (op != POINTER_PLUS_EXPR
> +                 && op != PLUS_EXPR
> +                 && op != MINUS_EXPR)
> +               goto prop_fail;
> +             tree operand = ipa_get_jf_pass_through_operand (jfunc);
> +             if (!tree_fits_shwi_p (operand))
> +               goto prop_fail;
> +             offset = tree_to_shwi (operand);
> +           }
> +         src_idx = ipa_get_jf_pass_through_formal_id (jfunc);
> +       }
> +      else
> +       {
> +         src_idx = ipa_get_jf_ancestor_formal_id (jfunc);
> +         offset = ipa_get_jf_ancestor_offset (jfunc);
> +       }
> +
> +      src_lats = ipa_get_parm_lattices (caller_info, src_idx);
> +      if (!src_lats->alignment.known
> +         || alignment_bottom_p (src_lats))
> +       goto prop_fail;
> +
> +      cur = src_lats->alignment;
> +      cur.misalign = (cur.misalign + offset) % cur.align;
> +    }
> +
> +  if (cur.known)
> +    {
> +      if (!dest_lat->alignment.known)
> +       {
> +         dest_lat->alignment = cur;
> +         return true;
> +       }
> +      else if (dest_lat->alignment.align == cur.align
> +              && dest_lat->alignment.misalign == cur.misalign)
> +       return false;
> +    }
> +
> + prop_fail:
> +  set_alignment_to_bottom (dest_lat);
> +  return true;
> +}
> +
>  /* If DEST_PLATS already has aggregate items, check that aggs_by_ref matches
>     NEW_AGGS_BY_REF and if not, mark all aggs as bottoms and return true (in all
>     other cases, return false).  If there are no aggregate items, set
> @@ -1705,6 +1809,8 @@ propagate_constants_accross_call (struct
>                                                          &dest_plats->itself);
>           ret |= propagate_context_accross_jump_function (cs, jump_func, i,
>                                                           &dest_plats->ctxlat);
> +         ret |= propagate_alignment_accross_jump_function (cs, jump_func,
> +                                                           dest_plats);
>           ret |= propagate_aggs_accross_jump_function (cs, jump_func,
>                                                        dest_plats);
>         }
> @@ -4191,6 +4297,63 @@ ipcp_decision_stage (struct ipa_topo_inf
>      }
>  }
>
> +/* Look up all alignment information that we have discovered and copy it over
> +   to the transformation summary.  */
> +
> +static void
> +ipcp_store_alignment_results (void)
> +{
> +  cgraph_node *node;
> +
> +  FOR_EACH_FUNCTION_WITH_GIMPLE_BODY (node)
> +  {
> +    ipa_node_params *info = IPA_NODE_REF (node);
> +    bool dumped_sth = false;
> +    bool found_useful_result = false;
> +
> +   if (info->ipcp_orig_node)
> +      info = IPA_NODE_REF (info->ipcp_orig_node);
> +
> +   unsigned count = ipa_get_param_count (info);
> +   for (unsigned i = 0; i < count ; i++)
> +     {
> +       ipcp_param_lattices *plats = ipa_get_parm_lattices (info, i);
> +       if (plats->alignment.known
> +          && plats->alignment.align > 0)
> +        {
> +          found_useful_result = true;
> +          break;
> +        }
> +     }
> +   if (!found_useful_result)
> +     continue;
> +
> +  ipcp_grow_transformations_if_necessary ();
> +   ipcp_transformation_summary *ts = ipcp_get_transformation_summary (node);
> +   vec_safe_reserve_exact (ts->alignments, count);
> +
> +   for (unsigned i = 0; i < count ; i++)
> +     {
> +       ipcp_param_lattices *plats = ipa_get_parm_lattices (info, i);
> +
> +       if (plats->alignment.align == 0)
> +        plats->alignment.known = false;
> +
> +       ts->alignments->quick_push (plats->alignment);
> +       if (!dump_file || !plats->alignment.known)
> +        continue;
> +       if (!dumped_sth)
> +        {
> +          fprintf (dump_file, "Propagated alignment info for function %s/%i:\n",
> +                   node->name (), node->order);
> +          dumped_sth = true;
> +        }
> +       fprintf (dump_file, "  param %i: align: %u, misalign: %u\n",
> +               i, plats->alignment.align, plats->alignment.misalign);
> +     }
> +  }
> +}
> +
>  /* The IPCP driver.  */
>
>  static unsigned int
> @@ -4232,6 +4395,8 @@ ipcp_driver (void)
>    ipcp_propagate_stage (&topo);
>    /* Decide what constant propagation and cloning should be performed.  */
>    ipcp_decision_stage (&topo);
> +  /* Store results of alignment propagation. */
> +  ipcp_store_alignment_results ();
>
>    /* Free all IPCP structures.  */
>    free_toporder_info (&topo);
> @@ -4304,9 +4469,9 @@ public:
>                       ipcp_generate_summary, /* generate_summary */
>                       ipcp_write_summary, /* write_summary */
>                       ipcp_read_summary, /* read_summary */
> -                     ipa_prop_write_all_agg_replacement, /*
> +                     ipcp_write_transformation_summaries, /*
>                       write_optimization_summary */
> -                     ipa_prop_read_all_agg_replacement, /*
> +                     ipcp_read_transformation_summaries, /*
>                       read_optimization_summary */
>                       NULL, /* stmt_fixup */
>                       0, /* function_transform_todo_flags_start */
> Index: src/gcc/ipa-prop.c
> ===================================================================
> --- src.orig/gcc/ipa-prop.c
> +++ src/gcc/ipa-prop.c
> @@ -133,8 +133,8 @@ struct func_body_info
>
>  /* Vector where the parameter infos are actually stored. */
>  vec<ipa_node_params> ipa_node_params_vector;
> -/* Vector of known aggregate values in cloned nodes.  */
> -vec<ipa_agg_replacement_value_p, va_gc> *ipa_node_agg_replacements;
> +/* Vector of IPA-CP transformation data for each clone.  */
> +vec<ipcp_transformation_summary, va_gc> *ipcp_transformations;
>  /* Vector where the parameter infos are actually stored. */
>  vec<ipa_edge_args, va_gc> *ipa_edge_args_vector;
>
> @@ -372,6 +372,15 @@ ipa_print_node_jump_functions_for_edge (
>           fprintf (f, "         Context: ");
>           ctx->dump (dump_file);
>         }
> +
> +      if (jump_func->alignment.known)
> +       {
> +         fprintf (f, "         Alignment: %u, misalignment: %u\n",
> +                  jump_func->alignment.align,
> +                  jump_func->alignment.misalign);
> +       }
> +      else
> +       fprintf (f, "         Unknown alignment\n");
>      }
>  }
>
> @@ -444,6 +453,15 @@ ipa_print_all_jump_functions (FILE *f)
>      }
>  }
>
> +/* Set jfunc to be a know-really nothing jump function.  */
> +
> +static void
> +ipa_set_jf_unknown (struct ipa_jump_func *jfunc)
> +{
> +  jfunc->type = IPA_JF_UNKNOWN;
> +  jfunc->alignment.known = false;
> +}
> +
>  /* Set JFUNC to be a copy of another jmp (to be used by jump function
>     combination code).  The two functions will share their rdesc.  */
>
> @@ -748,7 +766,7 @@ detect_type_change_from_memory_writes (t
>    if (!tci.type_maybe_changed)
>      return false;
>
> -  jfunc->type = IPA_JF_UNKNOWN;
> +  ipa_set_jf_unknown (jfunc);
>    return true;
>  }
>
> @@ -1715,6 +1733,24 @@ ipa_compute_jump_functions_for_edge (str
>             useful_context = true;
>         }
>
> +      if (POINTER_TYPE_P (TREE_TYPE(arg)))
> +       {
> +         unsigned HOST_WIDE_INT hwi_bitpos;
> +         unsigned align;
> +
> +         if (get_pointer_alignment_1 (arg, &align, &hwi_bitpos)
> +             && align > BITS_PER_UNIT)
> +           {
> +             jfunc->alignment.known = true;
> +             jfunc->alignment.align = align;
> +             jfunc->alignment.misalign = hwi_bitpos / BITS_PER_UNIT;
> +           }
> +         else
> +           gcc_assert (!jfunc->alignment.known);
> +       }
> +      else
> +       gcc_assert (!jfunc->alignment.known);
> +
>        if (is_gimple_ip_invariant (arg))
>         ipa_set_jf_constant (jfunc, arg, cs);
>        else if (!is_gimple_reg_type (TREE_TYPE (arg))
> @@ -2412,7 +2448,7 @@ update_jump_functions_after_inlining (st
>              don't.  */
>           if (dst_fid >= ipa_get_cs_argument_count (top))
>             {
> -             dst->type = IPA_JF_UNKNOWN;
> +             ipa_set_jf_unknown (dst);
>               continue;
>             }
>
> @@ -2466,7 +2502,7 @@ update_jump_functions_after_inlining (st
>                 src->value.ancestor.agg_preserved;
>             }
>           else
> -           dst->type = IPA_JF_UNKNOWN;
> +           ipa_set_jf_unknown (dst);
>         }
>        else if (dst->type == IPA_JF_PASS_THROUGH)
>         {
> @@ -2504,7 +2540,7 @@ update_jump_functions_after_inlining (st
>               switch (src->type)
>                 {
>                 case IPA_JF_UNKNOWN:
> -                 dst->type = IPA_JF_UNKNOWN;
> +                 ipa_set_jf_unknown (dst);
>                   break;
>                 case IPA_JF_CONST:
>                   ipa_set_jf_cst_copy (dst, src);
> @@ -2558,7 +2594,7 @@ update_jump_functions_after_inlining (st
>                 }
>             }
>           else
> -           dst->type = IPA_JF_UNKNOWN;
> +           ipa_set_jf_unknown (dst);
>         }
>      }
>  }
> @@ -3329,18 +3365,24 @@ ipa_free_all_node_params (void)
>    ipa_node_params_vector.release ();
>  }
>
> +/* Grow ipcp_transformations if necessary.  */
> +
> +void
> +ipcp_grow_transformations_if_necessary (void)
> +{
> +  if (vec_safe_length (ipcp_transformations)
> +      <= (unsigned) symtab->cgraph_max_uid)
> +    vec_safe_grow_cleared (ipcp_transformations, symtab->cgraph_max_uid + 1);
> +}
> +
>  /* Set the aggregate replacements of NODE to be AGGVALS.  */
>
>  void
>  ipa_set_node_agg_value_chain (struct cgraph_node *node,
>                               struct ipa_agg_replacement_value *aggvals)
>  {
> -  if (vec_safe_length (ipa_node_agg_replacements)
> -      <= (unsigned) symtab->cgraph_max_uid)
> -    vec_safe_grow_cleared (ipa_node_agg_replacements,
> -                          symtab->cgraph_max_uid + 1);
> -
> -  (*ipa_node_agg_replacements)[node->uid] = aggvals;
> +  ipcp_grow_transformations_if_necessary ();
> +  (*ipcp_transformations)[node->uid].agg_values = aggvals;
>  }
>
>  /* Hook that is called by cgraph.c when an edge is removed.  */
> @@ -3381,8 +3423,11 @@ ipa_node_removal_hook (struct cgraph_nod
>    /* During IPA-CP updating we can be called on not-yet analyze clones.  */
>    if (ipa_node_params_vector.length () > (unsigned)node->uid)
>      ipa_free_node_params_substructures (IPA_NODE_REF (node));
> -  if (vec_safe_length (ipa_node_agg_replacements) > (unsigned)node->uid)
> -    (*ipa_node_agg_replacements)[(unsigned)node->uid] = NULL;
> +  if (vec_safe_length (ipcp_transformations) > (unsigned)node->uid)
> +    {
> +      (*ipcp_transformations)[(unsigned)node->uid].agg_values = NULL;
> +      (*ipcp_transformations)[(unsigned)node->uid].alignments = NULL;
> +    }
>  }
>
>  /* Hook that is called by cgraph.c when an edge is duplicated.  */
> @@ -3510,21 +3555,35 @@ ipa_node_duplication_hook (struct cgraph
>    new_info->node_enqueued = old_info->node_enqueued;
>
>    old_av = ipa_get_agg_replacements_for_node (src);
> -  if (!old_av)
> -    return;
> -
> -  new_av = NULL;
> -  while (old_av)
> +  if (old_av)
>      {
> -      struct ipa_agg_replacement_value *v;
> +      new_av = NULL;
> +      while (old_av)
> +       {
> +         struct ipa_agg_replacement_value *v;
>
> -      v = ggc_alloc<ipa_agg_replacement_value> ();
> -      memcpy (v, old_av, sizeof (*v));
> -      v->next = new_av;
> -      new_av = v;
> -      old_av = old_av->next;
> +         v = ggc_alloc<ipa_agg_replacement_value> ();
> +         memcpy (v, old_av, sizeof (*v));
> +         v->next = new_av;
> +         new_av = v;
> +         old_av = old_av->next;
> +       }
> +      ipa_set_node_agg_value_chain (dst, new_av);
> +    }
> +
> +  ipcp_transformation_summary *src_trans = ipcp_get_transformation_summary (src);
> +
> +  if (src_trans && vec_safe_length (src_trans->alignments) > 0)
> +    {
> +      ipcp_grow_transformations_if_necessary ();
> +      src_trans = ipcp_get_transformation_summary (src);
> +      const vec<ipa_alignment, va_gc> *src_alignments = src_trans->alignments;
> +      vec<ipa_alignment, va_gc> *&dst_alignments
> +       = ipcp_get_transformation_summary (dst)->alignments;
> +      vec_safe_reserve_exact (dst_alignments, src_alignments->length ());
> +      for (unsigned i = 0; i < src_alignments->length (); ++i)
> +       dst_alignments->quick_push ((*src_alignments)[i]);
>      }
> -  ipa_set_node_agg_value_chain (dst, new_av);
>  }
>
>
> @@ -4452,6 +4511,15 @@ ipa_write_jump_function (struct output_b
>        streamer_write_uhwi (ob, item->offset);
>        stream_write_tree (ob, item->value, true);
>      }
> +
> +  bp = bitpack_create (ob->main_stream);
> +  bp_pack_value (&bp, jump_func->alignment.known, 1);
> +  streamer_write_bitpack (&bp);
> +  if (jump_func->alignment.known)
> +    {
> +      streamer_write_uhwi (ob, jump_func->alignment.align);
> +      streamer_write_uhwi (ob, jump_func->alignment.misalign);
> +    }
>  }
>
>  /* Read in jump function JUMP_FUNC from IB.  */
> @@ -4470,7 +4538,7 @@ ipa_read_jump_function (struct lto_input
>    switch (jftype)
>      {
>      case IPA_JF_UNKNOWN:
> -      jump_func->type = IPA_JF_UNKNOWN;
> +      ipa_set_jf_unknown (jump_func);
>        break;
>      case IPA_JF_CONST:
>        ipa_set_jf_constant (jump_func, stream_read_tree (ib, data_in), cs);
> @@ -4517,6 +4585,17 @@ ipa_read_jump_function (struct lto_input
>        item.value = stream_read_tree (ib, data_in);
>        jump_func->agg.items->quick_push (item);
>      }
> +
> +  struct bitpack_d bp = streamer_read_bitpack (ib);
> +  bool alignment_known = bp_unpack_value (&bp, 1);
> +  if (alignment_known)
> +    {
> +      jump_func->alignment.known = true;
> +      jump_func->alignment.align = streamer_read_uhwi (ib);
> +      jump_func->alignment.misalign = streamer_read_uhwi (ib);
> +    }
> +  else
> +    jump_func->alignment.known = false;
>  }
>
>  /* Stream out parts of cgraph_indirect_call_info corresponding to CS that are
> @@ -4828,7 +4907,7 @@ ipa_update_after_lto_read (void)
>  }
>
>  void
> -write_agg_replacement_chain (struct output_block *ob, struct cgraph_node *node)
> +write_ipcp_transformation_info (output_block *ob, cgraph_node *node)
>  {
>    int node_ref;
>    unsigned int count = 0;
> @@ -4856,14 +4935,38 @@ write_agg_replacement_chain (struct outp
>        bp_pack_value (&bp, av->by_ref, 1);
>        streamer_write_bitpack (&bp);
>      }
> +
> +  ipcp_transformation_summary *ts = ipcp_get_transformation_summary (node);
> +  if (ts && vec_safe_length (ts->alignments) > 0)
> +    {
> +      count = ts->alignments->length ();
> +
> +      streamer_write_uhwi (ob, count);
> +      for (unsigned i = 0; i < count; ++i)
> +       {
> +         ipa_alignment *parm_al = &(*ts->alignments)[i];
> +
> +         struct bitpack_d bp;
> +         bp = bitpack_create (ob->main_stream);
> +         bp_pack_value (&bp, parm_al->known, 1);
> +         streamer_write_bitpack (&bp);
> +         if (parm_al->known)
> +           {
> +             streamer_write_uhwi (ob, parm_al->align);
> +             streamer_write_hwi_in_range (ob->main_stream, 0, parm_al->align,
> +                                          parm_al->misalign);
> +           }
> +       }
> +    }
> +  else
> +    streamer_write_uhwi (ob, 0);
>  }
>
>  /* Stream in the aggregate value replacement chain for NODE from IB.  */
>
>  static void
> -read_agg_replacement_chain (struct lto_input_block *ib,
> -                           struct cgraph_node *node,
> -                           struct data_in *data_in)
> +read_ipcp_transformation_info (lto_input_block *ib, cgraph_node *node,
> +                              data_in *data_in)
>  {
>    struct ipa_agg_replacement_value *aggvals = NULL;
>    unsigned int count, i;
> @@ -4884,12 +4987,37 @@ read_agg_replacement_chain (struct lto_i
>        aggvals = av;
>      }
>    ipa_set_node_agg_value_chain (node, aggvals);
> +
> +  count = streamer_read_uhwi (ib);
> +  if (count > 0)
> +    {
> +      ipcp_grow_transformations_if_necessary ();
> +
> +      ipcp_transformation_summary *ts = ipcp_get_transformation_summary (node);
> +      vec_safe_grow_cleared (ts->alignments, count);
> +
> +      for (i = 0; i < count; i++)
> +       {
> +         ipa_alignment *parm_al;
> +         parm_al = &(*ts->alignments)[i];
> +         struct bitpack_d bp;
> +         bp = streamer_read_bitpack (ib);
> +         parm_al->known = bp_unpack_value (&bp, 1);
> +         if (parm_al->known)
> +           {
> +             parm_al->align = streamer_read_uhwi (ib);
> +             parm_al->misalign
> +               = streamer_read_hwi_in_range (ib, "ipa-prop misalign",
> +                                             0, parm_al->align);
> +           }
> +       }
> +    }
>  }
>
>  /* Write all aggregate replacement for nodes in set.  */
>
>  void
> -ipa_prop_write_all_agg_replacement (void)
> +ipcp_write_transformation_summaries (void)
>  {
>    struct cgraph_node *node;
>    struct output_block *ob;
> @@ -4897,9 +5025,6 @@ ipa_prop_write_all_agg_replacement (void
>    lto_symtab_encoder_iterator lsei;
>    lto_symtab_encoder_t encoder;
>
> -  if (!ipa_node_agg_replacements)
> -    return;
> -
>    ob = create_output_block (LTO_section_ipcp_transform);
>    encoder = ob->decl_state->symtab_node_encoder;
>    ob->symbol = NULL;
> @@ -4907,8 +5032,7 @@ ipa_prop_write_all_agg_replacement (void
>         lsei_next_function_in_partition (&lsei))
>      {
>        node = lsei_cgraph_node (lsei);
> -      if (node->has_gimple_body_p ()
> -         && ipa_get_agg_replacements_for_node (node) != NULL)
> +      if (node->has_gimple_body_p ())
>         count++;
>      }
>
> @@ -4918,9 +5042,8 @@ ipa_prop_write_all_agg_replacement (void
>         lsei_next_function_in_partition (&lsei))
>      {
>        node = lsei_cgraph_node (lsei);
> -      if (node->has_gimple_body_p ()
> -         && ipa_get_agg_replacements_for_node (node) != NULL)
> -       write_agg_replacement_chain (ob, node);
> +      if (node->has_gimple_body_p ())
> +       write_ipcp_transformation_info (ob, node);
>      }
>    streamer_write_char_stream (ob->main_stream, 0);
>    produce_asm (ob, NULL);
> @@ -4962,7 +5085,7 @@ read_replacements_section (struct lto_fi
>        node = dyn_cast<cgraph_node *> (lto_symtab_encoder_deref (encoder,
>                                                                 index));
>        gcc_assert (node->definition);
> -      read_agg_replacement_chain (&ib_main, node, data_in);
> +      read_ipcp_transformation_info (&ib_main, node, data_in);
>      }
>    lto_free_section_data (file_data, LTO_section_jump_functions, NULL, data,
>                          len);
> @@ -4972,7 +5095,7 @@ read_replacements_section (struct lto_fi
>  /* Read IPA-CP aggregate replacements.  */
>
>  void
> -ipa_prop_read_all_agg_replacement (void)
> +ipcp_read_transformation_summaries (void)
>  {
>    struct lto_file_decl_data **file_data_vec = lto_get_file_decl_data ();
>    struct lto_file_decl_data *file_data;
> @@ -5139,6 +5262,58 @@ ipcp_modif_dom_walker::before_dom_childr
>
>  }
>
> +/* Update alignment of formal parameters as described in
> +   ipcp_transformation_summary.  */
> +
> +static void
> +ipcp_update_alignments (struct cgraph_node *node)
> +{
> +  tree fndecl = node->decl;
> +  tree parm = DECL_ARGUMENTS (fndecl);
> +  tree next_parm = parm;
> +  ipcp_transformation_summary *ts = ipcp_get_transformation_summary (node);
> +  if (!ts || vec_safe_length (ts->alignments) == 0)
> +    return;
> +  const vec<ipa_alignment, va_gc> &alignments = *ts->alignments;
> +  unsigned count = alignments.length ();
> +
> +  for (unsigned i = 0; i < count; ++i, parm = next_parm)
> +    {
> +      if (node->clone.combined_args_to_skip
> +         && bitmap_bit_p (node->clone.combined_args_to_skip, i))
> +       continue;
> +      gcc_checking_assert (parm);
> +      next_parm = DECL_CHAIN (parm);
> +
> +      if (!alignments[i].known || !is_gimple_reg (parm))
> +       continue;
> +      tree ddef = ssa_default_def (DECL_STRUCT_FUNCTION (node->decl), parm);
> +      if (!ddef)
> +       continue;
> +
> +      if (dump_file)
> +       fprintf (dump_file, "  Adjusting alignment of param %u to %u, "
> +                "misalignment to %u\n", i, alignments[i].align,
> +                alignments[i].misalign);
> +
> +      struct ptr_info_def *pi = get_ptr_info (ddef);
> +      gcc_checking_assert (pi);
> +      unsigned old_align;
> +      unsigned old_misalign;
> +      bool old_known = get_ptr_info_alignment (pi, &old_align, &old_misalign);
> +
> +      if (old_known
> +         && old_align >= alignments[i].align)
> +       {
> +         if (dump_file)
> +           fprintf (dump_file, "    But the alignment has already "
> +                    "been %u.\n", old_align);

Suggest "was already" instead of "has already been".

> +         continue;
> +       }
> +      set_ptr_info_alignment (pi, alignments[i].align, alignments[i].misalign);
> +    }
> +}
> +
>  /* IPCP transformation phase doing propagation of aggregate values.  */
>
>  unsigned int
> @@ -5157,6 +5332,7 @@ ipcp_transform_function (struct cgraph_n
>      fprintf (dump_file, "Modification phase of node %s/%i\n",
>              node->name (), node->order);
>
> +  ipcp_update_alignments (node);
>    aggval = ipa_get_agg_replacements_for_node (node);
>    if (!aggval)
>        return 0;
> @@ -5186,7 +5362,8 @@ ipcp_transform_function (struct cgraph_n
>      free_ipa_bb_info (bi);
>    fbi.bb_infos.release ();
>    free_dominance_info (CDI_DOMINATORS);
> -  (*ipa_node_agg_replacements)[node->uid] = NULL;
> +  (*ipcp_transformations)[node->uid].agg_values = NULL;
> +  (*ipcp_transformations)[node->uid].alignments = NULL;
>    descriptors.release ();
>
>    if (!something_changed)
> Index: src/gcc/ipa-prop.h
> ===================================================================
> --- src.orig/gcc/ipa-prop.h
> +++ src/gcc/ipa-prop.h
> @@ -144,6 +144,17 @@ struct GTY(()) ipa_agg_jump_function
>
>  typedef struct ipa_agg_jump_function *ipa_agg_jump_function_p;
>
> +/* Info about poiner alignments. */

"pointer"

> +struct GTY(()) ipa_alignment
> +{
> +  /* The data fields below are valid only if known is true.  */
> +  bool known;

Just curious: why is the "known" flag necessary? The comments for
ptr_info_def say that align=0 means unknown.

> +  /* See ptr_info_def and get_pointer_alignment_1 for description of these
> +     two.  */
> +  unsigned align;
> +  unsigned misalign;
> +};
> +
>  /* A jump function for a callsite represents the values passed as actual
>     arguments of the callsite. See enum jump_func_type for the various
>     types of jump functions supported.  */
> @@ -153,6 +164,9 @@ struct GTY (()) ipa_jump_func
>       description.  */
>    struct ipa_agg_jump_function agg;
>
> +  /* Information about alignment of pointers. */
> +  struct ipa_alignment alignment;
> +
>    enum jump_func_type type;
>    /* Represents a value of a jump function.  pass_through is used only in jump
>       function context.  constant represents the actual constant in constant jump
> @@ -402,10 +416,19 @@ struct GTY(()) ipa_agg_replacement_value
>    bool by_ref;
>  };
>
> -typedef struct ipa_agg_replacement_value *ipa_agg_replacement_value_p;
> +/* Structure holding information for the transformation phase of IPA-CP.  */
> +
> +struct GTY(()) ipcp_transformation_summary
> +{
> +  /* Linked list of known aggregate values.  */
> +  ipa_agg_replacement_value *agg_values;
> +  /* Alignemnt information for pointers.  */

"Alignment"

> +  vec<ipa_alignment, va_gc> *alignments;
> +};
>
>  void ipa_set_node_agg_value_chain (struct cgraph_node *node,
>                                    struct ipa_agg_replacement_value *aggvals);
> +void ipcp_grow_transformations_if_necessary (void);
>
>  /* ipa_edge_args stores information related to a callsite and particularly its
>     arguments.  It can be accessed by the IPA_EDGE_REF macro.  */
> @@ -451,8 +474,8 @@ ipa_get_ith_polymorhic_call_context (str
>
>  /* Vector where the parameter infos are actually stored. */
>  extern vec<ipa_node_params> ipa_node_params_vector;
> -/* Vector of known aggregate values in cloned nodes.  */
> -extern GTY(()) vec<ipa_agg_replacement_value_p, va_gc> *ipa_node_agg_replacements;
> +/* Vector of IPA-CP transformation data for each clone.  */
> +extern GTY(()) vec<ipcp_transformation_summary, va_gc> *ipcp_transformations;
>  /* Vector where the parameter infos are actually stored. */
>  extern GTY(()) vec<ipa_edge_args, va_gc> *ipa_edge_args_vector;
>
> @@ -510,14 +533,21 @@ ipa_edge_args_info_available_for_edge_p
>    return ((unsigned) edge->uid < vec_safe_length (ipa_edge_args_vector));
>  }
>
> +static inline ipcp_transformation_summary *
> +ipcp_get_transformation_summary (cgraph_node *node)
> +{
> +  if ((unsigned) node->uid >= vec_safe_length (ipcp_transformations))
> +    return NULL;
> +  return &(*ipcp_transformations)[node->uid];
> +}
> +
>  /* Return the aggregate replacements for NODE, if there are any.  */
>
>  static inline struct ipa_agg_replacement_value *
> -ipa_get_agg_replacements_for_node (struct cgraph_node *node)
> +ipa_get_agg_replacements_for_node (cgraph_node *node)
>  {
> -  if ((unsigned) node->uid >= vec_safe_length (ipa_node_agg_replacements))
> -    return NULL;
> -  return (*ipa_node_agg_replacements)[node->uid];
> +  ipcp_transformation_summary *ts = ipcp_get_transformation_summary (node);
> +  return ts ? ts->agg_values : NULL;
>  }
>
>  /* Function formal parameters related computations.  */
> @@ -646,8 +676,8 @@ void ipa_dump_agg_replacement_values (FI
>                                       struct ipa_agg_replacement_value *av);
>  void ipa_prop_write_jump_functions (void);
>  void ipa_prop_read_jump_functions (void);
> -void ipa_prop_write_all_agg_replacement (void);
> -void ipa_prop_read_all_agg_replacement (void);
> +void ipcp_write_transformation_summaries (void);
> +void ipcp_read_transformation_summaries (void);
>  void ipa_update_after_lto_read (void);
>  int ipa_get_param_decl_index (struct ipa_node_params *, tree);
>  tree ipa_value_from_jfunc (struct ipa_node_params *info,
> Index: src/gcc/testsuite/gcc.dg/ipa/propalign-1.c
> ===================================================================
> --- /dev/null
> +++ src/gcc/testsuite/gcc.dg/ipa/propalign-1.c
> @@ -0,0 +1,32 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -fno-early-inlining -fdump-ipa-cp -fdump-tree-optimized" } */
> +
> +#include <stdint.h>
> +
> +extern int fail_the_test(void *);
> +extern int pass_the_test(void *);
> +extern int diversion (void *);
> +
> +static int __attribute__((noinline))
> +foo (void *p)
> +{
> +  uintptr_t a = (uintptr_t) p;
> +
> +  if (a % 4)
> +    return fail_the_test (p);
> +  else
> +    return pass_the_test (p);
> +}
> +
> +int
> +bar (void)
> +{
> +  double buf[8] __attribute__ ((__aligned__(__BIGGEST_ALIGNMENT__)));
> +  return foo (&buf);
> +}
> +
> +
> +/* { dg-final { scan-ipa-dump "Adjusting alignment of param" "cp" } } */
> +/* { dg-final { scan-tree-dump-not "fail_the_test" "optimized" } } */
> +/* { dg-final { cleanup-ipa-dump "cp" } } */
> +/* { dg-final { cleanup-tree-dump "optimized" } } */
> Index: src/gcc/testsuite/gcc.dg/ipa/propalign-2.c
> ===================================================================
> --- /dev/null
> +++ src/gcc/testsuite/gcc.dg/ipa/propalign-2.c
> @@ -0,0 +1,58 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -fno-early-inlining -fdump-ipa-cp -fdump-tree-optimized" } */
> +
> +#include <stdint.h>
> +
> +extern int fail_the_test(void *);
> +extern int pass_the_test(void *);
> +extern int diversion (void *);
> +
> +struct somestruct
> +{
> +  void *whee;
> +  void *oops;
> +};
> +
> +struct container
> +{
> +  struct somestruct first;
> +  struct somestruct buf[32];
> +};
> +
> +static int __attribute__((noinline))
> +foo (void *p)
> +{
> +  uintptr_t a = (uintptr_t) p;
> +
> +  if (a % 4)
> +    return fail_the_test (p);
> +  else
> +    return pass_the_test (p);
> +}
> +
> +int
> +bar (void)
> +{
> +  struct container c;
> +  return foo (c.buf);
> +}
> +
> +
> +static int
> +through (struct somestruct *p)
> +{
> +  diversion (p);
> +  return foo (&p[16]);
> +}
> +
> +int
> +bar2 (void)
> +{
> +  struct container c;
> +  through (c.buf);
> +}
> +
> +/* { dg-final { scan-ipa-dump "Adjusting alignment of param" "cp" } } */
> +/* { dg-final { scan-tree-dump-not "fail_the_test" "optimized" } } */
> +/* { dg-final { cleanup-ipa-dump "cp" } } */
> +/* { dg-final { cleanup-tree-dump "optimized" } } */

Hi,

thank you very much for pointing out all the spelling mistakes.  I
suppose you could tell I was writing this in a hurry :-)  Nevertheless
I should probably make a habit of spell checking stuff before
sending it out.

As far as  your question is concerned:

On Wed, Dec 03, 2014 at 10:53:54AM +0000, Jay Foad wrote:
> > Index: src/gcc/ipa-prop.h
> > ===================================================================
> > --- src.orig/gcc/ipa-prop.h
> > +++ src/gcc/ipa-prop.h
> > @@ -144,6 +144,17 @@ struct GTY(()) ipa_agg_jump_function
> >
> >  typedef struct ipa_agg_jump_function *ipa_agg_jump_function_p;
> >
> > +/* Info about poiner alignments. */
> 
> "pointer"
> 
> > +struct GTY(()) ipa_alignment
> > +{
> > +  /* The data fields below are valid only if known is true.  */
> > +  bool known;
> 
> Just curious: why is the "known" flag necessary? The comments for
> ptr_info_def say that align=0 means unknown.

It is necessary.  In IPA-CP, when know is false, this means the
lattice is in TOP state (i.e. once we learn something about the
parameter, let's overwrite this), whereas when it is true and
alignment is 0, it means it is in BOTTOM state (i.e. we know we cannot
rely on this and never will be able to).

Thanks,

Martin

On 3 December 2014 at 14:36, Martin Jambor <mjambor@suse.cz> wrote:
> On Wed, Dec 03, 2014 at 10:53:54AM +0000, Jay Foad wrote:
>> > Index: src/gcc/ipa-prop.h
>> > ===================================================================
>> > --- src.orig/gcc/ipa-prop.h
>> > +++ src/gcc/ipa-prop.h
>> > @@ -144,6 +144,17 @@ struct GTY(()) ipa_agg_jump_function
>> >
>> >  typedef struct ipa_agg_jump_function *ipa_agg_jump_function_p;
>> >
>> > +/* Info about poiner alignments. */
>>
>> "pointer"
>>
>> > +struct GTY(()) ipa_alignment
>> > +{
>> > +  /* The data fields below are valid only if known is true.  */
>> > +  bool known;
>>
>> Just curious: why is the "known" flag necessary? The comments for
>> ptr_info_def say that align=0 means unknown.
>
> It is necessary.  In IPA-CP, when know is false, this means the
> lattice is in TOP state (i.e. once we learn something about the
> parameter, let's overwrite this), whereas when it is true and
> alignment is 0, it means it is in BOTTOM state (i.e. we know we cannot
> rely on this and never will be able to).

Can't you use align=1, misalign=0 for TOP ? This means that we don't
know anything useful about the pointer yet, just that it's a multiple
of 1 (which is trivially true for all pointers, isn't it?).

When you have vectors of these struct they will pack MUCH more nicely
without the "bool known" field.

Thanks,
Jay.