[PATCH 0.5/2] ipa-sra: Restructure how cloning and call redirection communicate (PR 93385)

[Jan Hubicka]

> 
> I was asked by Richi to split my fix for PR 93385 for easier review
> into IPA-SRA materialization refactoring and the actual DCE addition.
> Fortunately it was mostly natural except for a temporary weird
> condition in ipa_param_body_adjustments::modify_call_stmt.
> Additionally.  In addition to the patch I posted previously, this one
> also deallocated the newly added summary in toplev::finalize and fixes
> a mistakenly uninitialized field.
> 
> This is the first part which basically replaces performed_splits in
> clone_info and the code which generates it, keeps it up-to-date and
> consumes it with new edge summaries which are much nicer.  It simply
> contains 1) a mapping from the original argument indices to the actual
> indices in the call statement as it is now, 2) information needed to
> identify arguments representing pass-through IPA-SRA splits with which
> have been added to the call arguments in place of an original
> argument/reference and 3) a delta to the index where va_args may start
> - so basically directly all the information that the consumer of
> performed_splits had to compute and we also do not need the weird
> dummy declarations.
> 
> The main disadvantage is that the information has to be created (and
> kept up-to-date) for all call graph edges associated with the given
> statement from all clones (including inline clones) of the clone where
> splitting or removal happened first.  But all of this happens during
> clone materialization so the only effect on WPA memory consumption is
> the removal of a pointer from clone_info.
> 
> The statement modification code also has to know the statement from
> the original function in order to be able to locate the edge summaries
> which at this point are still keyed to these.  However, the code is
> already quite heavily dependant on how things are structured in
> tree-inline.c and in order to fix bugs like these it probably has to
> be.
> 
> The subsequent patch needs this new information to be able to remove
> arguments from calls during materialization and communicate this
> information to the call redirection.
> 
> 2021-05-17  Martin Jambor  <mjambor@suse.cz>
> 
> 	PR ipa/93385
> 	* symtab-clones.h (clone_info): Removed member param_adjustments.
> 	* ipa-param-manipulation.h: Adjust initial comment to reflect how we
> 	deal with pass-through splits now.
> 	(ipa_param_performed_split): Removed.
> 	(ipa_param_adjustments::modify_call): Adjusted parameters.
> 	(class ipa_param_body_adjustments): Adjusted parameters of
> 	register_replacement, modify_gimple_stmt and modify_call_stmt.
> 	(ipa_verify_edge_has_no_modifications): Declare.
> 	(ipa_edge_modifications_finalize): Declare.
> 	* cgraph.c (cgraph_edge::redirect_call_stmt_to_callee): Remove
> 	performed_splits processing, pas only edge to padjs->modify_call,
> 	check that call arguments were not modified if they should not have
> 	been.
> 	* cgraphclones.c (cgraph_node::create_clone): Do not copy performed
> 	splits.
> 	* ipa-param-manipulation.c (struct pass_through_split_map): New type.
> 	(ipa_edge_modification_info): Likewise.
> 	(ipa_edge_modification_sum): Likewise.
> 	(ipa_edge_modifications): New edge summary.
> 	(ipa_verify_edge_has_no_modifications): New function.
> 	(transitive_split_p): Removed.
> 	(transitive_split_map): Likewise.
> 	(init_transitive_splits): Likewise.
> 	(ipa_param_adjustments::modify_call): Adjusted to use the new edge
> 	summary instead of performed_splits.
> 	(ipa_param_body_adjustments::register_replacement): Drop dummy
> 	parameter, set base_index of the created ipa_param_body_replacement.
> 	(phi_arg_will_live_p): New function.
> 	(ipa_param_body_adjustments::common_initialization): Do not create
> 	IPA_SRA dummy decls.
> 	(simple_tree_swap_info): Removed.
> 	(remap_split_decl_to_dummy): Likewise.
> 	(record_argument_state_1): New function.
> 	(record_argument_state): Likewise.
> 	(ipa_param_body_adjustments::modify_call_stmt): New parameter
> 	orig_stmt.  Do not work with dummy decls, save necessary info about
> 	changes to ipa_edge_modifications.
> 	(ipa_param_body_adjustments::modify_gimple_stmt): New parameter
> 	orig_stmt, pass it to modify_call_stmt.
> 	(ipa_param_body_adjustments::modify_cfun_body): Adjust call to
> 	modify_gimple_stmt.
> 	(ipa_edge_modifications_finalize): New function.
> 	* tree-inline.c (remap_gimple_stmt): Pass original statement to
> 	modify_gimple_stmt.
> 	(copy_phis_for_bb): Do not copy dead PHI nodes.
> 	(expand_call_inline): Do not remap performed_splits.
> 	(update_clone_info): Likewise.
> 	* toplev.c: Include ipa-param-manipulation.h.
> 	(toplev::finalize): Call ipa_edge_modifications_finalize.
> ---
>  gcc/cgraph.c                 |  22 +-
>  gcc/cgraphclones.c           |   3 -
>  gcc/ipa-param-manipulation.c | 803 +++++++++++++++++++----------------
>  gcc/ipa-param-manipulation.h |  82 ++--
>  gcc/symtab-clones.h          |  15 +-
>  gcc/toplev.c                 |   2 +
>  gcc/tree-inline.c            | 103 +----
>  7 files changed, 485 insertions(+), 545 deletions(-)
> 
> diff --git a/gcc/cgraph.c b/gcc/cgraph.c
> index d7c78d518bc..d473da5a325 100644
> --- a/gcc/cgraph.c
> +++ b/gcc/cgraph.c
> @@ -1506,8 +1506,6 @@ cgraph_edge::redirect_call_stmt_to_callee (cgraph_edge *e)
>      }
>  
>    clone_info *callee_info = clone_info::get (e->callee);
> -  clone_info *caller_info = clone_info::get (e->caller);
> -
>    if (symtab->dump_file)
>      {
>        fprintf (symtab->dump_file, "updating call of %s -> %s: ",
> @@ -1515,18 +1513,6 @@ cgraph_edge::redirect_call_stmt_to_callee (cgraph_edge *e)
>        print_gimple_stmt (symtab->dump_file, e->call_stmt, 0, dump_flags);
>        if (callee_info && callee_info->param_adjustments)
>  	callee_info->param_adjustments->dump (symtab->dump_file);
> -      unsigned performed_len
> -	= caller_info ? vec_safe_length (caller_info->performed_splits) : 0;
> -      if (performed_len > 0)
> -	fprintf (symtab->dump_file, "Performed splits records:\n");
> -      for (unsigned i = 0; i < performed_len; i++)
> -	{
> -	  ipa_param_performed_split *sm
> -	    = &(*caller_info->performed_splits)[i];
> -	  print_node_brief (symtab->dump_file, "  dummy_decl: ", sm->dummy_decl,
> -			    TDF_UID);
> -	  fprintf (symtab->dump_file, ", unit_offset: %u\n", sm->unit_offset);
> -	}
>      }
>  
>    if (ipa_param_adjustments *padjs
> @@ -1541,10 +1527,7 @@ cgraph_edge::redirect_call_stmt_to_callee (cgraph_edge *e)
>  	remove_stmt_from_eh_lp (e->call_stmt);
>  
>        tree old_fntype = gimple_call_fntype (e->call_stmt);
> -      new_stmt = padjs->modify_call (e->call_stmt,
> -				     caller_info
> -				     ? caller_info->performed_splits : NULL,
> -				     e->callee->decl, false);
> +      new_stmt = padjs->modify_call (e, false);
>        cgraph_node *origin = e->callee;
>        while (origin->clone_of)
>  	origin = origin->clone_of;
> @@ -1564,6 +1547,9 @@ cgraph_edge::redirect_call_stmt_to_callee (cgraph_edge *e)
>      }
>    else
>      {
> +      if (flag_checking
> +	  && !fndecl_built_in_p (e->callee->decl, BUILT_IN_UNREACHABLE))
> +	ipa_verify_edge_has_no_modifications (e);
>        new_stmt = e->call_stmt;
>        gimple_call_set_fndecl (new_stmt, e->callee->decl);
>        update_stmt_fn (DECL_STRUCT_FUNCTION (e->caller->decl), new_stmt);
> diff --git a/gcc/cgraphclones.c b/gcc/cgraphclones.c
> index 9f86463b42d..7e463acab91 100644
> --- a/gcc/cgraphclones.c
> +++ b/gcc/cgraphclones.c
> @@ -414,9 +414,6 @@ cgraph_node::create_clone (tree new_decl, profile_count prof_count,
>    else if (info && info->param_adjustments)
>      clone_info::get_create (new_node)->param_adjustments
>  	 = info->param_adjustments;
> -  if (info && info->performed_splits)
> -    clone_info::get_create (new_node)->performed_splits
> -	 = vec_safe_copy (info->performed_splits);
>    new_node->split_part = split_part;
>  
>    FOR_EACH_VEC_ELT (redirect_callers, i, e)
> diff --git a/gcc/ipa-param-manipulation.c b/gcc/ipa-param-manipulation.c
> index f2d91476655..6a423391d2f 100644
> --- a/gcc/ipa-param-manipulation.c
> +++ b/gcc/ipa-param-manipulation.c
> @@ -62,6 +62,80 @@ static const char *ipa_param_op_names[IPA_PARAM_PREFIX_COUNT]
>       "IPA_PARAM_OP_NEW",
>       "IPA_PARAM_OP_SPLIT"};
>  
> +/* Structure to hold declarations representing pass-through IPA-SRA splits.  In
> +   essence, it tells new index for a combination of original index and
> +   offset.  */
> +
> +struct pass_through_split_map
> +{
> +  /* Original argument index.  */
> +  unsigned base_index;
> +  /* Offset of the split part in the original argument.  */
> +  unsigned unit_offset;

I wonder, most of ipa-prop and param handling seems to be using integers
for offsets.  While people usually do not pass more than 2gb on stack,
shouldn't we convert them eventually to specialised types we have?
> +  /* Index of the split part in the call statement - where clone
> +     materialization put it.  */
> +  int new_index;
> +};
> +
> +/* Information about some call statements that needs to be conveyed from clone
> +   materialization to edge redirection. */
> +
> +class ipa_edge_modification_info
> +{
> + public:
> +  ipa_edge_modification_info ()
> +    {}
> +
> +  /* Mapping of original argument indices to where those arguments sit in the
> +     call statement now or to a negative index if they were removed.  */
> +  auto_vec<int> index_map;
> +  /* Information about ISRA replacements put into the call statement at the
> +     clone materialization stages.  */
> +  auto_vec<pass_through_split_map> pass_through_map;
> +  /* Necessary adjustment to ipa_param_adjustments::m_always_copy_start when
> +     redirecting the call.  */
> +  int always_copy_delta = 0;
> +};
> +
> +/* Class for storing and retrieving summaries about cal statement
> +   modifications.  */
> +
> +class ipa_edge_modification_sum
> +  : public call_summary <ipa_edge_modification_info *>
> +{
> + public:
> +  ipa_edge_modification_sum (symbol_table *table)
> +    : call_summary<ipa_edge_modification_info *> (table)
> +  {
> +  }
> +
> +  /* Hook that is called by summary when an edge is duplicated.  */
> +
> +  virtual void duplicate (cgraph_edge *,
> +			  cgraph_edge *,
> +			  ipa_edge_modification_info *old_info,
> +			  ipa_edge_modification_info *new_info)
> +  {
> +    new_info->index_map.safe_splice (old_info->index_map);
> +    new_info->pass_through_map.safe_splice (old_info->pass_through_map);
> +    new_info->always_copy_delta = old_info->always_copy_delta;
> +  }
> +};
> +
> +/* Call summary to store information about edges which have had their arguments
> +   partially modified already.  */
> +
> +static ipa_edge_modification_sum *ipa_edge_modifications;
> +
> +/* Fail compilation if CS has any summary associated with it in
> +   ipa_edge_modifications.  */
> +
> +DEBUG_FUNCTION void
> +ipa_verify_edge_has_no_modifications (cgraph_edge *cs)
> +{
> +  gcc_assert (!ipa_edge_modifications || !ipa_edge_modifications->get (cs));
> +}
> +

Patch is OK, sorry for taking so long to get to it.
Honza