Fix handling of access ranges in ipa-modref
Jan Hubicka
hubicka@ucw.cz
Sun Oct 11 12:21:15 GMT 2020
Hi,
this patch fixes the range tracking in argument and re-enables it for clones
(the bug that broke dealII and x264 benchmarks)
It turned out that there was three problems
1) for SRA/ipa-cp clones we did not update summarries to represent new
signature. This is now done in modref_transform.
I tested it in ipa-sra testcases and it seems to work fine, but Martin,
can you please take a look?
Param adjustment interface provides original indexes for new indexes of
parameters. I need reverse information: for original index I need new
that I compute via array map and then do the rewrite.
Martin, if things passed by references are translated to stuff
passed by value, we may eliminate the corrsponding acess records,
because reading function parameters is not considered a side-effect
by mod-ref. Is there easy way to detect such change?
2) The propagation via jump functions (in applying inline decision)
mixed bits and bytes in parameter offsets.
Martin, it is not clear to me why the offset is in bits - there is no
way to pass a pointer to non-byte aligned address and it seems that this
only adds a risk of overflows.
There seems to be overflow check missing in
update_jump_functions_after_inlining, but it seems to me that these days
this should be poly_int64.
3) There was bug disabling late mod-ref during IPA, since the ipa flag was
set incorrectly in the summary.
This made it bit hard to work out what happens in dealII and x264. They was
both hit primarily by 2) but if there was no 3) it would not trigger the
problem, so I did not get the idea to doube-check the jump function
handling.
1) seems to be surprisingly harmless because we have only one clone in
dealII that needs updating. It seems that sra and ipa-cp heuristics are
still way too conservative.
Current disambiguation stats for cc1plus are:
Alias oracle query stats:
refs_may_alias_p: 63936508 disambiguations, 74133491 queries
ref_maybe_used_by_call_p: 141601 disambiguations, 64838386 queries
call_may_clobber_ref_p: 22977 disambiguations, 28758 queries
nonoverlapping_component_refs_p: 0 disambiguations, 37176 queries
nonoverlapping_refs_since_match_p: 19409 disambiguations, 55558 must overlaps, 75758 queries
aliasing_component_refs_p: 54733 disambiguations, 755912 queries
TBAA oracle: 22986556 disambiguations 55156567 queries
16068160 are in alias set 0
10559362 queries asked about the same object
125 queries asked about the same alias set
0 access volatile
3912975 are dependent in the DAG
1629389 are aritificially in conflict with void *
Modref stats:
modref use: 11111 disambiguations, 39535 queries
modref clobber: 1399913 disambiguations, 1719874 queries
3500169 tbaa queries (2.035131 per modref query)
621878 base compares (0.361583 per modref query)
PTA query stats:
pt_solution_includes: 967033 disambiguations, 13594707 queries
pt_solutions_intersect: 1032740 disambiguations, 13097793 queries
This is comparable to previous stats
https://gcc.gnu.org/pipermail/gcc-patches/2020-October/555314.html
those seems to have similar disambiguation stats but more querries.
This may be related to
https://gcc.gnu.org/pipermail/gcc-patches/2020-October/555812.html
that disables some TBAA disambiguation where gimple memory model requires so.
(indeed modref TBAA query count is down by 36%)
Bootstrapped/regtested x86_64-linux, comitted (in 4 commits for which I
apologize. I was intending to commit to branch)
2020-10-10 Jan Hubicka <hubicka@ucw.cz>
* ipa-modref-tree.h (struct modref_tree): Revert prevoius change.
* ipa-modref.c (analyze_function): Dump original summary.
(modref_read): Only set IPA if streaming summary (not optimization
summary).
(remap_arguments): New function.
(modref_transform): New function.
(compute_parm_map): Fix offset calculation.
(ipa_merge_modref_summary_after_inlining): Do not merge stores when
they can be ignored.
diff --git a/gcc/ipa-modref-tree.h b/gcc/ipa-modref-tree.h
index 8d7f2864793..b37280d18c7 100644
--- a/gcc/ipa-modref-tree.h
+++ b/gcc/ipa-modref-tree.h
@@ -496,8 +496,7 @@ struct GTY((user)) modref_tree
/* Copy OTHER to THIS. */
void copy_from (modref_tree <T> *other)
{
- auto_vec <modref_parm_map, 32> parm_map;
- merge (other, &parm_map);
+ merge (other, NULL);
}
/* Search BASE in tree; return NULL if failed. */
diff --git a/gcc/ipa-modref.c b/gcc/ipa-modref.c
index c22c0d233f7..dd59e804c0f 100644
--- a/gcc/ipa-modref.c
+++ b/gcc/ipa-modref.c
@@ -757,7 +757,14 @@ analyze_function (function *f, bool ipa)
if (!summaries)
summaries = modref_summaries::create_ggc (symtab);
else /* Remove existing summary if we are re-running the pass. */
- summaries->remove (cgraph_node::get (f->decl));
+ {
+ if (dump_file && summaries->get (cgraph_node::get (f->decl)))
+ {
+ fprintf (dump_file, "Past summary:\n");
+ summaries->get (cgraph_node::get (f->decl))->dump (dump_file);
+ }
+ summaries->remove (cgraph_node::get (f->decl));
+ }
((modref_summaries *)summaries)->ipa = ipa;
@@ -1290,7 +1297,7 @@ modref_read (void)
if (!summaries)
summaries = modref_summaries::create_ggc (symtab);
- ((modref_summaries *)summaries)->ipa = true;
+ ((modref_summaries *)summaries)->ipa = !flag_ltrans;
while ((file_data = file_data_vec[j++]))
{
@@ -1309,6 +1316,81 @@ modref_read (void)
}
}
+/* Update parameter indexes in TT according to MAP. */
+
+void
+remap_arguments (vec <int> *map, modref_records *tt)
+{
+ size_t i;
+ modref_base_node <alias_set_type> *base_node;
+ FOR_EACH_VEC_SAFE_ELT (tt->bases, i, base_node)
+ {
+ size_t j;
+ modref_ref_node <alias_set_type> *ref_node;
+ FOR_EACH_VEC_SAFE_ELT (base_node->refs, j, ref_node)
+ {
+ size_t k;
+ modref_access_node *access_node;
+ FOR_EACH_VEC_SAFE_ELT (ref_node->accesses, k, access_node)
+ if (access_node->parm_index > 0)
+ {
+ if (access_node->parm_index < (int)map->length ())
+ access_node->parm_index = (*map)[access_node->parm_index];
+ else
+ access_node->parm_index = -1;
+ }
+ }
+ }
+}
+
+/* If signature changed, update the summary. */
+
+static unsigned int
+modref_transform (struct cgraph_node *node)
+{
+ if (!node->clone.param_adjustments || !summaries)
+ return 0;
+ modref_summary *r = summaries->get (node);
+ if (!r)
+ return 0;
+ if (dump_file)
+ {
+ fprintf (dump_file, "Updating summary for %s from:\n",
+ node->dump_name ());
+ r->dump (dump_file);
+ }
+
+ size_t i, max = 0;
+ ipa_adjusted_param *p;
+
+ FOR_EACH_VEC_SAFE_ELT (node->clone.param_adjustments->m_adj_params, i, p)
+ {
+ int idx = node->clone.param_adjustments->get_original_index (i);
+ if (idx > (int)max)
+ max = idx;
+ }
+
+ auto_vec <int, 32> map;
+
+ map.reserve (max + 1);
+ for (i = 0; i <= max; i++)
+ map.quick_push (-1);
+ FOR_EACH_VEC_SAFE_ELT (node->clone.param_adjustments->m_adj_params, i, p)
+ {
+ int idx = node->clone.param_adjustments->get_original_index (i);
+ if (idx >= 0)
+ map[idx] = i;
+ }
+ remap_arguments (&map, r->loads);
+ remap_arguments (&map, r->stores);
+ if (dump_file)
+ {
+ fprintf (dump_file, "to:\n");
+ r->dump (dump_file);
+ }
+ return 0;
+}
+
/* Definition of the modref IPA pass. */
const pass_data pass_data_ipa_modref =
{
@@ -1335,7 +1417,7 @@ public:
modref_read, /* read_optimization_summary */
NULL, /* stmt_fixup */
0, /* function_transform_todo_flags_start */
- NULL, /* function_transform */
+ modref_transform,/* function_transform */
NULL) /* variable_transform */
{}
@@ -1448,7 +1530,10 @@ compute_parm_map (cgraph_edge *callee_edge, vec<modref_parm_map> *parm_map)
{
(*parm_map)[i].parm_index = ipa_get_jf_ancestor_formal_id (jf);
(*parm_map)[i].parm_offset_known = true;
- (*parm_map)[i].parm_offset = ipa_get_jf_ancestor_offset (jf);
+ gcc_checking_assert
+ (!(ipa_get_jf_ancestor_offset (jf) & (BITS_PER_UNIT - 1)));
+ (*parm_map)[i].parm_offset
+ = ipa_get_jf_ancestor_offset (jf) >> LOG2_BITS_PER_UNIT;
}
else
(*parm_map)[i].parm_index = -1;
@@ -1503,10 +1588,13 @@ ipa_merge_modref_summary_after_inlining (cgraph_edge *edge)
compute_parm_map (edge, &parm_map);
- if (to_info->loads)
- to_info->loads->merge (callee_info->loads, &parm_map);
- if (to_info->stores)
- to_info->stores->merge (callee_info->stores, &parm_map);
+ if (!ignore_stores_p (edge->callee->decl, flags))
+ {
+ if (to_info->loads)
+ to_info->loads->merge (callee_info->loads, &parm_map);
+ if (to_info->stores)
+ to_info->stores->merge (callee_info->stores, &parm_map);
+ }
if (to_info->loads_lto)
to_info->loads_lto->merge (callee_info->loads_lto, &parm_map);
if (to_info->stores_lto)
More information about the Gcc-patches
mailing list