[PATCH] True IPA reimplementation of IPA-SRA
Richard Biener
richard.guenther@gmail.com
Thu May 16 12:09:00 GMT 2019
On Fri, May 10, 2019 at 10:31 AM Martin Jambor <mjambor@suse.cz> wrote:
>
> Hello,
>
> this is a follow-up from a WIP patch I sent here in late December:
> https://gcc.gnu.org/ml/gcc-patches/2018-12/msg01765.html
>
> Just like the last time, the patch below is is a reimplementation of
> IPA-SRA to make it a full IPA pass that can handle strictly connected
> components in the call-graph, can take advantage of LTO and does not
> weirdly switch functions in pass-pipeline like our current quasi-IPA SRA
> does. Unlike the current IPA-SRA it can also remove return values, even
> in SCCs. On the other hand, it is less powerful when it comes to
> structures passed by reference. By design it will not create references
> to bits of an aggregate because that turned out to be just obfuscation
> in practice. However, it also cannot usually split aggregates passed by
> reference that are just passed to another function (where splitting
> would be useful) because it cannot perform the same TBAA analysis like
> the current implementation which already knows what types it should look
> at because it has access to bodies of all functions attempts to modify.
So that's just because the analysis is imperfect? I mean if we can handle
foo (X *p) { do_something (p->a); }
X a; a.a = 1; foo (&a);
then we should be able to handle
bar (X *p) { foo (p); }
X a; a.a = 1; bar (&a);
by just applying local analysis of foo when looking at what to do for bar?
So isn't that what a IPA propagation step should do?
> Since the last time I have fixed a number of bugs that Martin Liška
> found when compiling a portion of openSUSE with the patch, removed all
> the FIXMEs, made long living memory structures more compact and
> self-reviewed the entire patch once.
>
> Therefore, I would like to ask for a review and eventually for an
> approval to commit the patch to the trunk. The patch survives
> bootstrap, LTO bootstrap and LTO profiledbootstrap on x86_64-linux. In
> the testsuite, it "fixes" 24 guality passes (all LTO ones) but breaks 12
> other ones (one is non-LTO). I would welcome any help with addressing
> these. Because the patch removes the old IPA-SRA, the input to the IPA
> pipeline looks different and so I could not just try to make it "process
> the debug statements like before."
>
> Because the patch is big I had to compress it to get it through to
> gcc-patches. Because of its size and because it contains a completely
> new file ipa-sra.c and total reimplementations of
> ipa-param-manipulation.[ch], and so I pushed my development branch to
> branch jamborm/ipa-sra on GCC git server
> (https://gcc.gnu.org/git/?p=gcc.git;a=shortlog;h=refs/heads/jamborm/ipa-sra).
> It may be more convenient to check it out and review it that way.
>
> Thanks in advance for any questions, comments and suggestions,
Thanks for doing this. I wonder how difficult it is to split the patch
into a) old IPA-SRA removal, b) refactoring c) IPA-SRA add (probably
easiest in that order). It's quite a large number of changes, a)
being mostly uninteresting (and pre-approved hereby, just not
independently, of course), b) is uninteresting to me, but I would like
to look at c), not sure if that's really only the new file, probably not
since IPA modifications have infrastructure bits.
Sorry for not mentioning earlier. Maybe just splitting out a) already
helps (you seem to remove code not in tree-sra.c).
Thanks,
Richard.
> Martin
>
>
>
> 2019-05-09 Martin Jambor <mjambor@suse.cz>
>
> * coretypes.h (cgraph_edge): Declare.
> * ipa-param-manipulation.c: Rewrite.
> * ipa-param-manipulation.h: Likewise.
> * Makefile.in (GTFILES): Added ipa-param-manipulation.h and ipa-sra.c.
> (OBJS): Added ipa-sra.o.
> * 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.
> * ipa-fnsummary.c (ipa_fn_summary_t::duplicate): Simplify
> ipa_replace_map check.
> * 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.
> * 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.
> * lto-section-in.c (lto_section_name): Added ipa-sra section.
> * lto-streamer.h (lto_section_type): Likewise.
> * tree-inline.h (copy_body_data): New field killed_new_ssa_names.
> (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.
> * tree-inline.h (struct copy_body_data): New field param_body_adjs.
> * 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.
> * tree-sra.c: Removed IPA-SRA. Include tree-sra.h.
> (type_internals_preclude_sra_p): Make public.
> * tree-sra.h: New file.
> * ipa-inline-transform.c (save_inline_function_body): Update to
> refelct new tree_function_versioning signature.
> * ipa-prop.c (adjust_agg_replacement_values): Use a helper from
> (ipcp_modif_dom_walker::before_dom_children): Likewise.
> ipa_param_adjustments to get current parameter indices.
> (ipcp_update_bits): Likewise.
> (ipcp_update_vr): Likewise.
> * ipa-split.c (split_function): Convert to using ipa_param_adjustments.
> * ipa-sra.c: New file.
> * multiple_target.c (create_target_clone): Update to reflet new type
> of create_version_clone_with_body.
> * trans-mem.c (ipa_tm_create_version): Update to reflect new type of
> tree_function_versioning.
> * tree-sra.c (modify_function): Update to reflect new type of
> tree_function_versioning.
> * params.def (PARAM_IPA_SRA_MAX_REPLACEMENT): New.
> (PARAM_SRA_MAX_TYPE_CHECK_STEPS): New.
> * passes.def: Remove old IPA-SRA and add new one.
> * tree-pass.h (make_pass_early_ipa_sra): Remove declaration.
> (make_pass_ipa_sra): Declare.
>
> testsuite/
> * g++.dg/ipa/pr81248.C: Adjust dg-options and dump-scan.
> * gcc.dg/ipa/ipa-sra-1.c: Likewise.
> * gcc.dg/ipa/ipa-sra-10.c: Likewise.
> * gcc.dg/ipa/ipa-sra-11.c: Likewise.
> * gcc.dg/ipa/ipa-sra-3.c: Likewise.
> * gcc.dg/ipa/ipa-sra-4.c: Likewise.
> * gcc.dg/ipa/ipa-sra-5.c: Likewise.
> * gcc.dg/ipa/ipacost-2.c: Disable ipa-sra.
> * gcc.dg/ipa/ipcp-agg-9.c: Likewise.
> * gcc.dg/ipa/pr78121.c: Adjust scan pattern.
> * gcc.dg/ipa/vrp1.c: Likewise.
> * gcc.dg/ipa/vrp2.c: Likewise.
> * gcc.dg/ipa/vrp3.c: Likewise.
> * gcc.dg/ipa/vrp7.c: Likewise.
> * gcc.dg/ipa/vrp8.c: Likewise.
> * gcc.dg/noreorder.c: use noipa attribute instead of noinline.
> * gcc.dg/ipa/20040703-wpa.c: New test.
> * gcc.dg/ipa/ipa-sra-12.c: New test.
> * gcc.dg/ipa/ipa-sra-13.c: Likewise.
> * gcc.dg/ipa/ipa-sra-14.c: Likewise.
> * gcc.dg/ipa/ipa-sra-15.c: Likewise.
> * gcc.dg/ipa/ipa-sra-16.c: Likewise.
> * gcc.dg/ipa/ipa-sra-17.c: Likewise.
> * gcc.dg/ipa/ipa-sra-18.c: Likewise.
> * gcc.dg/ipa/ipa-sra-19.c: Likewise.
> * gcc.dg/ipa/ipa-sra-20.c: Likewise.
> * gcc.dg/ipa/ipa-sra-21.c: Likewise.
> * gcc.dg/sso/ipa-sra-1.c: Likewise.
>
> * gcc.dg/ipa/ipa-sra-2.c: Mark to be removed.
> * gcc.dg/ipa/ipa-sra-6.c: Likewise.
>
>
More information about the Gcc-patches
mailing list