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

[Jan Hubicka <hubicka at ucw dot cz>]

> 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
> +/* 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.
> diff --git a/gcc/ipa-param-manipulation.h b/gcc/ipa-param-manipulation.h
> index 71fc4a201aa..b9da00bb5c9 100644
> --- a/gcc/ipa-param-manipulation.h
> +++ b/gcc/ipa-param-manipulation.h
> @@ -16,101 +16,394 @@ for more details.
>  
>  You should have received a copy of the GNU General Public License
>  along with GCC; see the file COPYING3.  If not see
> -<http://www.gnu.org/licenses/>.  */
> +<http://www.gnu.org/licenses/>.
> +
> +
> +
> +This file defines classes and other data structures that are used to manipulate
> +the prototype of a function, especially to create, remove or split its formal
> +parameters, but also to remove its return value, and also its call statements
> +correspondingly.
> +
> +The most basic one is a vector of structures ipa_adjusted_param.  It is simply
> +a description how the new parameters should look like after the transformation
> +in what way they relate to the previous ones (if in any).  Such relation to an
> +old parameter can be an outright copy or an IPA-SRA replacement. If an old
> +parameter is not listed or otherwise mentioned, it is removed as unused or at
> +least unnecessary.  Note that this most basic structure does not work for
> +modifying calls of functions with variable number of arguments.
> +
> +Class ipa_param_adjustments is only a little more than a thin encapsulation of
> +a vector of ipa_param_adjustments.  Along with this vector it contains an index
> +of the first potential vararg argument and a boolean flag whether the return
> +value should be removed or not.  Moreover, the class contains method
> +modify_call which can transform a call statement so that it correctly calls a
> +modified function.  These two data structures were designed to have a small
> +memory footprint because they are allocated for each clone of a call graph node
> +that has its prototype changed and live until the end of IPA clone
> +materialization and call redirection phase.
> +
> +On the other hand, class ipa_param_body_adjustments can afford to allocate more
> +data because its life span is much smaller, it is allocated and destroyed in
> +the course of materialization of each single clone that needs it or only when a
> +particular pass needs to change a function it is operating on.  This class has
> +various methods required to change function declaration and the body of the
> +function according to instructions given either by class ipa_param_adjustments
> +or only a vector of ipa_adjusted_params.
> +
> +When these classes are used in the context of call graph clone materialization
> +and subsequent call statement redirection - which is the point at which we
> +modify arguments in call statements - they need to cooperate with each other in
> +order to handle what we refer to as transitive (IPA-SRA) splits.  These are
> +situations when a formal parameter of one function is split into several
> +smaller ones and some of them are then passed on in a call to another function
> +because the formal parameter of this callee has also been split.
> +
> +Consider a simple example:
> +
> +struct S {int a, b, c;};
> +struct Z {int x; S s;};
> +
> +foo (S s)
> +{
> +  use (s.b);
> +}
> +
> +bar (Z z)
> +{
> +  use (z.s.a);
> +  foo (z.s);
> +}
> +
> +baz ()
> +{
> +  bar (*global);
> +}
> +
> +Both bar and foo would have their parameter split.  Foo would receive one
> +replacement representing s.b.  Function bar would see its parameter split into
> +one replacement representing z.s.a and another representing z.s.b which would
> +be passed on to foo.  It would be a so called transitive split IPA-SRA
> +replacement, one which is passed in a call as an actual argument to another
> +IPA-SRA replacement in another function.
> +
> +Note that the call chain the example can be arbitrarily long and recursive and
> +that any function in it can be cloned by another IPA pass and any number of
> +adjacent functions in the call chain can be inlined into each other.  Call
> +redirection takes place only after bodies of the function have been modified by
> +all of the above.
> +
> +Call redirection has to be able to find the right decl or SSA_NAME that
> +corresponds to the transitive split in the caller.  The SSA names are assigned
> +right after clone materialization/ modification and cannot be "added"
> +afterwards.  Moreover, if the caller has been inlined the SSA_NAMEs in question
> +no longer belong to PARM_DECLs but to VAR_DECLs, indistinguishable from any
> +others.
> +
> +Therefore, when clone materialization finds a call statement which it knows is
> +a part of a transitive split, it will modify it into:
> +
> +  foo (DUMMY_Z_VAR.s, repl_for_a, repl_for_b, <rest of original arguments>);
> +
> +It will also store {DUMMY_S_VAR, 32} and {DUMMY_S_VAR, 64} representing offsets
> +of z.s.a and z.s.b (assuming a 32-bit int) into foo's cgraph node
> +clone->performed_splits vector (which is storing structures of type
> +ipa_param_performed_split also defined in this header file).
> +
> +Call redirection will identify that expression DUMMY_Z_VAR.s is based on a
> +variable stored in performed_splits vector and learn that the following
> +arguments, already in SSA form, represent offsets 32 and 64 in a split original
> +parameter.  It subtracts offset of DUMMY_Z_VAR.s from 32 and 64 and arrives at
> +offsets 0 and 32 within callee's original parameter.  At this point it also
> +knows from the call graph that only the bit with offset 32 is needed and so
> +changes the call statement into final:
> +
> +bar (repl_for_b, <rest of original arguments>);  */

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.

Honza