public inbox for gcc-cvs@sourceware.org
help / color / mirror / Atom feed
* [gcc r12-4978] Fix inter-procedural EAF flags propagation with respect to !binds_to_current_def_p
@ 2021-11-07 17:21 Jan Hubicka
0 siblings, 0 replies; only message in thread
From: Jan Hubicka @ 2021-11-07 17:21 UTC (permalink / raw)
To: gcc-cvs
https://gcc.gnu.org/g:f6f704fd104b79fc88914978772737cd05423059
commit r12-4978-gf6f704fd104b79fc88914978772737cd05423059
Author: Jan Hubicka <hubicka@ucw.cz>
Date: Sun Nov 7 18:20:45 2021 +0100
Fix inter-procedural EAF flags propagation with respect to !binds_to_current_def_p
While proofreading the code for handling EAF flags of !binds_to_current_def_p I
noticed that the interprocedural dataflow actually ignores the flag possibly
introducing wrong code on quite complex interposable functions in non-trivial
recursion cycles (or at ltrans partition boundary).
This patch unifies the flags changes to single place (remove_useless_eaf_flags)
and does extend modref_merge_call_site_flags to do the right thing.
lto-bootstrapped/regtested x86_64-linux. Plan to commit it today after bit
more testing (firefox/clang build).
gcc/ChangeLog:
* gimple.c (gimple_call_arg_flags): Use interposable_eaf_flags.
(gimple_call_retslot_flags): Likewise.
(gimple_call_static_chain_flags): Likewise.
* ipa-modref.c (remove_useless_eaf_flags): Do not remove everything for
NOVOPS.
(modref_summary::useful_p): Likewise.
(modref_summary_lto::useful_p): Likewise.
(analyze_parms): Do not give up on NOVOPS.
(analyze_function): When dumping report chnages in EAF flags
between IPA and local pass.
(modref_merge_call_site_flags): Compute implicit eaf flags
based on callee ecf_flags and fnspec; if the function does not
bind to current defs use interposable_eaf_flags.
(modref_propagate_flags_in_scc): Update.
* ipa-modref.h (interposable_eaf_flags): New function.
Diff:
---
gcc/gimple.c | 34 ++--------
gcc/ipa-modref.c | 195 ++++++++++++++++++++++++++++++++++++++++++++++++-------
gcc/ipa-modref.h | 29 +++++++++
3 files changed, 203 insertions(+), 55 deletions(-)
diff --git a/gcc/gimple.c b/gcc/gimple.c
index 7a578f5113e..3d1d3a15b2c 100644
--- a/gcc/gimple.c
+++ b/gcc/gimple.c
@@ -1595,17 +1595,7 @@ gimple_call_arg_flags (const gcall *stmt, unsigned arg)
/* We have possibly optimized out load. Be conservative here. */
if (!node->binds_to_current_def_p ())
- {
- if ((modref_flags & EAF_UNUSED) && !(flags & EAF_UNUSED))
- {
- modref_flags &= ~EAF_UNUSED;
- modref_flags |= EAF_NOESCAPE;
- }
- if ((modref_flags & EAF_NOREAD) && !(flags & EAF_NOREAD))
- modref_flags &= ~EAF_NOREAD;
- if ((modref_flags & EAF_DIRECT) && !(flags & EAF_DIRECT))
- modref_flags &= ~EAF_DIRECT;
- }
+ modref_flags = interposable_eaf_flags (modref_flags, flags);
if (dbg_cnt (ipa_mod_ref_pta))
flags |= modref_flags;
}
@@ -1633,13 +1623,7 @@ gimple_call_retslot_flags (const gcall *stmt)
/* We have possibly optimized out load. Be conservative here. */
if (!node->binds_to_current_def_p ())
- {
- if ((modref_flags & EAF_UNUSED) && !(flags & EAF_UNUSED))
- {
- modref_flags &= ~EAF_UNUSED;
- modref_flags |= EAF_NOESCAPE;
- }
- }
+ modref_flags = interposable_eaf_flags (modref_flags, flags);
if (dbg_cnt (ipa_mod_ref_pta))
flags |= modref_flags;
}
@@ -1665,19 +1649,9 @@ gimple_call_static_chain_flags (const gcall *stmt)
{
int modref_flags = summary->static_chain_flags;
- /* We have possibly optimized out load. Be conservative here. */
+ /* ??? Nested functions should always bind to current def. */
if (!node->binds_to_current_def_p ())
- {
- if ((modref_flags & EAF_UNUSED) && !(flags & EAF_UNUSED))
- {
- modref_flags &= ~EAF_UNUSED;
- modref_flags |= EAF_NOESCAPE;
- }
- if ((modref_flags & EAF_NOREAD) && !(flags & EAF_NOREAD))
- modref_flags &= ~EAF_NOREAD;
- if ((modref_flags & EAF_DIRECT) && !(flags & EAF_DIRECT))
- modref_flags &= ~EAF_DIRECT;
- }
+ modref_flags = interposable_eaf_flags (modref_flags, flags);
if (dbg_cnt (ipa_mod_ref_pta))
flags |= modref_flags;
}
diff --git a/gcc/ipa-modref.c b/gcc/ipa-modref.c
index 5209fbdfbf4..4e64ee5d9fd 100644
--- a/gcc/ipa-modref.c
+++ b/gcc/ipa-modref.c
@@ -291,9 +291,7 @@ modref_summary::~modref_summary ()
static int
remove_useless_eaf_flags (int eaf_flags, int ecf_flags, bool returns_void)
{
- if (ecf_flags & ECF_NOVOPS)
- return 0;
- if (ecf_flags & ECF_CONST)
+ if (ecf_flags & (ECF_CONST | ECF_NOVOPS))
eaf_flags &= ~implicit_const_eaf_flags;
else if (ecf_flags & ECF_PURE)
eaf_flags &= ~implicit_pure_eaf_flags;
@@ -319,8 +317,6 @@ eaf_flags_useful_p (vec <eaf_flags_t> &flags, int ecf_flags)
bool
modref_summary::useful_p (int ecf_flags, bool check_flags)
{
- if (ecf_flags & ECF_NOVOPS)
- return false;
if (arg_flags.length () && !check_flags)
return true;
if (check_flags && eaf_flags_useful_p (arg_flags, ecf_flags))
@@ -331,7 +327,7 @@ modref_summary::useful_p (int ecf_flags, bool check_flags)
if (check_flags
&& remove_useless_eaf_flags (static_chain_flags, ecf_flags, false))
return true;
- if (ecf_flags & ECF_CONST)
+ if (ecf_flags & (ECF_CONST | ECF_NOVOPS))
return false;
if (loads && !loads->every_base)
return true;
@@ -405,8 +401,6 @@ modref_summary_lto::~modref_summary_lto ()
bool
modref_summary_lto::useful_p (int ecf_flags, bool check_flags)
{
- if (ecf_flags & ECF_NOVOPS)
- return false;
if (arg_flags.length () && !check_flags)
return true;
if (check_flags && eaf_flags_useful_p (arg_flags, ecf_flags))
@@ -417,7 +411,7 @@ modref_summary_lto::useful_p (int ecf_flags, bool check_flags)
if (check_flags
&& remove_useless_eaf_flags (static_chain_flags, ecf_flags, false))
return true;
- if (ecf_flags & ECF_CONST)
+ if (ecf_flags & (ECF_CONST | ECF_NOVOPS))
return false;
if (loads && !loads->every_base)
return true;
@@ -2321,10 +2315,6 @@ analyze_parms (modref_summary *summary, modref_summary_lto *summary_lto,
tree retslot = NULL;
tree static_chain = NULL;
- /* For novops functions we have nothing to gain by EAF flags. */
- if (ecf_flags & ECF_NOVOPS)
- return;
-
/* If there is return slot, look up its SSA name. */
if (DECL_RESULT (current_function_decl)
&& DECL_BY_REFERENCE (DECL_RESULT (current_function_decl)))
@@ -2471,6 +2461,11 @@ analyze_function (function *f, bool ipa)
modref_summary *summary = NULL;
modref_summary_lto *summary_lto = NULL;
+ bool past_flags_known = false;
+ auto_vec <eaf_flags_t> past_flags;
+ int past_retslot_flags = 0;
+ int past_static_chain_flags = 0;
+
/* Initialize the summary.
If we run in local mode there is possibly pre-existing summary from
IPA pass. Dump it so it is easy to compare if mod-ref info has
@@ -2490,6 +2485,11 @@ analyze_function (function *f, bool ipa)
fprintf (dump_file, "Past summary:\n");
optimization_summaries->get
(cgraph_node::get (f->decl))->dump (dump_file);
+ past_flags.reserve_exact (summary->arg_flags.length ());
+ past_flags.splice (summary->arg_flags);
+ past_retslot_flags = summary->retslot_flags;
+ past_static_chain_flags = summary->static_chain_flags;
+ past_flags_known = true;
}
optimization_summaries->remove (cgraph_node::get (f->decl));
}
@@ -2630,6 +2630,108 @@ analyze_function (function *f, bool ipa)
if (summary_lto)
summary_lto->dump (dump_file);
dump_modref_edge_summaries (dump_file, fnode, 2);
+ /* To simplify debugging, compare IPA and local solutions. */
+ if (past_flags_known && summary)
+ {
+ size_t len = summary->arg_flags.length ();
+
+ if (past_flags.length () > len)
+ len = past_flags.length ();
+ for (size_t i = 0; i < len; i++)
+ {
+ int old_flags = i < past_flags.length () ? past_flags[i] : 0;
+ int new_flags = i < summary->arg_flags.length ()
+ ? summary->arg_flags[i] : 0;
+ old_flags = remove_useless_eaf_flags
+ (old_flags, flags_from_decl_or_type (current_function_decl),
+ VOID_TYPE_P (TREE_TYPE (TREE_TYPE (current_function_decl))));
+ if (old_flags != new_flags)
+ {
+ if ((old_flags & ~new_flags) == 0)
+ fprintf (dump_file, " Flags for param %i improved:",
+ (int)i);
+ else if ((new_flags & ~old_flags) == 0)
+ fprintf (dump_file, " Flags for param %i worsened:",
+ (int)i);
+ else
+ fprintf (dump_file, " Flags for param %i changed:",
+ (int)i);
+ dump_eaf_flags (dump_file, old_flags, false);
+ fprintf (dump_file, " -> ");
+ dump_eaf_flags (dump_file, new_flags, true);
+ }
+ }
+ past_retslot_flags = remove_useless_eaf_flags
+ (past_retslot_flags,
+ flags_from_decl_or_type (current_function_decl),
+ VOID_TYPE_P (TREE_TYPE (TREE_TYPE (current_function_decl))));
+ if (past_retslot_flags != summary->retslot_flags)
+ {
+ if ((past_retslot_flags & ~summary->retslot_flags) == 0)
+ fprintf (dump_file, " Flags for retslot improved:");
+ else if ((summary->retslot_flags & ~past_retslot_flags) == 0)
+ fprintf (dump_file, " Flags for retslot worsened:");
+ else
+ fprintf (dump_file, " Flags for retslot changed:");
+ dump_eaf_flags (dump_file, past_retslot_flags, false);
+ fprintf (dump_file, " -> ");
+ dump_eaf_flags (dump_file, summary->retslot_flags, true);
+ }
+ past_static_chain_flags = remove_useless_eaf_flags
+ (past_static_chain_flags,
+ flags_from_decl_or_type (current_function_decl),
+ VOID_TYPE_P (TREE_TYPE (TREE_TYPE (current_function_decl))));
+ if (past_static_chain_flags != summary->static_chain_flags)
+ {
+ if ((past_static_chain_flags & ~summary->static_chain_flags) == 0)
+ fprintf (dump_file, " Flags for static chain improved:");
+ else if ((summary->static_chain_flags
+ & ~past_static_chain_flags) == 0)
+ fprintf (dump_file, " Flags for static chain worsened:");
+ else
+ fprintf (dump_file, " Flags for static chain changed:");
+ dump_eaf_flags (dump_file, past_static_chain_flags, false);
+ fprintf (dump_file, " -> ");
+ dump_eaf_flags (dump_file, summary->static_chain_flags, true);
+ }
+ }
+ else if (past_flags_known && !summary)
+ {
+ for (size_t i = 0; i < past_flags.length (); i++)
+ {
+ int old_flags = past_flags[i];
+ old_flags = remove_useless_eaf_flags
+ (old_flags, flags_from_decl_or_type (current_function_decl),
+ VOID_TYPE_P (TREE_TYPE (TREE_TYPE (current_function_decl))));
+ if (old_flags)
+ {
+ fprintf (dump_file, " Flags for param %i worsened:",
+ (int)i);
+ dump_eaf_flags (dump_file, old_flags, false);
+ fprintf (dump_file, " -> \n");
+ }
+ }
+ past_retslot_flags = remove_useless_eaf_flags
+ (past_retslot_flags,
+ flags_from_decl_or_type (current_function_decl),
+ VOID_TYPE_P (TREE_TYPE (TREE_TYPE (current_function_decl))));
+ if (past_retslot_flags)
+ {
+ fprintf (dump_file, " Flags for retslot worsened:");
+ dump_eaf_flags (dump_file, past_retslot_flags, false);
+ fprintf (dump_file, " ->\n");
+ }
+ past_static_chain_flags = remove_useless_eaf_flags
+ (past_static_chain_flags,
+ flags_from_decl_or_type (current_function_decl),
+ VOID_TYPE_P (TREE_TYPE (TREE_TYPE (current_function_decl))));
+ if (past_static_chain_flags)
+ {
+ fprintf (dump_file, " Flags for static chain worsened:");
+ dump_eaf_flags (dump_file, past_static_chain_flags, false);
+ fprintf (dump_file, " ->\n");
+ }
+ }
}
}
@@ -4039,12 +4141,15 @@ modref_merge_call_site_flags (escape_summary *sum,
modref_summary *summary,
modref_summary_lto *summary_lto,
tree caller,
- int ecf_flags)
+ cgraph_edge *e,
+ int caller_ecf_flags,
+ int callee_ecf_flags,
+ bool binds_to_current_def)
{
escape_entry *ee;
unsigned int i;
bool changed = false;
- bool ignore_stores = ignore_stores_p (caller, ecf_flags);
+ bool ignore_stores = ignore_stores_p (caller, callee_ecf_flags);
/* If we have no useful info to propagate. */
if ((!cur_summary || !cur_summary->arg_flags.length ())
@@ -4055,6 +4160,8 @@ modref_merge_call_site_flags (escape_summary *sum,
{
int flags = 0;
int flags_lto = 0;
+ /* Returning the value is already accounted to at local propagation. */
+ int implicit_flags = EAF_NOT_RETURNED | EAF_NOT_RETURNED_DIRECTLY;
if (summary && ee->arg < summary->arg_flags.length ())
flags = summary->arg_flags[ee->arg];
@@ -4066,14 +4173,43 @@ modref_merge_call_site_flags (escape_summary *sum,
flags = deref_flags (flags, ignore_stores);
flags_lto = deref_flags (flags_lto, ignore_stores);
}
- else if (ignore_stores)
+ if (ignore_stores)
+ implicit_flags |= ignore_stores_eaf_flags;
+ if (callee_ecf_flags & ECF_PURE)
+ implicit_flags |= implicit_pure_eaf_flags;
+ if (callee_ecf_flags & (ECF_CONST | ECF_NOVOPS))
+ implicit_flags |= implicit_const_eaf_flags;
+ class fnspec_summary *fnspec_sum = fnspec_summaries->get (e);
+ if (fnspec_sum)
{
- flags |= ignore_stores_eaf_flags;
- flags_lto |= ignore_stores_eaf_flags;
+ attr_fnspec fnspec (fnspec_sum->fnspec);
+ int fnspec_flags = 0;
+
+ if (fnspec.arg_specified_p (ee->arg))
+ {
+ if (!fnspec.arg_used_p (ee->arg))
+ fnspec_flags = EAF_UNUSED;
+ else
+ {
+ if (fnspec.arg_direct_p (ee->arg))
+ fnspec_flags |= EAF_DIRECT;
+ if (fnspec.arg_noescape_p (ee->arg))
+ fnspec_flags |= EAF_NOESCAPE | EAF_NODIRECTESCAPE;
+ if (fnspec.arg_readonly_p (ee->arg))
+ fnspec_flags |= EAF_NOCLOBBER;
+ }
+ }
+ implicit_flags |= fnspec_flags;
+ }
+ if (!ee->direct)
+ implicit_flags = deref_flags (implicit_flags, ignore_stores);
+ flags |= implicit_flags;
+ flags_lto |= implicit_flags;
+ if (!binds_to_current_def && (flags || flags_lto))
+ {
+ flags = interposable_eaf_flags (flags, implicit_flags);
+ flags_lto = interposable_eaf_flags (flags_lto, implicit_flags);
}
- /* Returning the value is already accounted to at local propagation. */
- flags |= ee->min_flags | EAF_NOT_RETURNED | EAF_NOT_RETURNED_DIRECTLY;
- flags_lto |= ee->min_flags | EAF_NOT_RETURNED | EAF_NOT_RETURNED_DIRECTLY;
/* Noescape implies that value also does not escape directly.
Fnspec machinery does set both so compensate for this. */
if (flags & EAF_NOESCAPE)
@@ -4095,7 +4231,7 @@ modref_merge_call_site_flags (escape_summary *sum,
if ((f & flags) != f)
{
f = remove_useless_eaf_flags
- (f & flags, ecf_flags,
+ (f & flags, caller_ecf_flags,
VOID_TYPE_P (TREE_TYPE (TREE_TYPE (caller))));
changed = true;
}
@@ -4112,7 +4248,7 @@ modref_merge_call_site_flags (escape_summary *sum,
if ((f & flags_lto) != f)
{
f = remove_useless_eaf_flags
- (f & flags_lto, ecf_flags,
+ (f & flags_lto, caller_ecf_flags,
VOID_TYPE_P (TREE_TYPE (TREE_TYPE (caller))));
changed = true;
}
@@ -4146,6 +4282,7 @@ modref_propagate_flags_in_scc (cgraph_node *component_node)
if (!cur_summary && !cur_summary_lto)
continue;
+ int caller_ecf_flags = flags_from_decl_or_type (cur->decl);
if (dump_file)
fprintf (dump_file, " Processing %s%s%s\n",
@@ -4164,7 +4301,11 @@ modref_propagate_flags_in_scc (cgraph_node *component_node)
changed |= modref_merge_call_site_flags
(sum, cur_summary, cur_summary_lto,
NULL, NULL,
- node->decl, e->indirect_info->ecf_flags);
+ node->decl,
+ e,
+ caller_ecf_flags,
+ e->indirect_info->ecf_flags,
+ false);
}
if (!cur_summary && !cur_summary_lto)
@@ -4216,7 +4357,11 @@ modref_propagate_flags_in_scc (cgraph_node *component_node)
changed |= modref_merge_call_site_flags
(sum, cur_summary, cur_summary_lto,
callee_summary, callee_summary_lto,
- node->decl, ecf_flags);
+ node->decl,
+ callee_edge,
+ caller_ecf_flags,
+ ecf_flags,
+ callee->binds_to_current_def_p ());
if (dump_file && changed)
{
if (cur_summary)
diff --git a/gcc/ipa-modref.h b/gcc/ipa-modref.h
index ddc86869069..20170a65ded 100644
--- a/gcc/ipa-modref.h
+++ b/gcc/ipa-modref.h
@@ -58,4 +58,33 @@ static const int implicit_pure_eaf_flags = EAF_NOCLOBBER | EAF_NOESCAPE
static const int ignore_stores_eaf_flags = EAF_DIRECT | EAF_NOCLOBBER | EAF_NOESCAPE
| EAF_NODIRECTESCAPE;
+/* If function does not bind to current def (i.e. it is inline in comdat
+ section), the modref analysis may not match the behaviour of function
+ which will be later symbol interposed to. All side effects must match
+ however it is possible that the other function body contains more loads
+ which may trap.
+ MODREF_FLAGS are flags determined by analysis of function body while
+ FLAGS are flags known otherwise (i.e. by fnspec, pure/const attributes
+ etc.) */
+static inline int
+interposable_eaf_flags (int modref_flags, int flags)
+{
+ /* If parameter was previously unused, we know it is only read
+ and its value is not used. */
+ if ((modref_flags & EAF_UNUSED) && !(flags & EAF_UNUSED))
+ {
+ modref_flags &= ~EAF_UNUSED;
+ modref_flags |= EAF_NOESCAPE | EAF_NOT_RETURNED
+ | EAF_NOT_RETURNED_DIRECTLY | EAF_NOCLOBBER;
+ }
+ /* We can not deterine that value is not read at all. */
+ if ((modref_flags & EAF_NOREAD) && !(flags & EAF_NOREAD))
+ modref_flags &= ~EAF_NOREAD;
+ /* Clear direct flags so we also know that value is possibly read
+ indirectly. */
+ if ((modref_flags & EAF_DIRECT) && !(flags & EAF_DIRECT))
+ modref_flags &= ~EAF_DIRECT;
+ return modref_flags;
+}
+
#endif
^ permalink raw reply [flat|nested] only message in thread
only message in thread, other threads:[~2021-11-07 17:21 UTC | newest]
Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-07 17:21 [gcc r12-4978] Fix inter-procedural EAF flags propagation with respect to !binds_to_current_def_p Jan Hubicka
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).