diff options
author | Martin Jambor <mjambor@suse.cz> | 2023-10-30 18:34:59 +0100 |
---|---|---|
committer | Martin Jambor <mjambor@suse.cz> | 2023-10-30 18:36:54 +0100 |
commit | 997c8219f020091d00fd225c81270aa551bfe9e4 (patch) | |
tree | ad39c75a3b0a4e443cb6dc0c12880ec088eb27dd | |
parent | 1437df40f12ade35fd1f1a3e4cbba4b4656cab0b (diff) | |
download | gcc-997c8219f020091d00fd225c81270aa551bfe9e4.zip gcc-997c8219f020091d00fd225c81270aa551bfe9e4.tar.gz gcc-997c8219f020091d00fd225c81270aa551bfe9e4.tar.bz2 |
ipa: Prune any IPA-CP aggregate constants known by modref to be killed (111157)
PR 111157 shows that IPA-modref and IPA-CP (when plugged into value
numbering) can optimize out a store both before a call (because the
call will overwrite it) and in the call (because the store is of the
same value) and by eliminating both create miscompilation.
This patch fixes that by pruning any constants from the list of IPA-CP
aggregate value constants that it knows the contents of the memory can
be "killed." Unfortunately, doing so is tricky. First, IPA-modref
loads override kills and so only stores not loaded are truly not
necessary. Looking stuff up there means doing what most of what
modref_may_alias may do but doing exactly what it does is tricky
because it takes also aliasing into account and has bail-out counters.
To err on the side of caution in order to avoid this miscompilation we
have to prune a constant when in doubt. However, pruning can
interfere with the mechanism of how clone materialization
distinguishes between the cases when a parameter was entirely removed
and when it was both IPA-CPed and IPA-SRAed (in order to make up for
the removal in debug info, which can bump into an assert when
compiling g++.dg/torture/pr103669.C when we are not careful).
Therefore this patch:
1) marks constants that IPA-modref has in its kill list with a new
"killed" flag, and
2) prunes the list from entries with this flag after materialization
and IPA-CP transformation is done using the template introduced in
the previous patch
It does not try to look up anything in the load lists, this will be
done as a follow-up in order to ease review.
gcc/ChangeLog:
2023-10-27 Martin Jambor <mjambor@suse.cz>
PR ipa/111157
* ipa-prop.h (struct ipa_argagg_value): Newf flag killed.
* ipa-modref.cc (ipcp_argagg_and_kill_overlap_p): New function.
(update_signature): Mark any any IPA-CP aggregate constants at
positions known to be killed as killed. Move check that there is
clone_info after this pruning.
* ipa-cp.cc (ipa_argagg_value_list::dump): Dump the killed flag.
(ipa_argagg_value_list::push_adjusted_values): Clear the new flag.
(push_agg_values_from_plats): Likewise.
(ipa_push_agg_values_from_jfunc): Likewise.
(estimate_local_effects): Likewise.
(push_agg_values_for_index_from_edge): Likewise.
* ipa-prop.cc (write_ipcp_transformation_info): Stream the killed
flag.
(read_ipcp_transformation_info): Likewise.
(ipcp_get_aggregate_const): Update comment, assert that encountered
record does not have killed flag set.
(ipcp_transform_function): Prune all aggregate constants with killed
set.
gcc/testsuite/ChangeLog:
2023-09-18 Martin Jambor <mjambor@suse.cz>
PR ipa/111157
* gcc.dg/lto/pr111157_0.c: New test.
* gcc.dg/lto/pr111157_1.c: Second file of the same new test.
-rw-r--r-- | gcc/ipa-cp.cc | 8 | ||||
-rw-r--r-- | gcc/ipa-modref.cc | 61 | ||||
-rw-r--r-- | gcc/ipa-prop.cc | 17 | ||||
-rw-r--r-- | gcc/ipa-prop.h | 4 | ||||
-rw-r--r-- | gcc/testsuite/gcc.dg/lto/pr111157_0.c | 24 | ||||
-rw-r--r-- | gcc/testsuite/gcc.dg/lto/pr111157_1.c | 10 |
6 files changed, 118 insertions, 6 deletions
diff --git a/gcc/ipa-cp.cc b/gcc/ipa-cp.cc index e056030..788157e 100644 --- a/gcc/ipa-cp.cc +++ b/gcc/ipa-cp.cc @@ -1271,6 +1271,8 @@ ipa_argagg_value_list::dump (FILE *f) print_generic_expr (f, av.value); if (av.by_ref) fprintf (f, "(by_ref)"); + if (av.killed) + fprintf (f, "(killed)"); comma = true; } fprintf (f, "\n"); @@ -1437,6 +1439,8 @@ ipa_argagg_value_list::push_adjusted_values (unsigned src_index, new_av.unit_offset = av->unit_offset - unit_delta; new_av.index = dest_index; new_av.by_ref = av->by_ref; + gcc_assert (!av->killed); + new_av.killed = false; /* Quick check that the offsets we push are indeed increasing. */ gcc_assert (first @@ -1473,6 +1477,7 @@ push_agg_values_from_plats (ipcp_param_lattices *plats, int dest_index, iav.unit_offset = aglat->offset / BITS_PER_UNIT - unit_delta; iav.index = dest_index; iav.by_ref = plats->aggs_by_ref; + iav.killed = false; gcc_assert (first || iav.unit_offset > prev_unit_offset); @@ -2139,6 +2144,7 @@ ipa_push_agg_values_from_jfunc (ipa_node_params *info, cgraph_node *node, iav.unit_offset = item.offset / BITS_PER_UNIT; iav.index = dst_index; iav.by_ref = agg_jfunc->by_ref; + iav.killed = 0; gcc_assert (first || iav.unit_offset > prev_unit_offset); @@ -3981,6 +3987,7 @@ estimate_local_effects (struct cgraph_node *node) avals.m_known_aggs[j].unit_offset = unit_offset; avals.m_known_aggs[j].index = index; avals.m_known_aggs[j].by_ref = plats->aggs_by_ref; + avals.m_known_aggs[j].killed = false; perform_estimation_of_a_value (node, &avals, removable_params_cost, 0, val); @@ -5857,6 +5864,7 @@ push_agg_values_for_index_from_edge (struct cgraph_edge *cs, int index, iav.unit_offset = agg_jf.offset / BITS_PER_UNIT; iav.index = index; iav.by_ref = jfunc->agg.by_ref; + iav.killed = false; gcc_assert (first || iav.unit_offset > prev_unit_offset); diff --git a/gcc/ipa-modref.cc b/gcc/ipa-modref.cc index fe55621..cbe788a 100644 --- a/gcc/ipa-modref.cc +++ b/gcc/ipa-modref.cc @@ -4065,21 +4065,74 @@ remap_kills (vec <modref_access_node> &kills, const vec <int> &map) i++; } +/* Return true if the V can overlap with KILL. */ + +static bool +ipcp_argagg_and_kill_overlap_p (const ipa_argagg_value &v, + const modref_access_node &kill) +{ + if (kill.parm_index == v.index) + { + gcc_assert (kill.parm_offset_known); + gcc_assert (known_eq (kill.max_size, kill.size)); + poly_int64 repl_size; + bool ok = poly_int_tree_p (TYPE_SIZE (TREE_TYPE (v.value)), + &repl_size); + gcc_assert (ok); + poly_int64 repl_offset (v.unit_offset); + repl_offset <<= LOG2_BITS_PER_UNIT; + poly_int64 combined_offset + = (kill.parm_offset << LOG2_BITS_PER_UNIT) + kill.offset; + if (ranges_maybe_overlap_p (repl_offset, repl_size, + combined_offset, kill.size)) + return true; + } + return false; +} + /* If signature changed, update the summary. */ static void update_signature (struct cgraph_node *node) { - clone_info *info = clone_info::get (node); - if (!info || !info->param_adjustments) - return; - modref_summary *r = optimization_summaries ? optimization_summaries->get (node) : NULL; modref_summary_lto *r_lto = summaries_lto ? summaries_lto->get (node) : NULL; if (!r && !r_lto) return; + + /* Propagating constants in killed memory can lead to eliminated stores in + both callees (because they are considered redundant) and callers, leading + to missing them altogether. */ + ipcp_transformation *ipcp_ts = ipcp_get_transformation_summary (node); + if (ipcp_ts) + { + for (auto &v : ipcp_ts->m_agg_values) + { + if (!v.by_ref) + continue; + if (r) + for (const modref_access_node &kill : r->kills) + if (ipcp_argagg_and_kill_overlap_p (v, kill)) + { + v.killed = true; + break; + } + if (!v.killed && r_lto) + for (const modref_access_node &kill : r_lto->kills) + if (ipcp_argagg_and_kill_overlap_p (v, kill)) + { + v.killed = true; + break; + } + } + } + + clone_info *info = clone_info::get (node); + if (!info || !info->param_adjustments) + return; + if (dump_file) { fprintf (dump_file, "Updating summary for %s from:\n", diff --git a/gcc/ipa-prop.cc b/gcc/ipa-prop.cc index 9442cdd..827bdb6 100644 --- a/gcc/ipa-prop.cc +++ b/gcc/ipa-prop.cc @@ -5299,6 +5299,7 @@ write_ipcp_transformation_info (output_block *ob, cgraph_node *node, bp = bitpack_create (ob->main_stream); bp_pack_value (&bp, av.by_ref, 1); + bp_pack_value (&bp, av.killed, 1); streamer_write_bitpack (&bp); } @@ -5331,6 +5332,7 @@ read_ipcp_transformation_info (lto_input_block *ib, cgraph_node *node, bitpack_d bp = streamer_read_bitpack (ib); av->by_ref = bp_unpack_value (&bp, 1); + av->killed = bp_unpack_value (&bp, 1); } } @@ -5616,7 +5618,9 @@ ipcp_modif_dom_walker::before_dom_children (basic_block bb) /* If IPA-CP discovered a constant in parameter PARM at OFFSET of a given SIZE - whether passed by reference or not is given by BY_REF - return that - constant. Otherwise return NULL_TREE. */ + constant. Otherwise return NULL_TREE. The is supposed to be used only + after clone materialization and transformation is done (because it asserts + that killed constants have been pruned). */ tree ipcp_get_aggregate_const (struct function *func, tree parm, bool by_ref, @@ -5634,7 +5638,11 @@ ipcp_get_aggregate_const (struct function *func, tree parm, bool by_ref, ipa_argagg_value_list avl (ts); unsigned unit_offset = bit_offset / BITS_PER_UNIT; - tree v = avl.get_value (index, unit_offset, by_ref); + const ipa_argagg_value *av = avl.get_elt (index, unit_offset); + if (!av || av->by_ref != by_ref) + return NULL_TREE; + gcc_assert (!av->killed); + tree v = av->value; if (!v || maybe_ne (tree_to_poly_int64 (TYPE_SIZE (TREE_TYPE (v))), bit_size)) return NULL_TREE; @@ -5884,6 +5892,11 @@ ipcp_transform_function (struct cgraph_node *node) free_ipa_bb_info (bi); fbi.bb_infos.release (); + ts->remove_argaggs_if ([](const ipa_argagg_value &v) + { + return v.killed; + }); + vec_free (descriptors); if (cfg_changed) delete_unreachable_blocks_update_callgraph (node, false); diff --git a/gcc/ipa-prop.h b/gcc/ipa-prop.h index 34b0d77..fcd0e5c 100644 --- a/gcc/ipa-prop.h +++ b/gcc/ipa-prop.h @@ -213,6 +213,10 @@ struct GTY(()) ipa_argagg_value unsigned index : IPA_PROP_ARG_INDEX_LIMIT_BITS; /* Whether the value was passed by reference. */ unsigned by_ref : 1; + /* Set if the value should not be used after materialization in + value_numbering. It is kept around just so that clone materialization can + distinguish a combined IPA-CP and IPA-SRA from a deleted argument. */ + unsigned killed : 1; }; /* A view into a sorted list of aggregate values in a particular context, be it diff --git a/gcc/testsuite/gcc.dg/lto/pr111157_0.c b/gcc/testsuite/gcc.dg/lto/pr111157_0.c new file mode 100644 index 0000000..8bb4c65 --- /dev/null +++ b/gcc/testsuite/gcc.dg/lto/pr111157_0.c @@ -0,0 +1,24 @@ +/* { dg-lto-do run } */ +/* { dg-lto-options { { -O2 -flto=auto } } } */ +/* { dg-extra-ld-options { -flto-partition=1to1 } } */ + +extern __attribute__((noinline)) +void foo (int *p); + + +void __attribute__((noinline)) +bar (void) +{ + int istat; + + istat = 1234; + foo (&istat); + if (istat != 1234) + __builtin_abort (); +} + +int main (int argc, char **argv) +{ + bar (); + return 0; +} diff --git a/gcc/testsuite/gcc.dg/lto/pr111157_1.c b/gcc/testsuite/gcc.dg/lto/pr111157_1.c new file mode 100644 index 0000000..f9ba5af --- /dev/null +++ b/gcc/testsuite/gcc.dg/lto/pr111157_1.c @@ -0,0 +1,10 @@ +volatile int v = 0; + +void __attribute__((noinline)) +foo (int *p) +{ + *p = 1234; + if (v) + *p = 0; + return; +} |