Re: [PATCH 2/4] New parameter manipulation infrastructure

[Martin Jambor <mjambor at suse dot cz>]

Hi,

On Fri, Sep 13 2019, Jan Hubicka wrote:
>> 2019-08-20  Martin Jambor  <mjambor@suse.cz>
>> 
>>         * Makefile.in (GTFILES): Added ipa-param-manipulation.h.
>>         * cgraph.h (ipa_replace_map): Removed fields old_tree, replace_p
>>         and ref_p, added fields param_adjustments and performed_splits.
>>         (struct cgraph_clone_info): Remove ags_to_skip and
>>         combined_args_to_skip, new field param_adjustments.
>>         (cgraph_node::create_clone): Changed parameters to use
>>         ipa_param_adjustments.
>>         (cgraph_node::create_virtual_clone): Likewise.
>>         (cgraph_node::create_virtual_clone_with_body): Likewise.
>>         (tree_function_versioning): Likewise.
>>         (cgraph_build_function_type_skip_args): Removed.
>>         * cgraph.c (cgraph_edge::redirect_call_stmt_to_callee): Convert to
>>         using ipa_param_adjustments.
>>         (clone_of_p): Likewise.
>>         * cgraphclones.c (cgraph_build_function_type_skip_args): Removed.
>>         (build_function_decl_skip_args): Likewise.
>>         (duplicate_thunk_for_node): Adjust parameters using
>>         ipa_param_body_adjustments, copy param_adjustments instead of
>>         args_to_skip.
>>         (cgraph_node::create_clone): Convert to using ipa_param_adjustments.
>>         (cgraph_node::create_virtual_clone): Likewise.
>>         (cgraph_node::create_version_clone_with_body): Likewise.
>>         (cgraph_materialize_clone): Likewise.
>>         (symbol_table::materialize_all_clones): Likewise.
>>         * coretypes.h (cgraph_edge): Declare.
>>         * ipa-cp.c (get_replacement_map): Do not initialize removed fields.
>>         (initialize_node_lattices): Make aware that some parameters might have
>>         already been removed.
>>         (want_remove_some_param_p): New function.
>>         (create_specialized_node): Convert to using ipa_param_adjustments and
>>         deal with possibly pre-existing adjustments.
>>         * ipa-fnsummary.c (ipa_fn_summary_t::duplicate): Simplify
>>         ipa_replace_map check.
>>         * ipa-inline-transform.c (save_inline_function_body): Update to
>>         refelct new tree_function_versioning signature.
>>         * ipa-param-manipulation.c: Rewrite.
>>         * ipa-param-manipulation.h: Likewise.
>>         * ipa-prop.c (adjust_agg_replacement_values): Use a helper from
>>         ipa_param_adjustments to get current parameter indices.
>>         (ipcp_modif_dom_walker::before_dom_children): Likewise.
>>         (ipcp_update_bits): Likewise.
>>         (ipcp_update_vr): Likewise.
>>         * ipa-split.c (split_function): Convert to using ipa_param_adjustments.
>>         * lto-cgraph.c (output_cgraph_opt_summary_p): Likewise.
>>         (output_node_opt_summary): Do not stream removed fields.  Stream
>>         parameter adjustments instead of argumetns to skip.
>>         (input_node_opt_summary): Likewise.
>>         (input_node_opt_summary): Likewise.
>>         * multiple_target.c (create_target_clone): Update to reflet new type
>>         of create_version_clone_with_body.
>>         * omp-simd-clone.c (simd_clone_vector_of_formal_parm_types): Adjust
>>         for the new interface.
>>         (simd_clone_clauses_extract): Likewise, make args an auto_vec.
>>         (simd_clone_compute_base_data_type): Likewise.
>>         (simd_clone_init_simd_arrays): Adjust for the new interface.
>>         (simd_clone_adjust_argument_types): Likewise.
>>         (struct modify_stmt_info): Likewise.
>>         (ipa_simd_modify_stmt_ops): Likewise.
>>         (ipa_simd_modify_function_body): Likewise.
>>         (simd_clone_adjust): Likewise.
>>         * trans-mem.c (ipa_tm_create_version): Update to reflect new type of
>>         tree_function_versioning.
>>         * tree-inline.h (copy_body_data): New fields killed_new_ssa_names and
>>         param_body_adjs.
>>         (copy_decl_to_var): Declare.
>>         * tree-inline.c (update_clone_info): Do not remap old_tree.
>>         (remap_gimple_stmt): Use ipa_param_body_adjustments to modify gimple
>>         statements, walk all extra generated statements and remap their
>>         operands.
>>         (redirect_all_calls): Add killed SSA names to a hash set.
>>         (remap_ssa_name): Do not remap killed SSA names.
>>         (copy_arguments_for_versioning): Renames to copy_arguments_nochange,
>>         half of functionality moved to ipa_param_body_adjustments.
>>         (copy_decl_to_var): Make exported.
>>         (copy_body): Destroy killed_new_ssa_names hash set.
>>         (expand_call_inline): Remap performed splits.
>>         (update_clone_info): Likewise.
>>         (tree_function_versioning): Simplify tree_map processing.  Updated to
>>         accept ipa_param_adjustments and use ipa_param_body_adjustments.
>
> OK

Thanks!

>> +/* Modify actual arguments of a function call in statement STMT, assuming it
>> +   calls CALLEE_DECL.  CALLER_ADJ must be the description of parameter
>> +   adjustments of the caller or NULL if there are none.  Return the new
>> +   statement that replaced the old one.  When invoked, cfun and
>> +   current_function_decl have to be set to the caller.  */
>> +
>> +gcall *
>> +ipa_param_adjustments::modify_call (gcall *stmt,
>> +				    vec<ipa_param_performed_split,
>> +				        va_gc> *performed_splits,
>> +				    tree callee_decl, bool update_references)
>> +{
>> +  unsigned len = vec_safe_length (m_adj_params);
>> +  auto_vec<tree, 16> vargs (len);
>> +  tree old_decl = gimple_call_fndecl (stmt);
>> +  unsigned old_nargs = gimple_call_num_args (stmt);
>> +  auto_vec<bool, 16> kept (old_nargs);
>> +  kept.quick_grow_cleared (old_nargs);
>> +
>> +  auto_vec <unsigned, 16> index_map;
>> +  auto_vec <transitive_split_map> trans_map;
>> +  bool transitive_remapping = false;
> vertical space here and move the quick_grow_cleared here too I guess.
> It seems that you omit vertical space after declaration at many places.

I'm afraid I do.  Especially if they are immediately initialized, I
don't view them as a "declaration block" any more.  I at least added a
line break here.

...

>
> I would probably also include one line comments for public member
> functions for new classes you introduce so one has most of essential
> info at one place.

OK, I have added the following comments to the class definitions (real
verbose function comments are then just before the method body, which I
believe is required by our coding conventions).

I'm in the last stages of testing a re-base of the patches and will
commit them shortly.

Thanks,

Martin


class GTY(()) ipa_param_adjustments
{
public:
  /* Constructor from NEW_PARAMS showing how new parameters should look like
      plus copying any pre-existing actual arguments starting from argument
      with index ALWAYS_COPY_START (if non-negative, negative means do not copy
      anything beyond what is described in NEW_PARAMS), and SKIP_RETURN, which
      indicates that the function should return void after transformation.  */

  ipa_param_adjustments (vec<ipa_adjusted_param, va_gc> *new_params,
			 int always_copy_start, bool skip_return)
    : m_adj_params (new_params), m_always_copy_start (always_copy_start),
    m_skip_return (skip_return)
    {}

  /* Modify a call statement arguments (and possibly remove the return value)
     as described in the data fields of this class.  */
  gcall *modify_call (gcall *stmt,
		      vec<ipa_param_performed_split, va_gc> *performed_splits,
		      tree callee_decl, bool update_references);
  /* Return if the first parameter is left intact.  */
  bool first_param_intact_p ();
  /* Build a function type corresponding to the modified call.  */
  tree build_new_function_type (tree old_type, bool type_is_original_p);
  /* Build a declaration corresponding to the target of the modified call.  */
  tree adjust_decl (tree orig_decl);
  /* Fill a vector marking which parameters are intact by the described
     modifications. */
  void get_surviving_params (vec<bool> *surviving_params);
  /* Fill a vector with new indices of surviving original parameters.  */
  void get_updated_indices (vec<int> *new_indices);

  void dump (FILE *f);
  void debug ();

  /* How the known part of arguments should look like.  */
  vec<ipa_adjusted_param, va_gc> *m_adj_params;

  /* If non-negative, copy any arguments starting at this offset without any
     modifications so that functions with variable number of arguments can be
     modified. This number should be equal to the number of original forma
     parameters.  */
  int m_always_copy_start;
  /* If true, make the function not return any value.  */
  bool m_skip_return;

private:
  ipa_param_adjustments () {}

  void init (vec<tree> *cur_params);
  int get_max_base_index ();
  bool method2func_p (tree orig_type);
};

/* ....clipped....  */


class ipa_param_body_adjustments
{
public:
  /* Constructor to use from within tree-inline.  */
  ipa_param_body_adjustments (ipa_param_adjustments *adjustments,
			      tree fndecl, tree old_fndecl,
			      struct copy_body_data *id, tree *vars,
			      vec<ipa_replace_map *, va_gc> *tree_map);
  /* Constructor to use for modifying a function outside of tree-inline from an
     instance of ipa_param_adjustments.  */
  ipa_param_body_adjustments (ipa_param_adjustments *adjustments,
			      tree fndecl);
  /* Constructor to use for modifying a function outside of tree-inline from a
     simple vector of desired parameter modification.  */
  ipa_param_body_adjustments (vec<ipa_adjusted_param, va_gc> *adj_params,
			      tree fndecl);

  /* The do-it-all function for modifying a function outside of
     tree-inline.  */
  bool perform_cfun_body_modifications ();

  /* Change the PARM_DECLs.  */
  void modify_formal_parameters ();
  /* Register a replacement decl for the transformation done in APM.  */
  void register_replacement (ipa_adjusted_param *apm, tree replacement,
			     tree dummy = NULL_TREE);
  /* Lookup a replacement for a given offset within a given parameter.  */
  tree lookup_replacement (tree base, unsigned unit_offset);
  /* Lookup a replacement for an expression, if there is one.  */
  ipa_param_body_replacement *get_expr_replacement (tree expr,
						    bool ignore_default_def);
  /* Lookup the new base for surviving names previously belonging to a
     parameter. */
  tree get_replacement_ssa_base (tree old_decl);
  /* Modify a statement.  */
  bool modify_gimple_stmt (gimple **stmt, gimple_seq *extra_stmts);
  /* Return the new chain of parameters.  */
  tree get_new_param_chain ();

  /* Pointers to data structures defining how the function should be
     modified.  */
  vec<ipa_adjusted_param, va_gc> *m_adj_params;
  ipa_param_adjustments *m_adjustments;

  /* Vector of old parameter declarations that must have their debug bind
     statements re-mapped and debug decls created.  */

  auto_vec<tree, 16> m_reset_debug_decls;

  /* Set to true if there are any IPA_PARAM_OP_SPLIT adjustments among stored
     adjustments.  */
  bool m_split_modifications_p;
private:

/* ....clipped....  */