[PATCH] True IPA reimplementation of IPA-SRA
Martin Jambor
mjambor@suse.cz
Thu May 16 14:04:00 GMT 2019
Hi Richi,
On Thu, May 16 2019, Richard Biener wrote:
> 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);
So because the call to foo dominates EXIT and uses default definition
MEM SSA, this example would be handled fine by the patch. But it cannot
handle for example (assuming p->a is an int):
bar (X *p) { *global_double_ptr = 0.0; foo (p); }
The current IPA-SRA can, because at the time it looks at foo, bar has
been already processed, and so it knows the load is of integer type. If
necessary, we could try TBAA for fields in X if there is a reasonable
number of them and at IPA level just check a flag saying that bar does
not engage in some type-punning.
Another example would be something like:
bar (X *p) { if (cond) bar (p); else do_something_else (p->a); }
The problem here is that the check if p is sure to be dereferenced when
bar is called is also done at the intra-procedural level. Well, it is
not actually a test if p is dereferenced but if the offset from p which
is known to be dereferenced covers p->a. We could do it symbolically,
arrive at some expression of the form
MIN(offsetof(a), known_dereference_offset_in (bar))
store that to the IPA summary and then evaluate at IPA level. If we
think that it is worth it.
Still, I don't think the situation is that much worse in practice
because IPA-SRA can only handle fairly simple cases anyway, and those
are actually often taken care of by indirect inlining.
>
> 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.
The analysis parts of the new IPA-SRA, both the intra-procedural and
inter-procedural are entirely in the new file ipa-sra.c so if you want
to review that, just grab that file from
https://gcc.gnu.org/git/?p=gcc.git;a=tree;h=refs/heads/jamborm/ipa-sra;hb=refs/heads/jamborm/ipa-sra
The transformation part, however, are what the "refactoring" is really
about because it is not the pass but rather the cgraph cloning
infrastructure that performs the actual transformation. This is so on
purpose because not only the bodies of changed functions need to be
adjusted but also all calls to them, and you cannot register a
pass-specific transformation function for that - and I need to actually
pass information from the body transformation to the call transformation
anyway.
So yes, this split would be possible and perhaps even easy but it would
not make much of a difference.
Thanks,
Martin
>
> 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