Hi,
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 nterposable
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 kive up on NOVOPS.
(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 --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..26a719ff40c 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)))
@@ -4039,12 +4029,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 +4048,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 +4061,42 @@ 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;
+ }
+ }
+ if (!ee->direct)
+ fnspec_flags = deref_flags (fnspec_flags, ignore_stores);
+ implicit_flags |= fnspec_flags;
+ }
+ 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 +4118,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 +4135,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 +4169,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 +4188,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 +4244,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
