| Message ID | 20201109131632.GA12394@kam.mff.cuni.cz |
|---|---|
| State | New |
| Headers | show |
| Series | Detect EAF flags in ipa-modref | expand |
Hi,
this is updated patch for autodetection of EAF flags. Still the goal is
to avoid fancy stuff and get besic logic in place (so no dataflow, no IPA
propagation, no attempts to handle trickier cases). There is one new failure
./gcc/testsuite/gcc/gcc.sum:FAIL: gcc.dg/sso/t2.c -Wno-scalar-storage-order -O1 -fno-inline output pattern test
./gcc/testsuite/gcc/gcc.sum:FAIL: gcc.dg/sso/t2.c -Wno-scalar-storage-order -O2 output pattern test
./gcc/testsuite/gcc/gcc.sum:FAIL: gcc.dg/sso/t2.c -Wno-scalar-storage-order -Os output pattern test
Which I blieve is bug exposed by detecting dump function to be EAF_DIRECT and
NOESCAPE (which it is) and packing/updacking the bitfields leads in one bit
difference. Still no idea why.
Patch seems to be quite effective on cc1plus turning:
Alias oracle query stats:
refs_may_alias_p: 65808750 disambiguations, 75664890 queries
ref_maybe_used_by_call_p: 153485 disambiguations, 66711204 queries
call_may_clobber_ref_p: 22816 disambiguations, 28889 queries
nonoverlapping_component_refs_p: 0 disambiguations, 36846 queries
nonoverlapping_refs_since_match_p: 27271 disambiguations, 58917 must overlaps, 86958 queries
aliasing_component_refs_p: 65808 disambiguations, 2067256 queries
TBAA oracle: 25929211 disambiguations 60395141 queries
12391384 are in alias set 0
10783783 queries asked about the same object
126 queries asked about the same alias set
0 access volatile
9598698 are dependent in the DAG
1691939 are aritificially in conflict with void *
Modref stats:
modref use: 14284 disambiguations, 53336 queries
modref clobber: 1660281 disambiguations, 2130440 queries
4311165 tbaa queries (2.023603 per modref query)
685304 base compares (0.321673 per modref query)
PTA query stats:
pt_solution_includes: 959190 disambiguations, 13169678 queries
pt_solutions_intersect: 1050969 disambiguations, 13246686 queries
Alias oracle query stats:
refs_may_alias_p: 66914578 disambiguations, 76692648 queries
ref_maybe_used_by_call_p: 244077 disambiguations, 67732086 queries
call_may_clobber_ref_p: 111475 disambiguations, 116613 queries
nonoverlapping_component_refs_p: 0 disambiguations, 37091 queries
nonoverlapping_refs_since_match_p: 27267 disambiguations, 59019 must overlaps, 87056 queries
aliasing_component_refs_p: 65870 disambiguations, 2063394 queries
TBAA oracle: 26024415 disambiguations 60579490 queries
12450910 are in alias set 0
10806673 queries asked about the same object
125 queries asked about the same alias set
0 access volatile
9605837 are dependent in the DAG
1691530 are aritificially in conflict with void *
Modref stats:
modref use: 14272 disambiguations, 277680 queries
modref clobber: 1669753 disambiguations, 7849135 queries
4330162 tbaa queries (0.551674 per modref query)
699241 base compares (0.089085 per modref query)
PTA query stats:
pt_solution_includes: 1833920 disambiguations, 13846032 queries
pt_solutions_intersect: 1093785 disambiguations, 13309954 queries
So almost twice as many pt_solution_includes disambiguations.
I will re-run the stats overnight to be sure that it is not independent
change (but both build was from almost same checkout).
Bootstrapped/regtested x86_64-linux, OK?
(I will analyze more the t2.c failure)
Honza
gcc/ChangeLog:
2020-11-10 Jan Hubicka <hubicka@ucw.cz>
* gimple.c: Include ipa-modref-tree.h and ipa-modref.h.
(gimple_call_arg_flags): Use modref to determine flags.
* ipa-modref.c: Include gimple-ssa.h, tree-phinodes.h,
tree-ssa-operands.h, stringpool.h and tree-ssanames.h.
(analyze_ssa_name_flags): Declare.
(modref_summary::useful_p): Summary is also useful if arg flags are
known.
(dump_eaf_flags): New function.
(modref_summary::dump): Use it.
(get_modref_function_summary): Be read for current_function_decl
being NULL.
(memory_access_to): New function.
(deref_flags): New function.
(call_lhs_flags): New function.
(analyze_parms): New function.
(analyze_function): Use it.
* ipa-modref.h (struct modref_summary): Add arg_flags.
gcc/testsuite/ChangeLog:
2020-11-10 Jan Hubicka <hubicka@ucw.cz>
* gcc.dg/torture/pta-ptrarith-1.c: Escape parametrs.
diff --git a/gcc/gimple.c b/gcc/gimple.c
index 1afed88e1f1..da90716aa23 100644
--- a/gcc/gimple.c
+++ b/gcc/gimple.c
@@ -46,6 +46,8 @@ along with GCC; see the file COPYING3. If not see
#include "asan.h"
#include "langhooks.h"
#include "attr-fnspec.h"
+#include "ipa-modref-tree.h"
+#include "ipa-modref.h"
/* All the tuples have their operand vector (if present) at the very bottom
@@ -1532,24 +1534,45 @@ int
gimple_call_arg_flags (const gcall *stmt, unsigned arg)
{
attr_fnspec fnspec = gimple_call_fnspec (stmt);
-
- if (!fnspec.known_p ())
- return 0;
-
int flags = 0;
- if (!fnspec.arg_specified_p (arg))
- ;
- else if (!fnspec.arg_used_p (arg))
- flags = EAF_UNUSED;
- else
+ if (fnspec.known_p ())
{
- if (fnspec.arg_direct_p (arg))
- flags |= EAF_DIRECT;
- if (fnspec.arg_noescape_p (arg))
- flags |= EAF_NOESCAPE;
- if (fnspec.arg_readonly_p (arg))
- flags |= EAF_NOCLOBBER;
+ if (!fnspec.arg_specified_p (arg))
+ ;
+ else if (!fnspec.arg_used_p (arg))
+ flags = EAF_UNUSED;
+ else
+ {
+ if (fnspec.arg_direct_p (arg))
+ flags |= EAF_DIRECT;
+ if (fnspec.arg_noescape_p (arg))
+ flags |= EAF_NOESCAPE;
+ if (fnspec.arg_readonly_p (arg))
+ flags |= EAF_NOCLOBBER;
+ }
+ }
+ tree callee = gimple_call_fndecl (stmt);
+ if (callee)
+ {
+ cgraph_node *node = cgraph_node::get (callee);
+ modref_summary *summary = node ? get_modref_function_summary (node)
+ : NULL;
+
+ if (summary && summary->arg_flags.length () > arg)
+ {
+ int modref_flags = summary->arg_flags[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;
+ if ((modref_flags & EAF_DIRECT) && !(flags & EAF_DIRECT))
+ modref_flags &= ~EAF_DIRECT;
+ }
+ flags |= modref_flags;
+ }
}
return flags;
}
diff --git a/gcc/ipa-modref.c b/gcc/ipa-modref.c
index 3f46bebed3c..bde626115ff 100644
--- a/gcc/ipa-modref.c
+++ b/gcc/ipa-modref.c
@@ -61,6 +61,14 @@ along with GCC; see the file COPYING3. If not see
#include "ipa-fnsummary.h"
#include "attr-fnspec.h"
#include "symtab-clones.h"
+#include "gimple-ssa.h"
+#include "tree-phinodes.h"
+#include "tree-ssa-operands.h"
+#include "ssa-iterators.h"
+#include "stringpool.h"
+#include "tree-ssanames.h"
+
+static int analyze_ssa_name_flags (tree name, vec<int> *known_flags, int depth);
/* We record fnspec specifiers for call edges since they depends on actual
gimple statements. */
@@ -186,6 +194,8 @@ modref_summary::useful_p (int ecf_flags)
{
if (ecf_flags & (ECF_CONST | ECF_NOVOPS))
return false;
+ if (arg_flags.length ())
+ return true;
if (loads && !loads->every_base)
return true;
if (ecf_flags & ECF_PURE)
@@ -355,6 +365,22 @@ dump_lto_records (modref_records_lto *tt, FILE *out)
}
}
+/* Dump EAF flags. */
+
+static void
+dump_eaf_flags (FILE *out, int flags)
+{
+ if (flags & EAF_DIRECT)
+ fprintf (out, " direct");
+ if (flags & EAF_NOCLOBBER)
+ fprintf (out, " noclobber");
+ if (flags & EAF_NOESCAPE)
+ fprintf (out, " noescape");
+ if (flags & EAF_UNUSED)
+ fprintf (out, " unused");
+ fprintf (out, "\n");
+}
+
/* Dump summary. */
void
@@ -372,6 +398,15 @@ modref_summary::dump (FILE *out)
}
if (writes_errno)
fprintf (out, " Writes errno\n");
+ if (arg_flags.length ())
+ {
+ for (unsigned int i = 0; i < arg_flags.length (); i++)
+ if (arg_flags[i])
+ {
+ fprintf (out, " parm %i flags:", i);
+ dump_eaf_flags (out, arg_flags[i]);
+ }
+ }
}
/* Dump summary. */
@@ -402,7 +437,8 @@ get_modref_function_summary (cgraph_node *func)
function. */
enum availability avail;
func = func->function_or_virtual_thunk_symbol
- (&avail, cgraph_node::get (current_function_decl));
+ (&avail, current_function_decl ?
+ cgraph_node::get (current_function_decl) : NULL);
if (avail <= AVAIL_INTERPOSABLE)
return NULL;
@@ -1067,6 +1103,315 @@ remove_summary (bool lto, bool nolto, bool ipa)
" - modref done with result: not tracked.\n");
}
+/* Return true if OP accesses memory pointed to by SSA_NAME. */
+
+bool
+memory_access_to (tree op, tree ssa_name)
+{
+ tree base = get_base_address (op);
+ if (!base)
+ return false;
+ if (TREE_CODE (base) != MEM_REF && TREE_CODE (base) != TARGET_MEM_REF)
+ return false;
+ return TREE_OPERAND (base, 0) == ssa_name;
+}
+
+/* Consider statement val = *arg.
+ return EAF flags of ARG that can be determined from EAF flags of VAL
+ (which are known to be FLAGS). If IGNORE_STORES is true we can ignore
+ all stores to VAL, i.e. when handling noreturn function. */
+
+static int
+deref_flags (int flags, bool ignore_stores)
+{
+ int ret = 0;
+ if (flags & EAF_UNUSED)
+ ret |= EAF_DIRECT | EAF_NOCLOBBER | EAF_NOESCAPE;
+ else
+ {
+ if ((flags & EAF_NOCLOBBER) || ignore_stores)
+ ret |= EAF_NOCLOBBER;
+ if ((flags & EAF_NOESCAPE) || ignore_stores)
+ ret |= EAF_NOESCAPE;
+ }
+ return ret;
+}
+
+/* Call statements may return their parameters. Consider argument number
+ ARG of USE_STMT and determine flags that can needs to be cleared
+ in case pointer possibly indirectly references from ARG I is returned.
+ KNOWN_FLAGS and DEPTH are same as in analyze_ssa_name_flags. */
+
+static int
+call_lhs_flags (gimple *use_stmt, int arg, vec<int> *known_flags, int depth)
+{
+ /* If there is no return value, no flags are affected. */
+ if (!gimple_call_lhs (use_stmt))
+ return EAF_DIRECT | EAF_NOCLOBBER | EAF_NOESCAPE | EAF_UNUSED;
+
+ /* If we know that function returns given argument and it is not ARG
+ we can still be happy. */
+ int flags = gimple_call_return_flags (as_a <gcall *> (use_stmt));
+ if ((flags & ERF_RETURNS_ARG)
+ && (flags & ERF_RETURN_ARG_MASK) != arg)
+ return EAF_DIRECT | EAF_NOCLOBBER | EAF_NOESCAPE | EAF_UNUSED;
+
+ /* If return value is SSA name determine its flags. */
+ if (TREE_CODE (gimple_call_lhs (use_stmt)) == SSA_NAME)
+ return analyze_ssa_name_flags
+ (gimple_call_lhs (use_stmt), known_flags,
+ depth + 1);
+ /* In the case of memory store we can do nothing. */
+ else
+ return 0;
+}
+
+/* Analyze EAF flags for SSA name NAME.
+ KNOWN_FLAGS is a cache for flags we already determined.
+ DEPTH is a recursion depth used to make debug output prettier. */
+
+static int
+analyze_ssa_name_flags (tree name, vec<int> *known_flags, int depth)
+{
+ imm_use_iterator ui;
+ gimple *use_stmt;
+ int flags = EAF_DIRECT | EAF_NOCLOBBER | EAF_NOESCAPE | EAF_UNUSED;
+
+ /* See if value is already computed. */
+ if ((*known_flags)[SSA_NAME_VERSION (name)])
+ {
+ /* Punt on cycles for now, so we do not need dataflow. */
+ if ((*known_flags)[SSA_NAME_VERSION (name)] == 1)
+ {
+ if (dump_file)
+ fprintf (dump_file,
+ "%*sGiving up on a cycle in SSA graph\n", depth * 4, "");
+ return 0;
+ }
+ return (*known_flags)[SSA_NAME_VERSION (name)] - 2;
+ }
+ /* Recursion guard. */
+ (*known_flags)[SSA_NAME_VERSION (name)] = 1;
+
+ if (dump_file)
+ {
+ fprintf (dump_file,
+ "%*sAnalyzing flags of ssa name: ", depth * 4, "");
+ print_generic_expr (dump_file, name);
+ fprintf (dump_file, "\n");
+ }
+
+ FOR_EACH_IMM_USE_STMT (use_stmt, ui, name)
+ {
+ if (flags == 0)
+ {
+ BREAK_FROM_IMM_USE_STMT (ui);
+ }
+ if (is_gimple_debug (use_stmt))
+ continue;
+ if (dump_file)
+ {
+ fprintf (dump_file, "%*s Analyzing stmt:", depth * 4, "");
+ print_gimple_stmt (dump_file, use_stmt, 0);
+ }
+
+ /* Gimple return may load the return value. */
+ if (gimple_code (use_stmt) == GIMPLE_RETURN)
+ {
+ if (memory_access_to (gimple_return_retval
+ (as_a <greturn *> (use_stmt)), name))
+ flags &= ~EAF_UNUSED;
+ }
+ /* Account for LHS store, arg loads and flags from callee function. */
+ else if (is_gimple_call (use_stmt))
+ {
+ tree callee = gimple_call_fndecl (use_stmt);
+
+ /* Recursion would require bit of propagation; give up for now. */
+ if (callee && recursive_call_p (current_function_decl, callee))
+ flags = 0;
+ else
+ {
+ int ecf_flags = gimple_call_flags (use_stmt);
+ bool ignore_stores = ignore_stores_p (current_function_decl,
+ ecf_flags);
+
+ /* Handle *name = func (...). */
+ if (gimple_call_lhs (use_stmt)
+ && memory_access_to (gimple_call_lhs (use_stmt), name))
+ flags &= ~(EAF_UNUSED | EAF_NOCLOBBER);
+
+ /* Handle all function parameters. */
+ for (unsigned i = 0; i < gimple_call_num_args (use_stmt); i++)
+ /* Name is directly passed to the callee. */
+ if (gimple_call_arg (use_stmt, i) == name)
+ {
+ if (ecf_flags & (ECF_CONST | ECF_NOVOPS))
+ flags &= ignore_stores
+ ? 0
+ : call_lhs_flags (use_stmt, i,
+ known_flags, depth);
+ else
+ {
+ int call_flags = gimple_call_arg_flags (as_a <gcall *>
+ (use_stmt), i);
+ if (ignore_stores)
+ call_flags |= EAF_NOCLOBBER | EAF_NOESCAPE;
+ else
+ call_flags &= call_lhs_flags (use_stmt, i,
+ known_flags, depth);
+
+ flags &= call_flags;
+ }
+ }
+ /* Name is dereferenced and passed to a callee. */
+ else if (memory_access_to (gimple_call_arg (use_stmt, i), name))
+ {
+ if (ecf_flags & (ECF_CONST | ECF_NOVOPS))
+ flags &= ~EAF_UNUSED;
+ else
+ flags &= deref_flags (gimple_call_arg_flags
+ (as_a <gcall *> (use_stmt), i),
+ ignore_stores);
+ if (!ignore_stores)
+ flags &= call_lhs_flags (use_stmt, i, known_flags,
+ depth);
+ }
+ }
+ /* Only NOCLOBBER or DIRECT flags alone are not useful (see comments
+ in tree-ssa-alias.c). Give up earlier. */
+ if ((flags & ~(EAF_DIRECT | EAF_NOCLOBBER)) == 0)
+ flags = 0;
+ }
+ else if (gimple_assign_load_p (use_stmt))
+ {
+ /* Memory to memory copy. */
+ if (gimple_store_p (use_stmt))
+ {
+ /* Handle *name = *exp. */
+ if (memory_access_to (gimple_assign_lhs (use_stmt), name))
+ flags &= ~(EAF_UNUSED | EAF_NOCLOBBER);
+
+ /* Handle *lhs = *name.
+
+ We do not track memory locations, so assume that value
+ is used arbitrarily. */
+ if (memory_access_to (gimple_assign_rhs1 (use_stmt), name))
+ flags = 0;
+ }
+ /* Handle lhs = *name. */
+ else if (memory_access_to (gimple_assign_rhs1 (use_stmt), name))
+ flags &= deref_flags (analyze_ssa_name_flags
+ (gimple_assign_lhs (use_stmt),
+ known_flags, depth + 1), false);
+ }
+ else if (gimple_store_p (use_stmt))
+ {
+ /* Handle *lhs = name. */
+ if (is_gimple_assign (use_stmt)
+ && gimple_assign_rhs1 (use_stmt) == name)
+ {
+ if (dump_file)
+ fprintf (dump_file, "%*s ssa name saved to memory\n",
+ depth * 4, "");
+ flags = 0;
+ }
+ /* Handle *name = exp. */
+ else if (is_gimple_assign (use_stmt)
+ && memory_access_to (gimple_assign_lhs (use_stmt), name))
+ flags &= ~(EAF_UNUSED | EAF_NOCLOBBER);
+ /* ASM statements etc. */
+ else if (!is_gimple_assign (use_stmt))
+ {
+ if (dump_file)
+ fprintf (dump_file, "%*s Unhandled store\n",
+ depth * 4, "");
+ flags = 0;
+ }
+ }
+ else if (is_gimple_assign (use_stmt))
+ {
+ enum tree_code code = gimple_assign_rhs_code (use_stmt);
+
+ /* See if operation is a merge as considered by
+ tree-ssa-structalias.c:find_func_aliases. */
+ if (!truth_value_p (code)
+ && code != POINTER_DIFF_EXPR
+ && (code != POINTER_PLUS_EXPR
+ || gimple_assign_rhs1 (use_stmt) == name))
+ flags &= analyze_ssa_name_flags
+ (gimple_assign_lhs (use_stmt), known_flags,
+ depth + 1);
+ }
+ else if (gimple_code (use_stmt) == GIMPLE_PHI)
+ {
+ flags &= analyze_ssa_name_flags
+ (gimple_phi_result (use_stmt), known_flags,
+ depth + 1);
+ }
+ /* Conditions are not considered escape points
+ by tree-ssa-structalias. */
+ else if (gimple_code (use_stmt) == GIMPLE_COND)
+ ;
+ else
+ {
+ if (dump_file)
+ fprintf (dump_file, "%*s Unhandled stmt\n", depth * 4, "");
+ flags = 0;
+ }
+
+ if (dump_file)
+ {
+ fprintf (dump_file, "%*s current flags of ", depth * 4, "");
+ print_generic_expr (dump_file, name);
+ dump_eaf_flags (dump_file, flags);
+ }
+ }
+ if (dump_file)
+ {
+ fprintf (dump_file, "%*sflags of ssa name ", depth * 4, "");
+ print_generic_expr (dump_file, name);
+ dump_eaf_flags (dump_file, flags);
+ }
+ (*known_flags)[SSA_NAME_VERSION (name)] = flags + 2;
+ return flags;
+}
+
+/* Determine EAF flags for function parameters. */
+
+static void
+analyze_parms (modref_summary *summary)
+{
+ unsigned int parm_index = 0;
+ unsigned int count = 0;
+
+ for (tree parm = DECL_ARGUMENTS (current_function_decl); parm;
+ parm = TREE_CHAIN (parm))
+ count++;
+
+ if (!count)
+ return;
+
+ auto_vec<int> known_flags;
+ known_flags.safe_grow_cleared (num_ssa_names);
+
+ for (tree parm = DECL_ARGUMENTS (current_function_decl); parm; parm_index++,
+ parm = TREE_CHAIN (parm))
+ {
+ tree name = ssa_default_def (cfun, parm);
+ if (!name)
+ continue;
+ int flags = analyze_ssa_name_flags (name, &known_flags, 0);
+
+ if (flags)
+ {
+ if (parm_index >= summary->arg_flags.length ())
+ summary->arg_flags.safe_grow_cleared (count);
+ summary->arg_flags[parm_index] = flags;
+ }
+ }
+}
+
/* Analyze function F. IPA indicates whether we're running in local mode
(false) or the IPA mode (true). */
@@ -1174,6 +1519,10 @@ analyze_function (function *f, bool ipa)
param_modref_max_accesses);
summary_lto->writes_errno = false;
}
+
+ if (!ipa)
+ analyze_parms (summary);
+
int ecf_flags = flags_from_decl_or_type (current_function_decl);
auto_vec <gimple *, 32> recursive_calls;
@@ -1191,8 +1540,9 @@ analyze_function (function *f, bool ipa)
|| ((!summary || !summary->useful_p (ecf_flags))
&& (!summary_lto || !summary_lto->useful_p (ecf_flags))))
{
- remove_summary (lto, nolto, ipa);
- return;
+ collapse_loads (summary, summary_lto);
+ collapse_stores (summary, summary_lto);
+ break;
}
}
}
diff --git a/gcc/ipa-modref.h b/gcc/ipa-modref.h
index 31ceffa8d34..8fa05aaf7fb 100644
--- a/gcc/ipa-modref.h
+++ b/gcc/ipa-modref.h
@@ -29,6 +29,7 @@ struct GTY(()) modref_summary
/* Load and stores in function (transitively closed to all callees) */
modref_records *loads;
modref_records *stores;
+ auto_vec<int> GTY((skip)) arg_flags;
modref_summary ();
~modref_summary ();
diff --git a/gcc/testsuite/gcc.dg/torture/pta-ptrarith-1.c b/gcc/testsuite/gcc.dg/torture/pta-ptrarith-1.c
index 99a548840df..85b68068b12 100644
--- a/gcc/testsuite/gcc.dg/torture/pta-ptrarith-1.c
+++ b/gcc/testsuite/gcc.dg/torture/pta-ptrarith-1.c
@@ -6,11 +6,14 @@ struct Foo {
int *p;
};
+struct Foo *ff;
+
void __attribute__((noinline))
foo (void *p)
{
struct Foo *f = (struct Foo *)p - 1;
*f->p = 0;
+ ff = f;
}
int bar (void)
> > Alias oracle query stats: > refs_may_alias_p: 65808750 disambiguations, 75664890 queries > ref_maybe_used_by_call_p: 153485 disambiguations, 66711204 queries > call_may_clobber_ref_p: 22816 disambiguations, 28889 queries > nonoverlapping_component_refs_p: 0 disambiguations, 36846 queries > nonoverlapping_refs_since_match_p: 27271 disambiguations, 58917 must overlaps, 86958 queries > aliasing_component_refs_p: 65808 disambiguations, 2067256 queries > TBAA oracle: 25929211 disambiguations 60395141 queries > 12391384 are in alias set 0 > 10783783 queries asked about the same object > 126 queries asked about the same alias set > 0 access volatile > 9598698 are dependent in the DAG > 1691939 are aritificially in conflict with void * > > Modref stats: > modref use: 14284 disambiguations, 53336 queries > modref clobber: 1660281 disambiguations, 2130440 queries > 4311165 tbaa queries (2.023603 per modref query) > 685304 base compares (0.321673 per modref query) > > PTA query stats: > pt_solution_includes: 959190 disambiguations, 13169678 queries > pt_solutions_intersect: 1050969 disambiguations, 13246686 queries I re-run the mainline withtout EAF flag propagation and the results are correct, so it is not due to independent change pushed between my tests. Honza
> Bootstrapped/regtested x86_64-linux, OK? > (I will analyze more the t2.c failure) I have found independent reproducer that is now in https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97775 Honza
> > Bootstrapped/regtested x86_64-linux, OK? > > (I will analyze more the t2.c failure) > > I have found independent reproducer that is now in > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97775 ... and with Jakub's fix the testcase works now. Honza > > Honza
On Tue, 10 Nov 2020, Jan Hubicka wrote: > Hi, > this is updated patch for autodetection of EAF flags. Still the goal is > to avoid fancy stuff and get besic logic in place (so no dataflow, no IPA > propagation, no attempts to handle trickier cases). There is one new failure > > ./gcc/testsuite/gcc/gcc.sum:FAIL: gcc.dg/sso/t2.c -Wno-scalar-storage-order -O1 -fno-inline output pattern test > ./gcc/testsuite/gcc/gcc.sum:FAIL: gcc.dg/sso/t2.c -Wno-scalar-storage-order -O2 output pattern test > ./gcc/testsuite/gcc/gcc.sum:FAIL: gcc.dg/sso/t2.c -Wno-scalar-storage-order -Os output pattern test > > Which I blieve is bug exposed by detecting dump function to be EAF_DIRECT and > NOESCAPE (which it is) and packing/updacking the bitfields leads in one bit > difference. Still no idea why. > > Patch seems to be quite effective on cc1plus turning: > > Alias oracle query stats: > refs_may_alias_p: 65808750 disambiguations, 75664890 queries > ref_maybe_used_by_call_p: 153485 disambiguations, 66711204 queries > call_may_clobber_ref_p: 22816 disambiguations, 28889 queries > nonoverlapping_component_refs_p: 0 disambiguations, 36846 queries > nonoverlapping_refs_since_match_p: 27271 disambiguations, 58917 must overlaps, 86958 queries > aliasing_component_refs_p: 65808 disambiguations, 2067256 queries > TBAA oracle: 25929211 disambiguations 60395141 queries > 12391384 are in alias set 0 > 10783783 queries asked about the same object > 126 queries asked about the same alias set > 0 access volatile > 9598698 are dependent in the DAG > 1691939 are aritificially in conflict with void * > > Modref stats: > modref use: 14284 disambiguations, 53336 queries > modref clobber: 1660281 disambiguations, 2130440 queries > 4311165 tbaa queries (2.023603 per modref query) > 685304 base compares (0.321673 per modref query) > > PTA query stats: > pt_solution_includes: 959190 disambiguations, 13169678 queries > pt_solutions_intersect: 1050969 disambiguations, 13246686 queries > > Alias oracle query stats: > refs_may_alias_p: 66914578 disambiguations, 76692648 queries > ref_maybe_used_by_call_p: 244077 disambiguations, 67732086 queries > call_may_clobber_ref_p: 111475 disambiguations, 116613 queries > nonoverlapping_component_refs_p: 0 disambiguations, 37091 queries > nonoverlapping_refs_since_match_p: 27267 disambiguations, 59019 must overlaps, 87056 queries > aliasing_component_refs_p: 65870 disambiguations, 2063394 queries > TBAA oracle: 26024415 disambiguations 60579490 queries > 12450910 are in alias set 0 > 10806673 queries asked about the same object > 125 queries asked about the same alias set > 0 access volatile > 9605837 are dependent in the DAG > 1691530 are aritificially in conflict with void * > > Modref stats: > modref use: 14272 disambiguations, 277680 queries > modref clobber: 1669753 disambiguations, 7849135 queries > 4330162 tbaa queries (0.551674 per modref query) > 699241 base compares (0.089085 per modref query) > > PTA query stats: > pt_solution_includes: 1833920 disambiguations, 13846032 queries > pt_solutions_intersect: 1093785 disambiguations, 13309954 queries > > So almost twice as many pt_solution_includes disambiguations. > I will re-run the stats overnight to be sure that it is not independent > change (but both build was from almost same checkout). > > Bootstrapped/regtested x86_64-linux, OK? > (I will analyze more the t2.c failure) > > Honza > > gcc/ChangeLog: > > 2020-11-10 Jan Hubicka <hubicka@ucw.cz> > > * gimple.c: Include ipa-modref-tree.h and ipa-modref.h. > (gimple_call_arg_flags): Use modref to determine flags. > * ipa-modref.c: Include gimple-ssa.h, tree-phinodes.h, > tree-ssa-operands.h, stringpool.h and tree-ssanames.h. > (analyze_ssa_name_flags): Declare. > (modref_summary::useful_p): Summary is also useful if arg flags are > known. > (dump_eaf_flags): New function. > (modref_summary::dump): Use it. > (get_modref_function_summary): Be read for current_function_decl > being NULL. > (memory_access_to): New function. > (deref_flags): New function. > (call_lhs_flags): New function. > (analyze_parms): New function. > (analyze_function): Use it. > * ipa-modref.h (struct modref_summary): Add arg_flags. > > gcc/testsuite/ChangeLog: > > 2020-11-10 Jan Hubicka <hubicka@ucw.cz> > > * gcc.dg/torture/pta-ptrarith-1.c: Escape parametrs. > > diff --git a/gcc/gimple.c b/gcc/gimple.c > index 1afed88e1f1..da90716aa23 100644 > --- a/gcc/gimple.c > +++ b/gcc/gimple.c > @@ -46,6 +46,8 @@ along with GCC; see the file COPYING3. If not see > #include "asan.h" > #include "langhooks.h" > #include "attr-fnspec.h" > +#include "ipa-modref-tree.h" > +#include "ipa-modref.h" > > > /* All the tuples have their operand vector (if present) at the very bottom > @@ -1532,24 +1534,45 @@ int > gimple_call_arg_flags (const gcall *stmt, unsigned arg) > { > attr_fnspec fnspec = gimple_call_fnspec (stmt); > - > - if (!fnspec.known_p ()) > - return 0; > - > int flags = 0; > > - if (!fnspec.arg_specified_p (arg)) > - ; > - else if (!fnspec.arg_used_p (arg)) > - flags = EAF_UNUSED; > - else > + if (fnspec.known_p ()) > { > - if (fnspec.arg_direct_p (arg)) > - flags |= EAF_DIRECT; > - if (fnspec.arg_noescape_p (arg)) > - flags |= EAF_NOESCAPE; > - if (fnspec.arg_readonly_p (arg)) > - flags |= EAF_NOCLOBBER; > + if (!fnspec.arg_specified_p (arg)) > + ; > + else if (!fnspec.arg_used_p (arg)) > + flags = EAF_UNUSED; > + else > + { > + if (fnspec.arg_direct_p (arg)) > + flags |= EAF_DIRECT; > + if (fnspec.arg_noescape_p (arg)) > + flags |= EAF_NOESCAPE; > + if (fnspec.arg_readonly_p (arg)) > + flags |= EAF_NOCLOBBER; > + } > + } > + tree callee = gimple_call_fndecl (stmt); > + if (callee) > + { > + cgraph_node *node = cgraph_node::get (callee); > + modref_summary *summary = node ? get_modref_function_summary (node) > + : NULL; > + > + if (summary && summary->arg_flags.length () > arg) So could we make modref "transform" push this as fnspec attribute or would that not really be an optimization? > + { > + int modref_flags = summary->arg_flags[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; > + if ((modref_flags & EAF_DIRECT) && !(flags & EAF_DIRECT)) > + modref_flags &= ~EAF_DIRECT; > + } > + flags |= modref_flags; > + } > } > return flags; > } > diff --git a/gcc/ipa-modref.c b/gcc/ipa-modref.c > index 3f46bebed3c..bde626115ff 100644 > --- a/gcc/ipa-modref.c > +++ b/gcc/ipa-modref.c > @@ -61,6 +61,14 @@ along with GCC; see the file COPYING3. If not see > #include "ipa-fnsummary.h" > #include "attr-fnspec.h" > #include "symtab-clones.h" > +#include "gimple-ssa.h" > +#include "tree-phinodes.h" > +#include "tree-ssa-operands.h" > +#include "ssa-iterators.h" > +#include "stringpool.h" > +#include "tree-ssanames.h" > + > +static int analyze_ssa_name_flags (tree name, vec<int> *known_flags, int depth); > > /* We record fnspec specifiers for call edges since they depends on actual > gimple statements. */ > @@ -186,6 +194,8 @@ modref_summary::useful_p (int ecf_flags) > { > if (ecf_flags & (ECF_CONST | ECF_NOVOPS)) > return false; > + if (arg_flags.length ()) > + return true; > if (loads && !loads->every_base) > return true; > if (ecf_flags & ECF_PURE) > @@ -355,6 +365,22 @@ dump_lto_records (modref_records_lto *tt, FILE *out) > } > } > > +/* Dump EAF flags. */ > + > +static void > +dump_eaf_flags (FILE *out, int flags) > +{ > + if (flags & EAF_DIRECT) > + fprintf (out, " direct"); > + if (flags & EAF_NOCLOBBER) > + fprintf (out, " noclobber"); > + if (flags & EAF_NOESCAPE) > + fprintf (out, " noescape"); > + if (flags & EAF_UNUSED) > + fprintf (out, " unused"); > + fprintf (out, "\n"); > +} > + > /* Dump summary. */ > > void > @@ -372,6 +398,15 @@ modref_summary::dump (FILE *out) > } > if (writes_errno) > fprintf (out, " Writes errno\n"); > + if (arg_flags.length ()) > + { > + for (unsigned int i = 0; i < arg_flags.length (); i++) > + if (arg_flags[i]) > + { > + fprintf (out, " parm %i flags:", i); > + dump_eaf_flags (out, arg_flags[i]); > + } > + } > } > > /* Dump summary. */ > @@ -402,7 +437,8 @@ get_modref_function_summary (cgraph_node *func) > function. */ > enum availability avail; > func = func->function_or_virtual_thunk_symbol > - (&avail, cgraph_node::get (current_function_decl)); > + (&avail, current_function_decl ? > + cgraph_node::get (current_function_decl) : NULL); > if (avail <= AVAIL_INTERPOSABLE) > return NULL; > > @@ -1067,6 +1103,315 @@ remove_summary (bool lto, bool nolto, bool ipa) > " - modref done with result: not tracked.\n"); > } > > +/* Return true if OP accesses memory pointed to by SSA_NAME. */ > + > +bool > +memory_access_to (tree op, tree ssa_name) > +{ > + tree base = get_base_address (op); > + if (!base) > + return false; > + if (TREE_CODE (base) != MEM_REF && TREE_CODE (base) != TARGET_MEM_REF) > + return false; > + return TREE_OPERAND (base, 0) == ssa_name; > +} > + > +/* Consider statement val = *arg. > + return EAF flags of ARG that can be determined from EAF flags of VAL > + (which are known to be FLAGS). If IGNORE_STORES is true we can ignore > + all stores to VAL, i.e. when handling noreturn function. */ > + > +static int > +deref_flags (int flags, bool ignore_stores) > +{ > + int ret = 0; > + if (flags & EAF_UNUSED) > + ret |= EAF_DIRECT | EAF_NOCLOBBER | EAF_NOESCAPE; > + else > + { > + if ((flags & EAF_NOCLOBBER) || ignore_stores) > + ret |= EAF_NOCLOBBER; > + if ((flags & EAF_NOESCAPE) || ignore_stores) > + ret |= EAF_NOESCAPE; > + } > + return ret; > +} > + > +/* Call statements may return their parameters. Consider argument number > + ARG of USE_STMT and determine flags that can needs to be cleared > + in case pointer possibly indirectly references from ARG I is returned. > + KNOWN_FLAGS and DEPTH are same as in analyze_ssa_name_flags. */ > + > +static int > +call_lhs_flags (gimple *use_stmt, int arg, vec<int> *known_flags, int depth) > +{ > + /* If there is no return value, no flags are affected. */ > + if (!gimple_call_lhs (use_stmt)) > + return EAF_DIRECT | EAF_NOCLOBBER | EAF_NOESCAPE | EAF_UNUSED; > + > + /* If we know that function returns given argument and it is not ARG > + we can still be happy. */ > + int flags = gimple_call_return_flags (as_a <gcall *> (use_stmt)); > + if ((flags & ERF_RETURNS_ARG) > + && (flags & ERF_RETURN_ARG_MASK) != arg) > + return EAF_DIRECT | EAF_NOCLOBBER | EAF_NOESCAPE | EAF_UNUSED; > + > + /* If return value is SSA name determine its flags. */ > + if (TREE_CODE (gimple_call_lhs (use_stmt)) == SSA_NAME) > + return analyze_ssa_name_flags > + (gimple_call_lhs (use_stmt), known_flags, > + depth + 1); > + /* In the case of memory store we can do nothing. */ > + else > + return 0; > +} > + > +/* Analyze EAF flags for SSA name NAME. > + KNOWN_FLAGS is a cache for flags we already determined. > + DEPTH is a recursion depth used to make debug output prettier. */ > + > +static int > +analyze_ssa_name_flags (tree name, vec<int> *known_flags, int depth) C++ has references which makes the access to known_flags nicer ;) > +{ > + imm_use_iterator ui; > + gimple *use_stmt; > + int flags = EAF_DIRECT | EAF_NOCLOBBER | EAF_NOESCAPE | EAF_UNUSED; > + > + /* See if value is already computed. */ > + if ((*known_flags)[SSA_NAME_VERSION (name)]) > + { > + /* Punt on cycles for now, so we do not need dataflow. */ > + if ((*known_flags)[SSA_NAME_VERSION (name)] == 1) > + { > + if (dump_file) > + fprintf (dump_file, > + "%*sGiving up on a cycle in SSA graph\n", depth * 4, ""); > + return 0; > + } > + return (*known_flags)[SSA_NAME_VERSION (name)] - 2; > + } > + /* Recursion guard. */ > + (*known_flags)[SSA_NAME_VERSION (name)] = 1; This also guards against multiple evaluations of the same stmts but only in some cases? Consider _1 = ..; _2 = _1 + _3; _4 = _1 + _5; _6 = _2 + _4; where we visit _2 = and _4 = from _1 but from both are going to visit _6. Maybe I'm blind but you're not limiting depth? Guess that asks for problems, esp. as you are recursing rather than using a worklist or so? I see you try to "optimize" the walk by only visiting def->use links from parameters but then a RPO walk over all stmts would be simpler iteration-wise ... > + if (dump_file) > + { > + fprintf (dump_file, > + "%*sAnalyzing flags of ssa name: ", depth * 4, ""); > + print_generic_expr (dump_file, name); > + fprintf (dump_file, "\n"); > + } > + > + FOR_EACH_IMM_USE_STMT (use_stmt, ui, name) > + { > + if (flags == 0) > + { > + BREAK_FROM_IMM_USE_STMT (ui); > + } > + if (is_gimple_debug (use_stmt)) > + continue; > + if (dump_file) > + { > + fprintf (dump_file, "%*s Analyzing stmt:", depth * 4, ""); > + print_gimple_stmt (dump_file, use_stmt, 0); > + } > + > + /* Gimple return may load the return value. */ > + if (gimple_code (use_stmt) == GIMPLE_RETURN) if (greturn *ret = dyn_cast <greturn *> (use_stmt)) makes the as_a below not needed, similar for the other cases (it also makes accesses cheaper, avoiding some checking code). > + { > + if (memory_access_to (gimple_return_retval > + (as_a <greturn *> (use_stmt)), name)) > + flags &= ~EAF_UNUSED; > + } > + /* Account for LHS store, arg loads and flags from callee function. */ > + else if (is_gimple_call (use_stmt)) > + { > + tree callee = gimple_call_fndecl (use_stmt); > + > + /* Recursion would require bit of propagation; give up for now. */ > + if (callee && recursive_call_p (current_function_decl, callee)) > + flags = 0; > + else > + { > + int ecf_flags = gimple_call_flags (use_stmt); > + bool ignore_stores = ignore_stores_p (current_function_decl, > + ecf_flags); > + > + /* Handle *name = func (...). */ > + if (gimple_call_lhs (use_stmt) > + && memory_access_to (gimple_call_lhs (use_stmt), name)) > + flags &= ~(EAF_UNUSED | EAF_NOCLOBBER); > + > + /* Handle all function parameters. */ > + for (unsigned i = 0; i < gimple_call_num_args (use_stmt); i++) > + /* Name is directly passed to the callee. */ > + if (gimple_call_arg (use_stmt, i) == name) > + { > + if (ecf_flags & (ECF_CONST | ECF_NOVOPS)) > + flags &= ignore_stores > + ? 0 > + : call_lhs_flags (use_stmt, i, > + known_flags, depth); > + else > + { > + int call_flags = gimple_call_arg_flags (as_a <gcall *> > + (use_stmt), i); > + if (ignore_stores) > + call_flags |= EAF_NOCLOBBER | EAF_NOESCAPE; > + else > + call_flags &= call_lhs_flags (use_stmt, i, > + known_flags, depth); > + > + flags &= call_flags; > + } > + } > + /* Name is dereferenced and passed to a callee. */ > + else if (memory_access_to (gimple_call_arg (use_stmt, i), name)) > + { > + if (ecf_flags & (ECF_CONST | ECF_NOVOPS)) > + flags &= ~EAF_UNUSED; > + else > + flags &= deref_flags (gimple_call_arg_flags > + (as_a <gcall *> (use_stmt), i), > + ignore_stores); > + if (!ignore_stores) > + flags &= call_lhs_flags (use_stmt, i, known_flags, > + depth); > + } Hmm, you forget gimple_call_chain at least which is at least argument passing? Possibly the chain argument is never a function parameter but how do you know... (partial inlining?) Then there's gimple_call_fn for indirect function calls to a function parameter? I guess it would be nice to amend the gimple_walk_ stuff to have a SSA name callback where the tree and the SSA use is passed. Well, for another time. > + } > + /* Only NOCLOBBER or DIRECT flags alone are not useful (see comments > + in tree-ssa-alias.c). Give up earlier. */ > + if ((flags & ~(EAF_DIRECT | EAF_NOCLOBBER)) == 0) > + flags = 0; > + } > + else if (gimple_assign_load_p (use_stmt)) > + { > + /* Memory to memory copy. */ > + if (gimple_store_p (use_stmt)) > + { > + /* Handle *name = *exp. */ > + if (memory_access_to (gimple_assign_lhs (use_stmt), name)) > + flags &= ~(EAF_UNUSED | EAF_NOCLOBBER); > + > + /* Handle *lhs = *name. > + > + We do not track memory locations, so assume that value > + is used arbitrarily. */ > + if (memory_access_to (gimple_assign_rhs1 (use_stmt), name)) > + flags = 0; > + } > + /* Handle lhs = *name. */ > + else if (memory_access_to (gimple_assign_rhs1 (use_stmt), name)) > + flags &= deref_flags (analyze_ssa_name_flags > + (gimple_assign_lhs (use_stmt), > + known_flags, depth + 1), false); > + } > + else if (gimple_store_p (use_stmt)) > + { > + /* Handle *lhs = name. */ > + if (is_gimple_assign (use_stmt) > + && gimple_assign_rhs1 (use_stmt) == name) > + { > + if (dump_file) > + fprintf (dump_file, "%*s ssa name saved to memory\n", > + depth * 4, ""); > + flags = 0; > + } > + /* Handle *name = exp. */ > + else if (is_gimple_assign (use_stmt) > + && memory_access_to (gimple_assign_lhs (use_stmt), name)) > + flags &= ~(EAF_UNUSED | EAF_NOCLOBBER); > + /* ASM statements etc. */ > + else if (!is_gimple_assign (use_stmt)) > + { > + if (dump_file) > + fprintf (dump_file, "%*s Unhandled store\n", > + depth * 4, ""); > + flags = 0; > + } > + } > + else if (is_gimple_assign (use_stmt)) > + { > + enum tree_code code = gimple_assign_rhs_code (use_stmt); > + > + /* See if operation is a merge as considered by > + tree-ssa-structalias.c:find_func_aliases. */ > + if (!truth_value_p (code) > + && code != POINTER_DIFF_EXPR > + && (code != POINTER_PLUS_EXPR > + || gimple_assign_rhs1 (use_stmt) == name)) > + flags &= analyze_ssa_name_flags > + (gimple_assign_lhs (use_stmt), known_flags, > + depth + 1); > + } > + else if (gimple_code (use_stmt) == GIMPLE_PHI) > + { > + flags &= analyze_ssa_name_flags > + (gimple_phi_result (use_stmt), known_flags, > + depth + 1); > + } > + /* Conditions are not considered escape points > + by tree-ssa-structalias. */ > + else if (gimple_code (use_stmt) == GIMPLE_COND) > + ; > + else > + { > + if (dump_file) > + fprintf (dump_file, "%*s Unhandled stmt\n", depth * 4, ""); > + flags = 0; > + } > + > + if (dump_file) > + { > + fprintf (dump_file, "%*s current flags of ", depth * 4, ""); > + print_generic_expr (dump_file, name); > + dump_eaf_flags (dump_file, flags); > + } > + } > + if (dump_file) > + { > + fprintf (dump_file, "%*sflags of ssa name ", depth * 4, ""); > + print_generic_expr (dump_file, name); > + dump_eaf_flags (dump_file, flags); > + } > + (*known_flags)[SSA_NAME_VERSION (name)] = flags + 2; > + return flags; > +} > + > +/* Determine EAF flags for function parameters. */ > + > +static void > +analyze_parms (modref_summary *summary) > +{ > + unsigned int parm_index = 0; > + unsigned int count = 0; > + > + for (tree parm = DECL_ARGUMENTS (current_function_decl); parm; > + parm = TREE_CHAIN (parm)) > + count++; > + > + if (!count) > + return; > + > + auto_vec<int> known_flags; > + known_flags.safe_grow_cleared (num_ssa_names); , true for exact reserve. Could be space optimized by not using auto_vec<int> but auto_vec<usigned char>? > + > + for (tree parm = DECL_ARGUMENTS (current_function_decl); parm; parm_index++, > + parm = TREE_CHAIN (parm)) > + { > + tree name = ssa_default_def (cfun, parm); > + if (!name) > + continue; looks like the vec might be quite sparse ... > + int flags = analyze_ssa_name_flags (name, &known_flags, 0); > + > + if (flags) > + { > + if (parm_index >= summary->arg_flags.length ()) > + summary->arg_flags.safe_grow_cleared (count); you want , true for exact reserve. > + summary->arg_flags[parm_index] = flags; maybe this as well - does it make sense to skip !is_gimple_reg_type params in the counting? Guess it makes lookup too complicated. But we do have testcases with >30000 parameters ... > + } > + } > +} > + > /* Analyze function F. IPA indicates whether we're running in local mode > (false) or the IPA mode (true). */ > > @@ -1174,6 +1519,10 @@ analyze_function (function *f, bool ipa) > param_modref_max_accesses); > summary_lto->writes_errno = false; > } > + > + if (!ipa) > + analyze_parms (summary); > + > int ecf_flags = flags_from_decl_or_type (current_function_decl); > auto_vec <gimple *, 32> recursive_calls; > > @@ -1191,8 +1540,9 @@ analyze_function (function *f, bool ipa) > || ((!summary || !summary->useful_p (ecf_flags)) > && (!summary_lto || !summary_lto->useful_p (ecf_flags)))) > { > - remove_summary (lto, nolto, ipa); > - return; > + collapse_loads (summary, summary_lto); > + collapse_stores (summary, summary_lto); > + break; > } > } > } > diff --git a/gcc/ipa-modref.h b/gcc/ipa-modref.h > index 31ceffa8d34..8fa05aaf7fb 100644 > --- a/gcc/ipa-modref.h > +++ b/gcc/ipa-modref.h > @@ -29,6 +29,7 @@ struct GTY(()) modref_summary > /* Load and stores in function (transitively closed to all callees) */ > modref_records *loads; > modref_records *stores; > + auto_vec<int> GTY((skip)) arg_flags; > > modref_summary (); > ~modref_summary (); > diff --git a/gcc/testsuite/gcc.dg/torture/pta-ptrarith-1.c b/gcc/testsuite/gcc.dg/torture/pta-ptrarith-1.c > index 99a548840df..85b68068b12 100644 > --- a/gcc/testsuite/gcc.dg/torture/pta-ptrarith-1.c > +++ b/gcc/testsuite/gcc.dg/torture/pta-ptrarith-1.c > @@ -6,11 +6,14 @@ struct Foo { > int *p; > }; > > +struct Foo *ff; > + > void __attribute__((noinline)) > foo (void *p) > { > struct Foo *f = (struct Foo *)p - 1; > *f->p = 0; > + ff = f; > } > > int bar (void) >
> > + tree callee = gimple_call_fndecl (stmt); > > + if (callee) > > + { > > + cgraph_node *node = cgraph_node::get (callee); > > + modref_summary *summary = node ? get_modref_function_summary (node) > > + : NULL; > > + > > + if (summary && summary->arg_flags.length () > arg) > > So could we make modref "transform" push this as fnspec attribute or > would that not really be an optimization? It was my original plan to synthetize fnspecs, but I think it is not very good idea: we have the summary readily available and we can represent information that fnspecs can't (do not have artificial limits on number of parameters or counts) I would preffer fnspecs to be used only for in-compiler declarations. > > + > > +/* Analyze EAF flags for SSA name NAME. > > + KNOWN_FLAGS is a cache for flags we already determined. > > + DEPTH is a recursion depth used to make debug output prettier. */ > > + > > +static int > > +analyze_ssa_name_flags (tree name, vec<int> *known_flags, int depth) > > C++ has references which makes the access to known_flags nicer ;) Yay, will chang that :) > > > +{ > > + imm_use_iterator ui; > > + gimple *use_stmt; > > + int flags = EAF_DIRECT | EAF_NOCLOBBER | EAF_NOESCAPE | EAF_UNUSED; > > + > > + /* See if value is already computed. */ > > + if ((*known_flags)[SSA_NAME_VERSION (name)]) > > + { > > + /* Punt on cycles for now, so we do not need dataflow. */ > > + if ((*known_flags)[SSA_NAME_VERSION (name)] == 1) > > + { > > + if (dump_file) > > + fprintf (dump_file, > > + "%*sGiving up on a cycle in SSA graph\n", depth * 4, ""); > > + return 0; > > + } > > + return (*known_flags)[SSA_NAME_VERSION (name)] - 2; > > + } > > + /* Recursion guard. */ > > + (*known_flags)[SSA_NAME_VERSION (name)] = 1; > > This also guards against multiple evaluations of the same stmts > but only in some cases? Consider > > _1 = ..; > _2 = _1 + _3; > _4 = _1 + _5; > _6 = _2 + _4; > > where we visit _2 = and _4 = from _1 but from both are going > to visit _6. Here we first push _6, then we go for _2 then for _1 evaluate _1, evalueate _2, go for _4 and evaluate _4, and evaluate _6. It is DFS and you need backward edge in DFS (comming from a PHI). Cycles seems to somewhat matter for GCC: we do have a lot of functions that walk linked lists that we could track otherwise. > > Maybe I'm blind but you're not limiting depth? Guess that asks > for problems, esp. as you are recursing rather than using a > worklist or so? > > I see you try to "optimize" the walk by only visiting def->use > links from parameters but then a RPO walk over all stmts would > be simpler iteration-wise ... We usually evaluate just small part of bigger functions (since we lose track quite easily after hitting first memory store). My plan was to change this to actual dataflow once we have it well defined (this means after discussing EAF flags with you and adding the logic to track callsites for true IPA pass that midly complicated things - for every ssa name I track callsite/arg pair where it is passed to either directly or indirectly. Then this is translaed into call summary and used by IPA pass to compute final flags) I guess I can add --param ipa-modref-walk-depth for now and handle dataflow incremntally? In particular I am not sure if I should just write iterated RPO myself or use tree-ssa-propagate.h (the second may be overkill). > > > + if (dump_file) > > + { > > + fprintf (dump_file, > > + "%*sAnalyzing flags of ssa name: ", depth * 4, ""); > > + print_generic_expr (dump_file, name); > > + fprintf (dump_file, "\n"); > > + } > > + > > + FOR_EACH_IMM_USE_STMT (use_stmt, ui, name) > > + { > > + if (flags == 0) > > + { > > + BREAK_FROM_IMM_USE_STMT (ui); > > + } > > + if (is_gimple_debug (use_stmt)) > > + continue; > > + if (dump_file) > > + { > > + fprintf (dump_file, "%*s Analyzing stmt:", depth * 4, ""); > > + print_gimple_stmt (dump_file, use_stmt, 0); > > + } > > + > > + /* Gimple return may load the return value. */ > > + if (gimple_code (use_stmt) == GIMPLE_RETURN) > > if (greturn *ret = dyn_cast <greturn *> (use_stmt)) > > makes the as_a below not needed, similar for the other cases (it > also makes accesses cheaper, avoiding some checking code). Looks cleaner indeed. > > > + { > > + if (memory_access_to (gimple_return_retval > > + (as_a <greturn *> (use_stmt)), name)) > > + flags &= ~EAF_UNUSED; > > + } > > + /* Account for LHS store, arg loads and flags from callee function. */ > > + else if (is_gimple_call (use_stmt)) > > + { > > + tree callee = gimple_call_fndecl (use_stmt); > > + > > + /* Recursion would require bit of propagation; give up for now. */ > > + if (callee && recursive_call_p (current_function_decl, callee)) > > + flags = 0; > > + else > > + { > > + int ecf_flags = gimple_call_flags (use_stmt); > > + bool ignore_stores = ignore_stores_p (current_function_decl, > > + ecf_flags); > > + > > + /* Handle *name = func (...). */ > > + if (gimple_call_lhs (use_stmt) > > + && memory_access_to (gimple_call_lhs (use_stmt), name)) > > + flags &= ~(EAF_UNUSED | EAF_NOCLOBBER); > > + > > + /* Handle all function parameters. */ > > + for (unsigned i = 0; i < gimple_call_num_args (use_stmt); i++) > > + /* Name is directly passed to the callee. */ > > + if (gimple_call_arg (use_stmt, i) == name) > > + { > > + if (ecf_flags & (ECF_CONST | ECF_NOVOPS)) > > + flags &= ignore_stores > > + ? 0 > > + : call_lhs_flags (use_stmt, i, > > + known_flags, depth); > > + else > > + { > > + int call_flags = gimple_call_arg_flags (as_a <gcall *> > > + (use_stmt), i); > > + if (ignore_stores) > > + call_flags |= EAF_NOCLOBBER | EAF_NOESCAPE; > > + else > > + call_flags &= call_lhs_flags (use_stmt, i, > > + known_flags, depth); > > + > > + flags &= call_flags; > > + } > > + } > > + /* Name is dereferenced and passed to a callee. */ > > + else if (memory_access_to (gimple_call_arg (use_stmt, i), name)) > > + { > > + if (ecf_flags & (ECF_CONST | ECF_NOVOPS)) > > + flags &= ~EAF_UNUSED; > > + else > > + flags &= deref_flags (gimple_call_arg_flags > > + (as_a <gcall *> (use_stmt), i), > > + ignore_stores); > > + if (!ignore_stores) > > + flags &= call_lhs_flags (use_stmt, i, known_flags, > > + depth); > > + } > > Hmm, you forget gimple_call_chain at least which is at least > argument passing? Possibly the chain argument is never a function > parameter but how do you know... (partial inlining?) Ah, right. I forgot about chain. I wil ladd it. > > Then there's gimple_call_fn for indirect function calls to a > function parameter? Well, it is never load or store, so I do not really care about it. for void test (int (*foo)()) { foo(); } I should be able to derive EAF_UNUSED. void test (int (**foo)()) { (*foo)(); } I will see sparate load. > > I guess it would be nice to amend the gimple_walk_ stuff to have > a SSA name callback where the tree and the SSA use is passed. > Well, for another time. > > > + } > > + /* Only NOCLOBBER or DIRECT flags alone are not useful (see comments > > + in tree-ssa-alias.c). Give up earlier. */ > > + if ((flags & ~(EAF_DIRECT | EAF_NOCLOBBER)) == 0) > > + flags = 0; > > + } > > + else if (gimple_assign_load_p (use_stmt)) > > + { > > + /* Memory to memory copy. */ > > + if (gimple_store_p (use_stmt)) > > + { > > + /* Handle *name = *exp. */ > > + if (memory_access_to (gimple_assign_lhs (use_stmt), name)) > > + flags &= ~(EAF_UNUSED | EAF_NOCLOBBER); > > + > > + /* Handle *lhs = *name. > > + > > + We do not track memory locations, so assume that value > > + is used arbitrarily. */ > > + if (memory_access_to (gimple_assign_rhs1 (use_stmt), name)) > > + flags = 0; > > + } > > + /* Handle lhs = *name. */ > > + else if (memory_access_to (gimple_assign_rhs1 (use_stmt), name)) > > + flags &= deref_flags (analyze_ssa_name_flags > > + (gimple_assign_lhs (use_stmt), > > + known_flags, depth + 1), false); > > + } > > + else if (gimple_store_p (use_stmt)) > > + { > > + /* Handle *lhs = name. */ > > + if (is_gimple_assign (use_stmt) > > + && gimple_assign_rhs1 (use_stmt) == name) > > + { > > + if (dump_file) > > + fprintf (dump_file, "%*s ssa name saved to memory\n", > > + depth * 4, ""); > > + flags = 0; > > + } > > + /* Handle *name = exp. */ > > + else if (is_gimple_assign (use_stmt) > > + && memory_access_to (gimple_assign_lhs (use_stmt), name)) > > + flags &= ~(EAF_UNUSED | EAF_NOCLOBBER); > > + /* ASM statements etc. */ > > + else if (!is_gimple_assign (use_stmt)) > > + { > > + if (dump_file) > > + fprintf (dump_file, "%*s Unhandled store\n", > > + depth * 4, ""); > > + flags = 0; > > + } > > + } > > + else if (is_gimple_assign (use_stmt)) > > + { > > + enum tree_code code = gimple_assign_rhs_code (use_stmt); > > + > > + /* See if operation is a merge as considered by > > + tree-ssa-structalias.c:find_func_aliases. */ > > + if (!truth_value_p (code) > > + && code != POINTER_DIFF_EXPR > > + && (code != POINTER_PLUS_EXPR > > + || gimple_assign_rhs1 (use_stmt) == name)) > > + flags &= analyze_ssa_name_flags > > + (gimple_assign_lhs (use_stmt), known_flags, > > + depth + 1); > > + } > > + else if (gimple_code (use_stmt) == GIMPLE_PHI) > > + { > > + flags &= analyze_ssa_name_flags > > + (gimple_phi_result (use_stmt), known_flags, > > + depth + 1); > > + } > > + /* Conditions are not considered escape points > > + by tree-ssa-structalias. */ > > + else if (gimple_code (use_stmt) == GIMPLE_COND) > > + ; > > + else > > + { > > + if (dump_file) > > + fprintf (dump_file, "%*s Unhandled stmt\n", depth * 4, ""); > > + flags = 0; > > + } > > + > > + if (dump_file) > > + { > > + fprintf (dump_file, "%*s current flags of ", depth * 4, ""); > > + print_generic_expr (dump_file, name); > > + dump_eaf_flags (dump_file, flags); > > + } > > + } > > + if (dump_file) > > + { > > + fprintf (dump_file, "%*sflags of ssa name ", depth * 4, ""); > > + print_generic_expr (dump_file, name); > > + dump_eaf_flags (dump_file, flags); > > + } > > + (*known_flags)[SSA_NAME_VERSION (name)] = flags + 2; > > + return flags; > > +} > > + > > +/* Determine EAF flags for function parameters. */ > > + > > +static void > > +analyze_parms (modref_summary *summary) > > +{ > > + unsigned int parm_index = 0; > > + unsigned int count = 0; > > + > > + for (tree parm = DECL_ARGUMENTS (current_function_decl); parm; > > + parm = TREE_CHAIN (parm)) > > + count++; > > + > > + if (!count) > > + return; > > + > > + auto_vec<int> known_flags; > > + known_flags.safe_grow_cleared (num_ssa_names); > > , true for exact reserve. Could be space optimized by not using > auto_vec<int> but auto_vec<usigned char>? I think there is not that much memory wasted, but indeed chars will be more effective. > > > + > > + for (tree parm = DECL_ARGUMENTS (current_function_decl); parm; parm_index++, > > + parm = TREE_CHAIN (parm)) > > + { > > + tree name = ssa_default_def (cfun, parm); > > + if (!name) > > + continue; > > looks like the vec might be quite sparse ... > > > + int flags = analyze_ssa_name_flags (name, &known_flags, 0); > > + > > + if (flags) > > + { > > + if (parm_index >= summary->arg_flags.length ()) > > + summary->arg_flags.safe_grow_cleared (count); > > you want , true for exact reserve. > > > + summary->arg_flags[parm_index] = flags; > > maybe this as well - does it make sense to skip !is_gimple_reg_type > params in the counting? Guess it makes lookup too complicated. > But we do have testcases with >30000 parameters ... Well, the index needs to be actual index all call argument. As said, i did not see any noticeable modref memory use increase at WPA (I watched) since it is at most 1 byte for parameter in case something got tracked. Thanks for review! Honza
Hi,
here is updaed patch.
Honza
Bootstrapped/regtested x86_64-linux, OK (after the fnspec fixes)?
2020-11-10 Jan Hubicka <hubicka@ucw.cz>
* gimple.c: Include ipa-modref-tree.h and ipa-modref.h.
(gimple_call_arg_flags): Use modref to determine flags.
* ipa-modref.c: Include gimple-ssa.h, tree-phinodes.h,
tree-ssa-operands.h, stringpool.h and tree-ssanames.h.
(analyze_ssa_name_flags): Declare.
(modref_summary::useful_p): Summary is also useful if arg flags are
known.
(dump_eaf_flags): New function.
(modref_summary::dump): Use it.
(get_modref_function_summary): Be read for current_function_decl
being NULL.
(memory_access_to): New function.
(deref_flags): New function.
(call_lhs_flags): New function.
(analyze_parms): New function.
(analyze_function): Use it.
* ipa-modref.h (struct modref_summary): Add arg_flags.
* doc/invoke.texi (ipa-modref-max-depth): Document.
* params.opt (ipa-modref-max-depth): New param.
gcc/testsuite/ChangeLog:
2020-11-10 Jan Hubicka <hubicka@ucw.cz>
* gcc.dg/torture/pta-ptrarith-1.c: Escape parametrs.
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index d2a188d7c75..0bd76d2841e 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -12953,6 +12953,10 @@ memory locations using the mod/ref information. This parameter ought to be
bigger than @option{--param ipa-modref-max-bases} and @option{--param
ipa-modref-max-refs}.
+@item ipa-modref-max-depth
+Specified the maximum depth of DFS walk used by modref escape analysis.
+Setting to 0 disables the analysis completely.
+
@item profile-func-internal-id
A parameter to control whether to use function internal id in profile
database lookup. If the value is 0, the compiler uses an id that
diff --git a/gcc/gimple.c b/gcc/gimple.c
index 1afed88e1f1..da90716aa23 100644
--- a/gcc/gimple.c
+++ b/gcc/gimple.c
@@ -46,6 +46,8 @@ along with GCC; see the file COPYING3. If not see
#include "asan.h"
#include "langhooks.h"
#include "attr-fnspec.h"
+#include "ipa-modref-tree.h"
+#include "ipa-modref.h"
/* All the tuples have their operand vector (if present) at the very bottom
@@ -1532,24 +1534,45 @@ int
gimple_call_arg_flags (const gcall *stmt, unsigned arg)
{
attr_fnspec fnspec = gimple_call_fnspec (stmt);
-
- if (!fnspec.known_p ())
- return 0;
-
int flags = 0;
- if (!fnspec.arg_specified_p (arg))
- ;
- else if (!fnspec.arg_used_p (arg))
- flags = EAF_UNUSED;
- else
+ if (fnspec.known_p ())
{
- if (fnspec.arg_direct_p (arg))
- flags |= EAF_DIRECT;
- if (fnspec.arg_noescape_p (arg))
- flags |= EAF_NOESCAPE;
- if (fnspec.arg_readonly_p (arg))
- flags |= EAF_NOCLOBBER;
+ if (!fnspec.arg_specified_p (arg))
+ ;
+ else if (!fnspec.arg_used_p (arg))
+ flags = EAF_UNUSED;
+ else
+ {
+ if (fnspec.arg_direct_p (arg))
+ flags |= EAF_DIRECT;
+ if (fnspec.arg_noescape_p (arg))
+ flags |= EAF_NOESCAPE;
+ if (fnspec.arg_readonly_p (arg))
+ flags |= EAF_NOCLOBBER;
+ }
+ }
+ tree callee = gimple_call_fndecl (stmt);
+ if (callee)
+ {
+ cgraph_node *node = cgraph_node::get (callee);
+ modref_summary *summary = node ? get_modref_function_summary (node)
+ : NULL;
+
+ if (summary && summary->arg_flags.length () > arg)
+ {
+ int modref_flags = summary->arg_flags[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;
+ if ((modref_flags & EAF_DIRECT) && !(flags & EAF_DIRECT))
+ modref_flags &= ~EAF_DIRECT;
+ }
+ flags |= modref_flags;
+ }
}
return flags;
}
diff --git a/gcc/ipa-modref.c b/gcc/ipa-modref.c
index 3f46bebed3c..30e76580fb0 100644
--- a/gcc/ipa-modref.c
+++ b/gcc/ipa-modref.c
@@ -61,6 +61,15 @@ along with GCC; see the file COPYING3. If not see
#include "ipa-fnsummary.h"
#include "attr-fnspec.h"
#include "symtab-clones.h"
+#include "gimple-ssa.h"
+#include "tree-phinodes.h"
+#include "tree-ssa-operands.h"
+#include "ssa-iterators.h"
+#include "stringpool.h"
+#include "tree-ssanames.h"
+
+static int analyze_ssa_name_flags (tree name,
+ vec<unsigned char> &known_flags, int depth);
/* We record fnspec specifiers for call edges since they depends on actual
gimple statements. */
@@ -186,6 +195,8 @@ modref_summary::useful_p (int ecf_flags)
{
if (ecf_flags & (ECF_CONST | ECF_NOVOPS))
return false;
+ if (arg_flags.length ())
+ return true;
if (loads && !loads->every_base)
return true;
if (ecf_flags & ECF_PURE)
@@ -355,6 +366,22 @@ dump_lto_records (modref_records_lto *tt, FILE *out)
}
}
+/* Dump EAF flags. */
+
+static void
+dump_eaf_flags (FILE *out, int flags)
+{
+ if (flags & EAF_DIRECT)
+ fprintf (out, " direct");
+ if (flags & EAF_NOCLOBBER)
+ fprintf (out, " noclobber");
+ if (flags & EAF_NOESCAPE)
+ fprintf (out, " noescape");
+ if (flags & EAF_UNUSED)
+ fprintf (out, " unused");
+ fprintf (out, "\n");
+}
+
/* Dump summary. */
void
@@ -372,6 +399,15 @@ modref_summary::dump (FILE *out)
}
if (writes_errno)
fprintf (out, " Writes errno\n");
+ if (arg_flags.length ())
+ {
+ for (unsigned int i = 0; i < arg_flags.length (); i++)
+ if (arg_flags[i])
+ {
+ fprintf (out, " parm %i flags:", i);
+ dump_eaf_flags (out, arg_flags[i]);
+ }
+ }
}
/* Dump summary. */
@@ -402,7 +438,8 @@ get_modref_function_summary (cgraph_node *func)
function. */
enum availability avail;
func = func->function_or_virtual_thunk_symbol
- (&avail, cgraph_node::get (current_function_decl));
+ (&avail, current_function_decl ?
+ cgraph_node::get (current_function_decl) : NULL);
if (avail <= AVAIL_INTERPOSABLE)
return NULL;
@@ -634,7 +671,7 @@ merge_call_side_effects (modref_summary *cur_summary,
cur_summary->loads->collapse ();
}
- parm_map.safe_grow_cleared (gimple_call_num_args (stmt));
+ parm_map.safe_grow_cleared (gimple_call_num_args (stmt), true);
for (unsigned i = 0; i < gimple_call_num_args (stmt); i++)
{
parm_map[i] = parm_map_for_arg (stmt, i);
@@ -1067,6 +1104,326 @@ remove_summary (bool lto, bool nolto, bool ipa)
" - modref done with result: not tracked.\n");
}
+/* Return true if OP accesses memory pointed to by SSA_NAME. */
+
+bool
+memory_access_to (tree op, tree ssa_name)
+{
+ tree base = get_base_address (op);
+ if (!base)
+ return false;
+ if (TREE_CODE (base) != MEM_REF && TREE_CODE (base) != TARGET_MEM_REF)
+ return false;
+ return TREE_OPERAND (base, 0) == ssa_name;
+}
+
+/* Consider statement val = *arg.
+ return EAF flags of ARG that can be determined from EAF flags of VAL
+ (which are known to be FLAGS). If IGNORE_STORES is true we can ignore
+ all stores to VAL, i.e. when handling noreturn function. */
+
+static int
+deref_flags (int flags, bool ignore_stores)
+{
+ int ret = 0;
+ if (flags & EAF_UNUSED)
+ ret |= EAF_DIRECT | EAF_NOCLOBBER | EAF_NOESCAPE;
+ else
+ {
+ if ((flags & EAF_NOCLOBBER) || ignore_stores)
+ ret |= EAF_NOCLOBBER;
+ if ((flags & EAF_NOESCAPE) || ignore_stores)
+ ret |= EAF_NOESCAPE;
+ }
+ return ret;
+}
+
+/* Call statements may return their parameters. Consider argument number
+ ARG of USE_STMT and determine flags that can needs to be cleared
+ in case pointer possibly indirectly references from ARG I is returned.
+ KNOWN_FLAGS and DEPTH are same as in analyze_ssa_name_flags. */
+
+static int
+call_lhs_flags (gcall *call, int arg,
+ vec<unsigned char> &known_flags, int depth)
+{
+ /* If there is no return value, no flags are affected. */
+ if (!gimple_call_lhs (call))
+ return EAF_DIRECT | EAF_NOCLOBBER | EAF_NOESCAPE | EAF_UNUSED;
+
+ /* If we know that function returns given argument and it is not ARG
+ we can still be happy. */
+ int flags = gimple_call_return_flags (call);
+ if ((flags & ERF_RETURNS_ARG)
+ && (flags & ERF_RETURN_ARG_MASK) != arg)
+ return EAF_DIRECT | EAF_NOCLOBBER | EAF_NOESCAPE | EAF_UNUSED;
+
+ /* If return value is SSA name determine its flags. */
+ if (TREE_CODE (gimple_call_lhs (call)) == SSA_NAME)
+ return analyze_ssa_name_flags
+ (gimple_call_lhs (call), known_flags,
+ depth + 1);
+ /* In the case of memory store we can do nothing. */
+ else
+ return 0;
+}
+
+/* Analyze EAF flags for SSA name NAME.
+ KNOWN_FLAGS is a cache for flags we already determined.
+ DEPTH is a recursion depth used to make debug output prettier. */
+
+static int
+analyze_ssa_name_flags (tree name, vec<unsigned char> &known_flags, int depth)
+{
+ imm_use_iterator ui;
+ gimple *use_stmt;
+ int flags = EAF_DIRECT | EAF_NOCLOBBER | EAF_NOESCAPE | EAF_UNUSED;
+
+ /* See if value is already computed. */
+ if (known_flags[SSA_NAME_VERSION (name)])
+ {
+ /* Punt on cycles for now, so we do not need dataflow. */
+ if (known_flags[SSA_NAME_VERSION (name)] == 1)
+ {
+ if (dump_file)
+ fprintf (dump_file,
+ "%*sGiving up on a cycle in SSA graph\n", depth * 4, "");
+ return 0;
+ }
+ return known_flags[SSA_NAME_VERSION (name)] - 2;
+ }
+ if (depth == param_modref_max_depth)
+ {
+ if (dump_file)
+ fprintf (dump_file,
+ "%*sGiving up on max depth\n", depth * 4, "");
+ return 0;
+ }
+ /* Recursion guard. */
+ known_flags[SSA_NAME_VERSION (name)] = 1;
+
+ if (dump_file)
+ {
+ fprintf (dump_file,
+ "%*sAnalyzing flags of ssa name: ", depth * 4, "");
+ print_generic_expr (dump_file, name);
+ fprintf (dump_file, "\n");
+ }
+
+ FOR_EACH_IMM_USE_STMT (use_stmt, ui, name)
+ {
+ if (flags == 0)
+ {
+ BREAK_FROM_IMM_USE_STMT (ui);
+ }
+ if (is_gimple_debug (use_stmt))
+ continue;
+ if (dump_file)
+ {
+ fprintf (dump_file, "%*s Analyzing stmt:", depth * 4, "");
+ print_gimple_stmt (dump_file, use_stmt, 0);
+ }
+
+ /* Gimple return may load the return value. */
+ if (greturn *ret = dyn_cast <greturn *> (use_stmt))
+ {
+ if (memory_access_to (gimple_return_retval (ret), name))
+ flags &= ~EAF_UNUSED;
+ }
+ /* Account for LHS store, arg loads and flags from callee function. */
+ else if (gcall *call = dyn_cast <gcall *> (use_stmt))
+ {
+ tree callee = gimple_call_fndecl (call);
+
+ /* Recursion would require bit of propagation; give up for now. */
+ if (callee && recursive_call_p (current_function_decl, callee))
+ flags = 0;
+ else
+ {
+ int ecf_flags = gimple_call_flags (call);
+ bool ignore_stores = ignore_stores_p (current_function_decl,
+ ecf_flags);
+
+ /* Handle *name = func (...). */
+ if (gimple_call_lhs (call)
+ && memory_access_to (gimple_call_lhs (call), name))
+ flags &= ~(EAF_UNUSED | EAF_NOCLOBBER);
+
+ /* We do not track accesses to the static chain (we could)
+ so give up. */
+ if (gimple_call_chain (call)
+ && (gimple_call_chain (call) == name))
+ flags = 0;
+
+ /* Handle all function parameters. */
+ for (unsigned i = 0; i < gimple_call_num_args (call); i++)
+ /* Name is directly passed to the callee. */
+ if (gimple_call_arg (call, i) == name)
+ {
+ if (ecf_flags & (ECF_CONST | ECF_NOVOPS))
+ flags &= ignore_stores
+ ? 0
+ : call_lhs_flags (call, i, known_flags, depth);
+ else
+ {
+ int call_flags = gimple_call_arg_flags (call, i);
+ if (ignore_stores)
+ call_flags |= EAF_NOCLOBBER | EAF_NOESCAPE;
+ else
+ call_flags &= call_lhs_flags (call, i,
+ known_flags, depth);
+
+ flags &= call_flags;
+ }
+ }
+ /* Name is dereferenced and passed to a callee. */
+ else if (memory_access_to (gimple_call_arg (call, i), name))
+ {
+ if (ecf_flags & (ECF_CONST | ECF_NOVOPS))
+ flags &= ~EAF_UNUSED;
+ else
+ flags &= deref_flags (gimple_call_arg_flags (call, i),
+ ignore_stores);
+ if (!ignore_stores)
+ flags &= call_lhs_flags (call, i, known_flags, depth);
+ }
+ }
+ /* Only NOCLOBBER or DIRECT flags alone are not useful (see comments
+ in tree-ssa-alias.c). Give up earlier. */
+ if ((flags & ~(EAF_DIRECT | EAF_NOCLOBBER)) == 0)
+ flags = 0;
+ }
+ else if (gimple_assign_load_p (use_stmt))
+ {
+ gassign *assign = as_a <gassign *> (use_stmt);
+ /* Memory to memory copy. */
+ if (gimple_store_p (assign))
+ {
+ /* Handle *name = *exp. */
+ if (memory_access_to (gimple_assign_lhs (assign), name))
+ flags &= ~(EAF_UNUSED | EAF_NOCLOBBER);
+
+ /* Handle *lhs = *name.
+
+ We do not track memory locations, so assume that value
+ is used arbitrarily. */
+ if (memory_access_to (gimple_assign_rhs1 (assign), name))
+ flags = 0;
+ }
+ /* Handle lhs = *name. */
+ else if (memory_access_to (gimple_assign_rhs1 (assign), name))
+ flags &= deref_flags (analyze_ssa_name_flags
+ (gimple_assign_lhs (assign),
+ known_flags, depth + 1), false);
+ }
+ else if (gimple_store_p (use_stmt))
+ {
+ gassign *assign = dyn_cast <gassign *> (use_stmt);
+
+ /* Handle *lhs = name. */
+ if (assign && gimple_assign_rhs1 (assign) == name)
+ {
+ if (dump_file)
+ fprintf (dump_file, "%*s ssa name saved to memory\n",
+ depth * 4, "");
+ flags = 0;
+ }
+ /* Handle *name = exp. */
+ else if (assign
+ && memory_access_to (gimple_assign_lhs (assign), name))
+ flags &= ~(EAF_UNUSED | EAF_NOCLOBBER);
+ /* ASM statements etc. */
+ else if (!assign)
+ {
+ if (dump_file)
+ fprintf (dump_file, "%*s Unhandled store\n",
+ depth * 4, "");
+ flags = 0;
+ }
+ }
+ else if (gassign *assign = dyn_cast <gassign *> (use_stmt))
+ {
+ enum tree_code code = gimple_assign_rhs_code (assign);
+
+ /* See if operation is a merge as considered by
+ tree-ssa-structalias.c:find_func_aliases. */
+ if (!truth_value_p (code)
+ && code != POINTER_DIFF_EXPR
+ && (code != POINTER_PLUS_EXPR
+ || gimple_assign_rhs1 (assign) == name))
+ flags &= analyze_ssa_name_flags
+ (gimple_assign_lhs (assign), known_flags,
+ depth + 1);
+ }
+ else if (gphi *phi = dyn_cast <gphi *> (use_stmt))
+ {
+ flags &= analyze_ssa_name_flags
+ (gimple_phi_result (phi), known_flags,
+ depth + 1);
+ }
+ /* Conditions are not considered escape points
+ by tree-ssa-structalias. */
+ else if (gimple_code (use_stmt) == GIMPLE_COND)
+ ;
+ else
+ {
+ if (dump_file)
+ fprintf (dump_file, "%*s Unhandled stmt\n", depth * 4, "");
+ flags = 0;
+ }
+
+ if (dump_file)
+ {
+ fprintf (dump_file, "%*s current flags of ", depth * 4, "");
+ print_generic_expr (dump_file, name);
+ dump_eaf_flags (dump_file, flags);
+ }
+ }
+ if (dump_file)
+ {
+ fprintf (dump_file, "%*sflags of ssa name ", depth * 4, "");
+ print_generic_expr (dump_file, name);
+ dump_eaf_flags (dump_file, flags);
+ }
+ known_flags[SSA_NAME_VERSION (name)] = flags + 2;
+ return flags;
+}
+
+/* Determine EAF flags for function parameters. */
+
+static void
+analyze_parms (modref_summary *summary)
+{
+ unsigned int parm_index = 0;
+ unsigned int count = 0;
+
+ for (tree parm = DECL_ARGUMENTS (current_function_decl); parm;
+ parm = TREE_CHAIN (parm))
+ count++;
+
+ if (!count)
+ return;
+
+ auto_vec<unsigned char> known_flags;
+ known_flags.safe_grow_cleared (num_ssa_names, true);
+
+ for (tree parm = DECL_ARGUMENTS (current_function_decl); parm; parm_index++,
+ parm = TREE_CHAIN (parm))
+ {
+ tree name = ssa_default_def (cfun, parm);
+ if (!name)
+ continue;
+ int flags = analyze_ssa_name_flags (name, known_flags, 0);
+
+ if (flags)
+ {
+ if (parm_index >= summary->arg_flags.length ())
+ summary->arg_flags.safe_grow_cleared (count, true);
+ summary->arg_flags[parm_index] = flags;
+ }
+ }
+}
+
/* Analyze function F. IPA indicates whether we're running in local mode
(false) or the IPA mode (true). */
@@ -1174,6 +1531,10 @@ analyze_function (function *f, bool ipa)
param_modref_max_accesses);
summary_lto->writes_errno = false;
}
+
+ if (!ipa)
+ analyze_parms (summary);
+
int ecf_flags = flags_from_decl_or_type (current_function_decl);
auto_vec <gimple *, 32> recursive_calls;
@@ -1191,8 +1552,9 @@ analyze_function (function *f, bool ipa)
|| ((!summary || !summary->useful_p (ecf_flags))
&& (!summary_lto || !summary_lto->useful_p (ecf_flags))))
{
- remove_summary (lto, nolto, ipa);
- return;
+ collapse_loads (summary, summary_lto);
+ collapse_stores (summary, summary_lto);
+ break;
}
}
}
@@ -1957,7 +2319,7 @@ compute_parm_map (cgraph_edge *callee_edge, vec<modref_parm_map> *parm_map)
: callee_edge->caller);
callee_pi = IPA_NODE_REF (callee);
- (*parm_map).safe_grow_cleared (count);
+ (*parm_map).safe_grow_cleared (count, true);
for (i = 0; i < count; i++)
{
diff --git a/gcc/ipa-modref.h b/gcc/ipa-modref.h
index 31ceffa8d34..59872301cd6 100644
--- a/gcc/ipa-modref.h
+++ b/gcc/ipa-modref.h
@@ -29,6 +29,7 @@ struct GTY(()) modref_summary
/* Load and stores in function (transitively closed to all callees) */
modref_records *loads;
modref_records *stores;
+ auto_vec<unsigned char> GTY((skip)) arg_flags;
modref_summary ();
~modref_summary ();
diff --git a/gcc/params.opt b/gcc/params.opt
index a33a371a395..70152bf59bb 100644
--- a/gcc/params.opt
+++ b/gcc/params.opt
@@ -931,6 +931,10 @@ Maximum number of accesse stored in each modref reference.
Common Joined UInteger Var(param_modref_max_tests) Init(64)
Maximum number of tests performed by modref query.
+-param=modref-max-depth=
+Common Joined UInteger Var(param_modref_max_depth) Init(256)
+Maximum depth of DFS walk used by modref escape analysis
+
-param=tm-max-aggregate-size=
Common Joined UInteger Var(param_tm_max_aggregate_size) Init(9) Param Optimization
Size in bytes after which thread-local aggregates should be instrumented with the logging functions instead of save/restore pairs.
On Tue, 10 Nov 2020, Jan Hubicka wrote: > > > + tree callee = gimple_call_fndecl (stmt); > > > + if (callee) > > > + { > > > + cgraph_node *node = cgraph_node::get (callee); > > > + modref_summary *summary = node ? get_modref_function_summary (node) > > > + : NULL; > > > + > > > + if (summary && summary->arg_flags.length () > arg) > > > > So could we make modref "transform" push this as fnspec attribute or > > would that not really be an optimization? > > It was my original plan to synthetize fnspecs, but I think it is not > very good idea: we have the summary readily available and we can > represent information that fnspecs can't > (do not have artificial limits on number of parameters or counts) > > I would preffer fnspecs to be used only for in-compiler declarations. Fine, I was just curious... > > > + > > > +/* Analyze EAF flags for SSA name NAME. > > > + KNOWN_FLAGS is a cache for flags we already determined. > > > + DEPTH is a recursion depth used to make debug output prettier. */ > > > + > > > +static int > > > +analyze_ssa_name_flags (tree name, vec<int> *known_flags, int depth) > > > > C++ has references which makes the access to known_flags nicer ;) > > Yay, will chang that :) > > > > > +{ > > > + imm_use_iterator ui; > > > + gimple *use_stmt; > > > + int flags = EAF_DIRECT | EAF_NOCLOBBER | EAF_NOESCAPE | EAF_UNUSED; > > > + > > > + /* See if value is already computed. */ > > > + if ((*known_flags)[SSA_NAME_VERSION (name)]) > > > + { > > > + /* Punt on cycles for now, so we do not need dataflow. */ > > > + if ((*known_flags)[SSA_NAME_VERSION (name)] == 1) > > > + { > > > + if (dump_file) > > > + fprintf (dump_file, > > > + "%*sGiving up on a cycle in SSA graph\n", depth * 4, ""); > > > + return 0; > > > + } > > > + return (*known_flags)[SSA_NAME_VERSION (name)] - 2; > > > + } > > > + /* Recursion guard. */ > > > + (*known_flags)[SSA_NAME_VERSION (name)] = 1; > > > > This also guards against multiple evaluations of the same stmts > > but only in some cases? Consider > > > > _1 = ..; > > _2 = _1 + _3; > > _4 = _1 + _5; > > _6 = _2 + _4; > > > > where we visit _2 = and _4 = from _1 but from both are going > > to visit _6. > > Here we first push _6, then we go for _2 then for _1 evaluate _1, > evalueate _2, go for _4 and evaluate _4, and evaluate _6. > It is DFS and you need backward edge in DFS (comming from a PHI). Hmm, but then we eventually evaluate _6 twice? > > Cycles seems to somewhat matter for GCC: we do have a lot of functions > that walk linked lists that we could track otherwise. > > > > Maybe I'm blind but you're not limiting depth? Guess that asks > > for problems, esp. as you are recursing rather than using a > > worklist or so? > > > > I see you try to "optimize" the walk by only visiting def->use > > links from parameters but then a RPO walk over all stmts would > > be simpler iteration-wise ... > We usually evaluate just small part of bigger functions (since we lose > track quite easily after hitting first memory store). My plan was to > change this to actual dataflow once we have it well defined > (this means after discussing EAF flags with you and adding the logic to > track callsites for true IPA pass that midly complicated things - for > every ssa name I track callsite/arg pair where it is passed to > either directly or indirectly. Then this is translaed into call summary > and used by IPA pass to compute final flags) > > I guess I can add --param ipa-modref-walk-depth for now and handle > dataflow incremntally? Works for me. > In particular I am not sure if I should just write iterated RPO myself > or use tree-ssa-propagate.h (the second may be overkill). tree-ssa-propagate.h is not to be used, it should DIE ;) I guess you do want to iterate SSA cycles rather than BB cycles so I suggest to re-surrect the SSA SCC discovery from the SCC value-numbering (see tree-ssa-sccvn.c:DFS () on the gcc-8-branch) which is non-recursive and micro-optimized. Could put it somewhere useful (tree-ssa.c?). > > > > > + if (dump_file) > > > + { > > > + fprintf (dump_file, > > > + "%*sAnalyzing flags of ssa name: ", depth * 4, ""); > > > + print_generic_expr (dump_file, name); > > > + fprintf (dump_file, "\n"); > > > + } > > > + > > > + FOR_EACH_IMM_USE_STMT (use_stmt, ui, name) > > > + { > > > + if (flags == 0) > > > + { > > > + BREAK_FROM_IMM_USE_STMT (ui); > > > + } > > > + if (is_gimple_debug (use_stmt)) > > > + continue; > > > + if (dump_file) > > > + { > > > + fprintf (dump_file, "%*s Analyzing stmt:", depth * 4, ""); > > > + print_gimple_stmt (dump_file, use_stmt, 0); > > > + } > > > + > > > + /* Gimple return may load the return value. */ > > > + if (gimple_code (use_stmt) == GIMPLE_RETURN) > > > > if (greturn *ret = dyn_cast <greturn *> (use_stmt)) > > > > makes the as_a below not needed, similar for the other cases (it > > also makes accesses cheaper, avoiding some checking code). > > Looks cleaner indeed. > > > > > + { > > > + if (memory_access_to (gimple_return_retval > > > + (as_a <greturn *> (use_stmt)), name)) > > > + flags &= ~EAF_UNUSED; > > > + } > > > + /* Account for LHS store, arg loads and flags from callee function. */ > > > + else if (is_gimple_call (use_stmt)) > > > + { > > > + tree callee = gimple_call_fndecl (use_stmt); > > > + > > > + /* Recursion would require bit of propagation; give up for now. */ > > > + if (callee && recursive_call_p (current_function_decl, callee)) > > > + flags = 0; > > > + else > > > + { > > > + int ecf_flags = gimple_call_flags (use_stmt); > > > + bool ignore_stores = ignore_stores_p (current_function_decl, > > > + ecf_flags); > > > + > > > + /* Handle *name = func (...). */ > > > + if (gimple_call_lhs (use_stmt) > > > + && memory_access_to (gimple_call_lhs (use_stmt), name)) > > > + flags &= ~(EAF_UNUSED | EAF_NOCLOBBER); > > > + > > > + /* Handle all function parameters. */ > > > + for (unsigned i = 0; i < gimple_call_num_args (use_stmt); i++) > > > + /* Name is directly passed to the callee. */ > > > + if (gimple_call_arg (use_stmt, i) == name) > > > + { > > > + if (ecf_flags & (ECF_CONST | ECF_NOVOPS)) > > > + flags &= ignore_stores > > > + ? 0 > > > + : call_lhs_flags (use_stmt, i, > > > + known_flags, depth); > > > + else > > > + { > > > + int call_flags = gimple_call_arg_flags (as_a <gcall *> > > > + (use_stmt), i); > > > + if (ignore_stores) > > > + call_flags |= EAF_NOCLOBBER | EAF_NOESCAPE; > > > + else > > > + call_flags &= call_lhs_flags (use_stmt, i, > > > + known_flags, depth); > > > + > > > + flags &= call_flags; > > > + } > > > + } > > > + /* Name is dereferenced and passed to a callee. */ > > > + else if (memory_access_to (gimple_call_arg (use_stmt, i), name)) > > > + { > > > + if (ecf_flags & (ECF_CONST | ECF_NOVOPS)) > > > + flags &= ~EAF_UNUSED; > > > + else > > > + flags &= deref_flags (gimple_call_arg_flags > > > + (as_a <gcall *> (use_stmt), i), > > > + ignore_stores); > > > + if (!ignore_stores) > > > + flags &= call_lhs_flags (use_stmt, i, known_flags, > > > + depth); > > > + } > > > > Hmm, you forget gimple_call_chain at least which is at least > > argument passing? Possibly the chain argument is never a function > > parameter but how do you know... (partial inlining?) > > Ah, right. I forgot about chain. > I wil ladd it. > > > > Then there's gimple_call_fn for indirect function calls to a > > function parameter? > > Well, it is never load or store, so I do not really care about it. > for > > void test (int (*foo)()) > { > foo(); > } > > I should be able to derive EAF_UNUSED. > > void test (int (**foo)()) > { > (*foo)(); > } > > I will see sparate load. > > > > I guess it would be nice to amend the gimple_walk_ stuff to have > > a SSA name callback where the tree and the SSA use is passed. > > Well, for another time. > > > > > + } > > > + /* Only NOCLOBBER or DIRECT flags alone are not useful (see comments > > > + in tree-ssa-alias.c). Give up earlier. */ > > > + if ((flags & ~(EAF_DIRECT | EAF_NOCLOBBER)) == 0) > > > + flags = 0; > > > + } > > > + else if (gimple_assign_load_p (use_stmt)) > > > + { > > > + /* Memory to memory copy. */ > > > + if (gimple_store_p (use_stmt)) > > > + { > > > + /* Handle *name = *exp. */ > > > + if (memory_access_to (gimple_assign_lhs (use_stmt), name)) > > > + flags &= ~(EAF_UNUSED | EAF_NOCLOBBER); > > > + > > > + /* Handle *lhs = *name. > > > + > > > + We do not track memory locations, so assume that value > > > + is used arbitrarily. */ > > > + if (memory_access_to (gimple_assign_rhs1 (use_stmt), name)) > > > + flags = 0; > > > + } > > > + /* Handle lhs = *name. */ > > > + else if (memory_access_to (gimple_assign_rhs1 (use_stmt), name)) > > > + flags &= deref_flags (analyze_ssa_name_flags > > > + (gimple_assign_lhs (use_stmt), > > > + known_flags, depth + 1), false); > > > + } > > > + else if (gimple_store_p (use_stmt)) > > > + { > > > + /* Handle *lhs = name. */ > > > + if (is_gimple_assign (use_stmt) > > > + && gimple_assign_rhs1 (use_stmt) == name) > > > + { > > > + if (dump_file) > > > + fprintf (dump_file, "%*s ssa name saved to memory\n", > > > + depth * 4, ""); > > > + flags = 0; > > > + } > > > + /* Handle *name = exp. */ > > > + else if (is_gimple_assign (use_stmt) > > > + && memory_access_to (gimple_assign_lhs (use_stmt), name)) > > > + flags &= ~(EAF_UNUSED | EAF_NOCLOBBER); > > > + /* ASM statements etc. */ > > > + else if (!is_gimple_assign (use_stmt)) > > > + { > > > + if (dump_file) > > > + fprintf (dump_file, "%*s Unhandled store\n", > > > + depth * 4, ""); > > > + flags = 0; > > > + } > > > + } > > > + else if (is_gimple_assign (use_stmt)) > > > + { > > > + enum tree_code code = gimple_assign_rhs_code (use_stmt); > > > + > > > + /* See if operation is a merge as considered by > > > + tree-ssa-structalias.c:find_func_aliases. */ > > > + if (!truth_value_p (code) > > > + && code != POINTER_DIFF_EXPR > > > + && (code != POINTER_PLUS_EXPR > > > + || gimple_assign_rhs1 (use_stmt) == name)) > > > + flags &= analyze_ssa_name_flags > > > + (gimple_assign_lhs (use_stmt), known_flags, > > > + depth + 1); > > > + } > > > + else if (gimple_code (use_stmt) == GIMPLE_PHI) > > > + { > > > + flags &= analyze_ssa_name_flags > > > + (gimple_phi_result (use_stmt), known_flags, > > > + depth + 1); > > > + } > > > + /* Conditions are not considered escape points > > > + by tree-ssa-structalias. */ > > > + else if (gimple_code (use_stmt) == GIMPLE_COND) > > > + ; > > > + else > > > + { > > > + if (dump_file) > > > + fprintf (dump_file, "%*s Unhandled stmt\n", depth * 4, ""); > > > + flags = 0; > > > + } > > > + > > > + if (dump_file) > > > + { > > > + fprintf (dump_file, "%*s current flags of ", depth * 4, ""); > > > + print_generic_expr (dump_file, name); > > > + dump_eaf_flags (dump_file, flags); > > > + } > > > + } > > > + if (dump_file) > > > + { > > > + fprintf (dump_file, "%*sflags of ssa name ", depth * 4, ""); > > > + print_generic_expr (dump_file, name); > > > + dump_eaf_flags (dump_file, flags); > > > + } > > > + (*known_flags)[SSA_NAME_VERSION (name)] = flags + 2; > > > + return flags; > > > +} > > > + > > > +/* Determine EAF flags for function parameters. */ > > > + > > > +static void > > > +analyze_parms (modref_summary *summary) > > > +{ > > > + unsigned int parm_index = 0; > > > + unsigned int count = 0; > > > + > > > + for (tree parm = DECL_ARGUMENTS (current_function_decl); parm; > > > + parm = TREE_CHAIN (parm)) > > > + count++; > > > + > > > + if (!count) > > > + return; > > > + > > > + auto_vec<int> known_flags; > > > + known_flags.safe_grow_cleared (num_ssa_names); > > > > , true for exact reserve. Could be space optimized by not using > > auto_vec<int> but auto_vec<usigned char>? > > I think there is not that much memory wasted, but indeed chars will be > more effective. > > > > > + > > > + for (tree parm = DECL_ARGUMENTS (current_function_decl); parm; parm_index++, > > > + parm = TREE_CHAIN (parm)) > > > + { > > > + tree name = ssa_default_def (cfun, parm); > > > + if (!name) > > > + continue; > > > > looks like the vec might be quite sparse ... > > > > > + int flags = analyze_ssa_name_flags (name, &known_flags, 0); > > > + > > > + if (flags) > > > + { > > > + if (parm_index >= summary->arg_flags.length ()) > > > + summary->arg_flags.safe_grow_cleared (count); > > > > you want , true for exact reserve. > > > > > + summary->arg_flags[parm_index] = flags; > > > > maybe this as well - does it make sense to skip !is_gimple_reg_type > > params in the counting? Guess it makes lookup too complicated. > > But we do have testcases with >30000 parameters ... > > Well, the index needs to be actual index all call argument. > As said, i did not see any noticeable modref memory use increase at WPA > (I watched) since it is at most 1 byte for parameter in case something > got tracked. > > Thanks for review! > > Honza >
On Tue, 10 Nov 2020, Jan Hubicka wrote: > Hi, > here is updaed patch. > > Honza > > Bootstrapped/regtested x86_64-linux, OK (after the fnspec fixes)? OK. Thanks, Richard. > > 2020-11-10 Jan Hubicka <hubicka@ucw.cz> > > * gimple.c: Include ipa-modref-tree.h and ipa-modref.h. > (gimple_call_arg_flags): Use modref to determine flags. > * ipa-modref.c: Include gimple-ssa.h, tree-phinodes.h, > tree-ssa-operands.h, stringpool.h and tree-ssanames.h. > (analyze_ssa_name_flags): Declare. > (modref_summary::useful_p): Summary is also useful if arg flags are > known. > (dump_eaf_flags): New function. > (modref_summary::dump): Use it. > (get_modref_function_summary): Be read for current_function_decl > being NULL. > (memory_access_to): New function. > (deref_flags): New function. > (call_lhs_flags): New function. > (analyze_parms): New function. > (analyze_function): Use it. > * ipa-modref.h (struct modref_summary): Add arg_flags. > * doc/invoke.texi (ipa-modref-max-depth): Document. > * params.opt (ipa-modref-max-depth): New param. > > gcc/testsuite/ChangeLog: > > 2020-11-10 Jan Hubicka <hubicka@ucw.cz> > > * gcc.dg/torture/pta-ptrarith-1.c: Escape parametrs. > > diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi > index d2a188d7c75..0bd76d2841e 100644 > --- a/gcc/doc/invoke.texi > +++ b/gcc/doc/invoke.texi > @@ -12953,6 +12953,10 @@ memory locations using the mod/ref information. This parameter ought to be > bigger than @option{--param ipa-modref-max-bases} and @option{--param > ipa-modref-max-refs}. > > +@item ipa-modref-max-depth > +Specified the maximum depth of DFS walk used by modref escape analysis. > +Setting to 0 disables the analysis completely. > + > @item profile-func-internal-id > A parameter to control whether to use function internal id in profile > database lookup. If the value is 0, the compiler uses an id that > diff --git a/gcc/gimple.c b/gcc/gimple.c > index 1afed88e1f1..da90716aa23 100644 > --- a/gcc/gimple.c > +++ b/gcc/gimple.c > @@ -46,6 +46,8 @@ along with GCC; see the file COPYING3. If not see > #include "asan.h" > #include "langhooks.h" > #include "attr-fnspec.h" > +#include "ipa-modref-tree.h" > +#include "ipa-modref.h" > > > /* All the tuples have their operand vector (if present) at the very bottom > @@ -1532,24 +1534,45 @@ int > gimple_call_arg_flags (const gcall *stmt, unsigned arg) > { > attr_fnspec fnspec = gimple_call_fnspec (stmt); > - > - if (!fnspec.known_p ()) > - return 0; > - > int flags = 0; > > - if (!fnspec.arg_specified_p (arg)) > - ; > - else if (!fnspec.arg_used_p (arg)) > - flags = EAF_UNUSED; > - else > + if (fnspec.known_p ()) > { > - if (fnspec.arg_direct_p (arg)) > - flags |= EAF_DIRECT; > - if (fnspec.arg_noescape_p (arg)) > - flags |= EAF_NOESCAPE; > - if (fnspec.arg_readonly_p (arg)) > - flags |= EAF_NOCLOBBER; > + if (!fnspec.arg_specified_p (arg)) > + ; > + else if (!fnspec.arg_used_p (arg)) > + flags = EAF_UNUSED; > + else > + { > + if (fnspec.arg_direct_p (arg)) > + flags |= EAF_DIRECT; > + if (fnspec.arg_noescape_p (arg)) > + flags |= EAF_NOESCAPE; > + if (fnspec.arg_readonly_p (arg)) > + flags |= EAF_NOCLOBBER; > + } > + } > + tree callee = gimple_call_fndecl (stmt); > + if (callee) > + { > + cgraph_node *node = cgraph_node::get (callee); > + modref_summary *summary = node ? get_modref_function_summary (node) > + : NULL; > + > + if (summary && summary->arg_flags.length () > arg) > + { > + int modref_flags = summary->arg_flags[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; > + if ((modref_flags & EAF_DIRECT) && !(flags & EAF_DIRECT)) > + modref_flags &= ~EAF_DIRECT; > + } > + flags |= modref_flags; > + } > } > return flags; > } > diff --git a/gcc/ipa-modref.c b/gcc/ipa-modref.c > index 3f46bebed3c..30e76580fb0 100644 > --- a/gcc/ipa-modref.c > +++ b/gcc/ipa-modref.c > @@ -61,6 +61,15 @@ along with GCC; see the file COPYING3. If not see > #include "ipa-fnsummary.h" > #include "attr-fnspec.h" > #include "symtab-clones.h" > +#include "gimple-ssa.h" > +#include "tree-phinodes.h" > +#include "tree-ssa-operands.h" > +#include "ssa-iterators.h" > +#include "stringpool.h" > +#include "tree-ssanames.h" > + > +static int analyze_ssa_name_flags (tree name, > + vec<unsigned char> &known_flags, int depth); > > /* We record fnspec specifiers for call edges since they depends on actual > gimple statements. */ > @@ -186,6 +195,8 @@ modref_summary::useful_p (int ecf_flags) > { > if (ecf_flags & (ECF_CONST | ECF_NOVOPS)) > return false; > + if (arg_flags.length ()) > + return true; > if (loads && !loads->every_base) > return true; > if (ecf_flags & ECF_PURE) > @@ -355,6 +366,22 @@ dump_lto_records (modref_records_lto *tt, FILE *out) > } > } > > +/* Dump EAF flags. */ > + > +static void > +dump_eaf_flags (FILE *out, int flags) > +{ > + if (flags & EAF_DIRECT) > + fprintf (out, " direct"); > + if (flags & EAF_NOCLOBBER) > + fprintf (out, " noclobber"); > + if (flags & EAF_NOESCAPE) > + fprintf (out, " noescape"); > + if (flags & EAF_UNUSED) > + fprintf (out, " unused"); > + fprintf (out, "\n"); > +} > + > /* Dump summary. */ > > void > @@ -372,6 +399,15 @@ modref_summary::dump (FILE *out) > } > if (writes_errno) > fprintf (out, " Writes errno\n"); > + if (arg_flags.length ()) > + { > + for (unsigned int i = 0; i < arg_flags.length (); i++) > + if (arg_flags[i]) > + { > + fprintf (out, " parm %i flags:", i); > + dump_eaf_flags (out, arg_flags[i]); > + } > + } > } > > /* Dump summary. */ > @@ -402,7 +438,8 @@ get_modref_function_summary (cgraph_node *func) > function. */ > enum availability avail; > func = func->function_or_virtual_thunk_symbol > - (&avail, cgraph_node::get (current_function_decl)); > + (&avail, current_function_decl ? > + cgraph_node::get (current_function_decl) : NULL); > if (avail <= AVAIL_INTERPOSABLE) > return NULL; > > @@ -634,7 +671,7 @@ merge_call_side_effects (modref_summary *cur_summary, > cur_summary->loads->collapse (); > } > > - parm_map.safe_grow_cleared (gimple_call_num_args (stmt)); > + parm_map.safe_grow_cleared (gimple_call_num_args (stmt), true); > for (unsigned i = 0; i < gimple_call_num_args (stmt); i++) > { > parm_map[i] = parm_map_for_arg (stmt, i); > @@ -1067,6 +1104,326 @@ remove_summary (bool lto, bool nolto, bool ipa) > " - modref done with result: not tracked.\n"); > } > > +/* Return true if OP accesses memory pointed to by SSA_NAME. */ > + > +bool > +memory_access_to (tree op, tree ssa_name) > +{ > + tree base = get_base_address (op); > + if (!base) > + return false; > + if (TREE_CODE (base) != MEM_REF && TREE_CODE (base) != TARGET_MEM_REF) > + return false; > + return TREE_OPERAND (base, 0) == ssa_name; > +} > + > +/* Consider statement val = *arg. > + return EAF flags of ARG that can be determined from EAF flags of VAL > + (which are known to be FLAGS). If IGNORE_STORES is true we can ignore > + all stores to VAL, i.e. when handling noreturn function. */ > + > +static int > +deref_flags (int flags, bool ignore_stores) > +{ > + int ret = 0; > + if (flags & EAF_UNUSED) > + ret |= EAF_DIRECT | EAF_NOCLOBBER | EAF_NOESCAPE; > + else > + { > + if ((flags & EAF_NOCLOBBER) || ignore_stores) > + ret |= EAF_NOCLOBBER; > + if ((flags & EAF_NOESCAPE) || ignore_stores) > + ret |= EAF_NOESCAPE; > + } > + return ret; > +} > + > +/* Call statements may return their parameters. Consider argument number > + ARG of USE_STMT and determine flags that can needs to be cleared > + in case pointer possibly indirectly references from ARG I is returned. > + KNOWN_FLAGS and DEPTH are same as in analyze_ssa_name_flags. */ > + > +static int > +call_lhs_flags (gcall *call, int arg, > + vec<unsigned char> &known_flags, int depth) > +{ > + /* If there is no return value, no flags are affected. */ > + if (!gimple_call_lhs (call)) > + return EAF_DIRECT | EAF_NOCLOBBER | EAF_NOESCAPE | EAF_UNUSED; > + > + /* If we know that function returns given argument and it is not ARG > + we can still be happy. */ > + int flags = gimple_call_return_flags (call); > + if ((flags & ERF_RETURNS_ARG) > + && (flags & ERF_RETURN_ARG_MASK) != arg) > + return EAF_DIRECT | EAF_NOCLOBBER | EAF_NOESCAPE | EAF_UNUSED; > + > + /* If return value is SSA name determine its flags. */ > + if (TREE_CODE (gimple_call_lhs (call)) == SSA_NAME) > + return analyze_ssa_name_flags > + (gimple_call_lhs (call), known_flags, > + depth + 1); > + /* In the case of memory store we can do nothing. */ > + else > + return 0; > +} > + > +/* Analyze EAF flags for SSA name NAME. > + KNOWN_FLAGS is a cache for flags we already determined. > + DEPTH is a recursion depth used to make debug output prettier. */ > + > +static int > +analyze_ssa_name_flags (tree name, vec<unsigned char> &known_flags, int depth) > +{ > + imm_use_iterator ui; > + gimple *use_stmt; > + int flags = EAF_DIRECT | EAF_NOCLOBBER | EAF_NOESCAPE | EAF_UNUSED; > + > + /* See if value is already computed. */ > + if (known_flags[SSA_NAME_VERSION (name)]) > + { > + /* Punt on cycles for now, so we do not need dataflow. */ > + if (known_flags[SSA_NAME_VERSION (name)] == 1) > + { > + if (dump_file) > + fprintf (dump_file, > + "%*sGiving up on a cycle in SSA graph\n", depth * 4, ""); > + return 0; > + } > + return known_flags[SSA_NAME_VERSION (name)] - 2; > + } > + if (depth == param_modref_max_depth) > + { > + if (dump_file) > + fprintf (dump_file, > + "%*sGiving up on max depth\n", depth * 4, ""); > + return 0; > + } > + /* Recursion guard. */ > + known_flags[SSA_NAME_VERSION (name)] = 1; > + > + if (dump_file) > + { > + fprintf (dump_file, > + "%*sAnalyzing flags of ssa name: ", depth * 4, ""); > + print_generic_expr (dump_file, name); > + fprintf (dump_file, "\n"); > + } > + > + FOR_EACH_IMM_USE_STMT (use_stmt, ui, name) > + { > + if (flags == 0) > + { > + BREAK_FROM_IMM_USE_STMT (ui); > + } > + if (is_gimple_debug (use_stmt)) > + continue; > + if (dump_file) > + { > + fprintf (dump_file, "%*s Analyzing stmt:", depth * 4, ""); > + print_gimple_stmt (dump_file, use_stmt, 0); > + } > + > + /* Gimple return may load the return value. */ > + if (greturn *ret = dyn_cast <greturn *> (use_stmt)) > + { > + if (memory_access_to (gimple_return_retval (ret), name)) > + flags &= ~EAF_UNUSED; > + } > + /* Account for LHS store, arg loads and flags from callee function. */ > + else if (gcall *call = dyn_cast <gcall *> (use_stmt)) > + { > + tree callee = gimple_call_fndecl (call); > + > + /* Recursion would require bit of propagation; give up for now. */ > + if (callee && recursive_call_p (current_function_decl, callee)) > + flags = 0; > + else > + { > + int ecf_flags = gimple_call_flags (call); > + bool ignore_stores = ignore_stores_p (current_function_decl, > + ecf_flags); > + > + /* Handle *name = func (...). */ > + if (gimple_call_lhs (call) > + && memory_access_to (gimple_call_lhs (call), name)) > + flags &= ~(EAF_UNUSED | EAF_NOCLOBBER); > + > + /* We do not track accesses to the static chain (we could) > + so give up. */ > + if (gimple_call_chain (call) > + && (gimple_call_chain (call) == name)) > + flags = 0; > + > + /* Handle all function parameters. */ > + for (unsigned i = 0; i < gimple_call_num_args (call); i++) > + /* Name is directly passed to the callee. */ > + if (gimple_call_arg (call, i) == name) > + { > + if (ecf_flags & (ECF_CONST | ECF_NOVOPS)) > + flags &= ignore_stores > + ? 0 > + : call_lhs_flags (call, i, known_flags, depth); > + else > + { > + int call_flags = gimple_call_arg_flags (call, i); > + if (ignore_stores) > + call_flags |= EAF_NOCLOBBER | EAF_NOESCAPE; > + else > + call_flags &= call_lhs_flags (call, i, > + known_flags, depth); > + > + flags &= call_flags; > + } > + } > + /* Name is dereferenced and passed to a callee. */ > + else if (memory_access_to (gimple_call_arg (call, i), name)) > + { > + if (ecf_flags & (ECF_CONST | ECF_NOVOPS)) > + flags &= ~EAF_UNUSED; > + else > + flags &= deref_flags (gimple_call_arg_flags (call, i), > + ignore_stores); > + if (!ignore_stores) > + flags &= call_lhs_flags (call, i, known_flags, depth); > + } > + } > + /* Only NOCLOBBER or DIRECT flags alone are not useful (see comments > + in tree-ssa-alias.c). Give up earlier. */ > + if ((flags & ~(EAF_DIRECT | EAF_NOCLOBBER)) == 0) > + flags = 0; > + } > + else if (gimple_assign_load_p (use_stmt)) > + { > + gassign *assign = as_a <gassign *> (use_stmt); > + /* Memory to memory copy. */ > + if (gimple_store_p (assign)) > + { > + /* Handle *name = *exp. */ > + if (memory_access_to (gimple_assign_lhs (assign), name)) > + flags &= ~(EAF_UNUSED | EAF_NOCLOBBER); > + > + /* Handle *lhs = *name. > + > + We do not track memory locations, so assume that value > + is used arbitrarily. */ > + if (memory_access_to (gimple_assign_rhs1 (assign), name)) > + flags = 0; > + } > + /* Handle lhs = *name. */ > + else if (memory_access_to (gimple_assign_rhs1 (assign), name)) > + flags &= deref_flags (analyze_ssa_name_flags > + (gimple_assign_lhs (assign), > + known_flags, depth + 1), false); > + } > + else if (gimple_store_p (use_stmt)) > + { > + gassign *assign = dyn_cast <gassign *> (use_stmt); > + > + /* Handle *lhs = name. */ > + if (assign && gimple_assign_rhs1 (assign) == name) > + { > + if (dump_file) > + fprintf (dump_file, "%*s ssa name saved to memory\n", > + depth * 4, ""); > + flags = 0; > + } > + /* Handle *name = exp. */ > + else if (assign > + && memory_access_to (gimple_assign_lhs (assign), name)) > + flags &= ~(EAF_UNUSED | EAF_NOCLOBBER); > + /* ASM statements etc. */ > + else if (!assign) > + { > + if (dump_file) > + fprintf (dump_file, "%*s Unhandled store\n", > + depth * 4, ""); > + flags = 0; > + } > + } > + else if (gassign *assign = dyn_cast <gassign *> (use_stmt)) > + { > + enum tree_code code = gimple_assign_rhs_code (assign); > + > + /* See if operation is a merge as considered by > + tree-ssa-structalias.c:find_func_aliases. */ > + if (!truth_value_p (code) > + && code != POINTER_DIFF_EXPR > + && (code != POINTER_PLUS_EXPR > + || gimple_assign_rhs1 (assign) == name)) > + flags &= analyze_ssa_name_flags > + (gimple_assign_lhs (assign), known_flags, > + depth + 1); > + } > + else if (gphi *phi = dyn_cast <gphi *> (use_stmt)) > + { > + flags &= analyze_ssa_name_flags > + (gimple_phi_result (phi), known_flags, > + depth + 1); > + } > + /* Conditions are not considered escape points > + by tree-ssa-structalias. */ > + else if (gimple_code (use_stmt) == GIMPLE_COND) > + ; > + else > + { > + if (dump_file) > + fprintf (dump_file, "%*s Unhandled stmt\n", depth * 4, ""); > + flags = 0; > + } > + > + if (dump_file) > + { > + fprintf (dump_file, "%*s current flags of ", depth * 4, ""); > + print_generic_expr (dump_file, name); > + dump_eaf_flags (dump_file, flags); > + } > + } > + if (dump_file) > + { > + fprintf (dump_file, "%*sflags of ssa name ", depth * 4, ""); > + print_generic_expr (dump_file, name); > + dump_eaf_flags (dump_file, flags); > + } > + known_flags[SSA_NAME_VERSION (name)] = flags + 2; > + return flags; > +} > + > +/* Determine EAF flags for function parameters. */ > + > +static void > +analyze_parms (modref_summary *summary) > +{ > + unsigned int parm_index = 0; > + unsigned int count = 0; > + > + for (tree parm = DECL_ARGUMENTS (current_function_decl); parm; > + parm = TREE_CHAIN (parm)) > + count++; > + > + if (!count) > + return; > + > + auto_vec<unsigned char> known_flags; > + known_flags.safe_grow_cleared (num_ssa_names, true); > + > + for (tree parm = DECL_ARGUMENTS (current_function_decl); parm; parm_index++, > + parm = TREE_CHAIN (parm)) > + { > + tree name = ssa_default_def (cfun, parm); > + if (!name) > + continue; > + int flags = analyze_ssa_name_flags (name, known_flags, 0); > + > + if (flags) > + { > + if (parm_index >= summary->arg_flags.length ()) > + summary->arg_flags.safe_grow_cleared (count, true); > + summary->arg_flags[parm_index] = flags; > + } > + } > +} > + > /* Analyze function F. IPA indicates whether we're running in local mode > (false) or the IPA mode (true). */ > > @@ -1174,6 +1531,10 @@ analyze_function (function *f, bool ipa) > param_modref_max_accesses); > summary_lto->writes_errno = false; > } > + > + if (!ipa) > + analyze_parms (summary); > + > int ecf_flags = flags_from_decl_or_type (current_function_decl); > auto_vec <gimple *, 32> recursive_calls; > > @@ -1191,8 +1552,9 @@ analyze_function (function *f, bool ipa) > || ((!summary || !summary->useful_p (ecf_flags)) > && (!summary_lto || !summary_lto->useful_p (ecf_flags)))) > { > - remove_summary (lto, nolto, ipa); > - return; > + collapse_loads (summary, summary_lto); > + collapse_stores (summary, summary_lto); > + break; > } > } > } > @@ -1957,7 +2319,7 @@ compute_parm_map (cgraph_edge *callee_edge, vec<modref_parm_map> *parm_map) > : callee_edge->caller); > callee_pi = IPA_NODE_REF (callee); > > - (*parm_map).safe_grow_cleared (count); > + (*parm_map).safe_grow_cleared (count, true); > > for (i = 0; i < count; i++) > { > diff --git a/gcc/ipa-modref.h b/gcc/ipa-modref.h > index 31ceffa8d34..59872301cd6 100644 > --- a/gcc/ipa-modref.h > +++ b/gcc/ipa-modref.h > @@ -29,6 +29,7 @@ struct GTY(()) modref_summary > /* Load and stores in function (transitively closed to all callees) */ > modref_records *loads; > modref_records *stores; > + auto_vec<unsigned char> GTY((skip)) arg_flags; > > modref_summary (); > ~modref_summary (); > diff --git a/gcc/params.opt b/gcc/params.opt > index a33a371a395..70152bf59bb 100644 > --- a/gcc/params.opt > +++ b/gcc/params.opt > @@ -931,6 +931,10 @@ Maximum number of accesse stored in each modref reference. > Common Joined UInteger Var(param_modref_max_tests) Init(64) > Maximum number of tests performed by modref query. > > +-param=modref-max-depth= > +Common Joined UInteger Var(param_modref_max_depth) Init(256) > +Maximum depth of DFS walk used by modref escape analysis > + > -param=tm-max-aggregate-size= > Common Joined UInteger Var(param_tm_max_aggregate_size) Init(9) Param Optimization > Size in bytes after which thread-local aggregates should be instrumented with the logging functions instead of save/restore pairs. >
This breaks bootstrap with go.
../../gcc/go/gofrontend/go-diagnostics.cc: In function 'std::string expand_message(const char*, va_list)':
../../gcc/go/gofrontend/go-diagnostics.cc:110:61: error: '<anonymous>' may be used uninitialized [-Werror=maybe-uninitialized]
110 | "memory allocation failed in vasprintf");
| ^
Andreas.
> This breaks bootstrap with go. > > ../../gcc/go/gofrontend/go-diagnostics.cc: In function 'std::string expand_message(const char*, va_list)': > ../../gcc/go/gofrontend/go-diagnostics.cc:110:61: error: '<anonymous>' may be used uninitialized [-Werror=maybe-uninitialized] > 110 | "memory allocation failed in vasprintf"); > | ^ Well, this may be simply a false positive (except that the warning is clearly mislocalted and cryptic) I was bootstrapping with go just few minutes ago, so I wonder what configure flags you use? Honza > > Andreas. > > -- > Andreas Schwab, schwab@linux-m68k.org > GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510 2552 DF73 E780 A9DA AEC1 > "And now for something completely different."
Hi Jan, >> This breaks bootstrap with go. >> >> ../../gcc/go/gofrontend/go-diagnostics.cc: In function 'std::string expand_message(const char*, va_list)': >> ../../gcc/go/gofrontend/go-diagnostics.cc:110:61: error: '<anonymous>' may be used uninitialized [-Werror=maybe-uninitialized] >> 110 | "memory allocation failed in vasprintf"); >> | ^ > > Well, this may be simply a false positive (except that the warning is > clearly mislocalted and cryptic) I was bootstrapping with go just few > minutes ago, so I wonder what configure flags you use? I'm seeing the same on both i386-pc-solaris2.11 and sparc-sun-solaris2.11, so I don't think there are special configure flags involved. Rainer
> Hi Jan, > > >> This breaks bootstrap with go. > >> > >> ../../gcc/go/gofrontend/go-diagnostics.cc: In function 'std::string expand_message(const char*, va_list)': > >> ../../gcc/go/gofrontend/go-diagnostics.cc:110:61: error: '<anonymous>' may be used uninitialized [-Werror=maybe-uninitialized] > >> 110 | "memory allocation failed in vasprintf"); > >> | ^ > > > > Well, this may be simply a false positive (except that the warning is > > clearly mislocalted and cryptic) I was bootstrapping with go just few > > minutes ago, so I wonder what configure flags you use? > > I'm seeing the same on both i386-pc-solaris2.11 and > sparc-sun-solaris2.11, so I don't think there are special configure > flags involved. When I configure with ../configure --enable-languages=go my build eventually finishes fine on curren trunk: /aux/hubicka/trunk-git/build-go/./gcc/gccgo -B/aux/hubicka/trunk-git/build-go/./gcc/ -B/usr/local/x86_64-pc-linux-gnu/bin/ -B/usr/local/x86_64-pc-linux-gnu/lib/ -isystem /usr/local/x86_64-pc-linux-gnu/include -isystem /usr/local/x86_64-pc-linux-gnu/sys-include -g -O2 -I ../x86_64-pc-linux-gnu/libgo -static-libstdc++ -static-libgcc -L ../x86_64-pc-linux-gnu/libgo -L ../x86_64-pc-linux-gnu/libgo/.libs -o cgo ../../gotools/../libgo/go/cmd/cgo/ast.go ../../gotools/../libgo/go/cmd/cgo/doc.go ../../gotools/../libgo/go/cmd/cgo/gcc.go ../../gotools/../libgo/go/cmd/cgo/godefs.go ../../gotools/../libgo/go/cmd/cgo/main.go ../../gotools/../libgo/go/cmd/cgo/out.go ../../gotools/../libgo/go/cmd/cgo/util.go zdefaultcc.go ../x86_64-pc-linux-gnu/libgo/libgotool.a make[2]: Leaving directory '/aux/hubicka/trunk-git/build-go/gotools' make[1]: Leaving directory '/aux/hubicka/trunk-git/build-go' On Debian SLES machines. Having preprocessed go-diagnostics.cc and .uninit1 dump would probably help. These are often false positives, but I saw similar warnings caused by wrong EAF_UNUSED flag discovery which led to init code being optimized out before inlining, so I would like to look into the dumps and figure out if that is uninit acting funny or there is some real problem. Honza > > Rainer > > -- > ----------------------------------------------------------------------------- > Rainer Orth, Center for Biotechnology, Bielefeld University
Hi Jan, >> I'm seeing the same on both i386-pc-solaris2.11 and >> sparc-sun-solaris2.11, so I don't think there are special configure >> flags involved. > > When I configure with ../configure --enable-languages=go my build > eventually finishes fine on curren trunk: [...] > On Debian SLES machines. Having preprocessed go-diagnostics.cc and > .uninit1 dump would probably help. apart from go/diagnostics.cc, I see the warning several times in libstdc++, but only for the 32-bit multilib. Try building either for i686-pc-linux-gnu or a bi-arch x86_64-pc-linux-gnu build. Rainer
> Hi Jan, > > >> I'm seeing the same on both i386-pc-solaris2.11 and > >> sparc-sun-solaris2.11, so I don't think there are special configure > >> flags involved. > > > > When I configure with ../configure --enable-languages=go my build > > eventually finishes fine on curren trunk: > [...] > > On Debian SLES machines. Having preprocessed go-diagnostics.cc and > > .uninit1 dump would probably help. > > apart from go/diagnostics.cc, I see the warning several times in > libstdc++, but only for the 32-bit multilib. > > Try building either for i686-pc-linux-gnu or a bi-arch > x86_64-pc-linux-gnu build. My build is bi-arch. I will check libstdc++ warnings, but we had some such warnings previously there too (plus we have tons of uninitialized warnings with LTO bootstrap). This really may depend on glibc headers and how string functions get expanded. This is why I think having preprocessed diagnotics.cc should help. Honza > > Rainer > > -- > ----------------------------------------------------------------------------- > Rainer Orth, Center for Biotechnology, Bielefeld University
On Fri, Nov 13, 2020 at 12:07 AM Richard Biener <rguenther@suse.de> wrote: > > On Tue, 10 Nov 2020, Jan Hubicka wrote: > > > Hi, > > here is updaed patch. > > > > Honza > > > > Bootstrapped/regtested x86_64-linux, OK (after the fnspec fixes)? > > OK. > > Thanks, > Richard. > This caused: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97836 H.J.
> On Fri, Nov 13, 2020 at 12:07 AM Richard Biener <rguenther@suse.de> wrote: > > > > On Tue, 10 Nov 2020, Jan Hubicka wrote: > > > > > Hi, > > > here is updaed patch. > > > > > > Honza > > > > > > Bootstrapped/regtested x86_64-linux, OK (after the fnspec fixes)? > > > > OK. > > > > Thanks, > > Richard. > > > > This caused: > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97836 I have pushed the following fix as obvious. The problem is caused by fact that I made ipa-modref to set EAF_UNUSED flag for parameters that are used as scalars but not used to read memory since that suits the use of the flag in tree-ssa-alias and mostly tree-ssa-structalias with exception of the rule of escaping to return value. At monday I will disuss with Richard if we want to adjust EAF_UNUSED or invernt EAF_NOREAD, but this should prevent wrong code issues. Honza 2020-11-15 Jan Hubicka <hubicka@ucw.cz> PR ipa/97836 * ipa-modref.c (analyze_ssa_name_flags): Make return to clear EAF_UNUSED flag. gcc/testsuite/ChangeLog: 2020-11-15 Jan Hubicka <hubicka@ucw.cz> * gcc.c-torture/execute/pr97836.c: New test. diff --git a/gcc/ipa-modref.c b/gcc/ipa-modref.c index 5273c200f00..4a43c50aa66 100644 --- a/gcc/ipa-modref.c +++ b/gcc/ipa-modref.c @@ -1224,10 +1224,12 @@ analyze_ssa_name_flags (tree name, vec<unsigned char> &known_flags, int depth) print_gimple_stmt (dump_file, use_stmt, 0); } - /* Gimple return may load the return value. */ + /* Gimple return may load the return value. + Returning name counts as an use by tree-ssa-structalias.c */ if (greturn *ret = dyn_cast <greturn *> (use_stmt)) { - if (memory_access_to (gimple_return_retval (ret), name)) + if (memory_access_to (gimple_return_retval (ret), name) + || name == gimple_return_retval (ret)) flags &= ~EAF_UNUSED; } /* Account for LHS store, arg loads and flags from callee function. */ diff --git a/gcc/testsuite/gcc.c-torture/execute/pr97836.c b/gcc/testsuite/gcc.c-torture/execute/pr97836.c new file mode 100644 index 00000000000..4585e1fc69d --- /dev/null +++ b/gcc/testsuite/gcc.c-torture/execute/pr97836.c @@ -0,0 +1,17 @@ +int a; + +int b(int c) { return 0; } + +static int *d(int *e) { + if (a) { + a = a && b(*e); + } + return e; +} + +int main() { + int f; + if (d(&f) != &f) + __builtin_abort(); + return 0; +}
Hi Jan, >> > When I configure with ../configure --enable-languages=go my build >> > eventually finishes fine on curren trunk: >> [...] >> > On Debian SLES machines. Having preprocessed go-diagnostics.cc and >> > .uninit1 dump would probably help. >> >> apart from go/diagnostics.cc, I see the warning several times in >> libstdc++, but only for the 32-bit multilib. >> >> Try building either for i686-pc-linux-gnu or a bi-arch >> x86_64-pc-linux-gnu build. > > My build is bi-arch. I will check libstdc++ warnings, but we had some > such warnings previously there too (plus we have tons of uninitialized > warnings with LTO bootstrap). seems to be an issue on 32-bit hosts: I get the same failure on i686-pc-linux-gnu. > This really may depend on glibc headers and how string functions get > expanded. This is why I think having preprocessed diagnotics.cc > should help. This is Solaris, so no glibc in sight ;-) I'll send the Linux/i686 preprocessed source and uninit1 dump in private mail. Reproduce with ccplus -fpreprocessed go-diagnostics.ii -quiet -mtune=generic -march=pentiumpro -O2 -Wextra -Wall -Werror -o go-diagnostics.s Rainer
See PR97840. Andreas.
> See PR97840.
Thanks,
this is a false positive where we fail to discover that pointed-to type
is empty on non-x86_64 targets. This is triggered by better alias
analysis caused by non-escape discovery.
While this is not a full fix (I hope someone with more experience on
C++ types and warnings will set up) I think it may be useful to avoid
warning on unused parameter.
Bootstrapped/regtested x86_64-linux, OK?
PR middle-end/97840
* tree-ssa-uninit.c (maybe_warn_pass_by_reference): Update prototype;
silence warning on EAF_UNUSED parameters.
(warn_uninitialized_vars): Update.
diff --git a/gcc/tree-ssa-uninit.c b/gcc/tree-ssa-uninit.c
index f23514395e0..1e074793b02 100644
--- a/gcc/tree-ssa-uninit.c
+++ b/gcc/tree-ssa-uninit.c
@@ -443,7 +443,7 @@ maybe_warn_operand (ao_ref &ref, gimple *stmt, tree lhs, tree rhs,
access implying read access to those objects. */
static void
-maybe_warn_pass_by_reference (gimple *stmt, wlimits &wlims)
+maybe_warn_pass_by_reference (gcall *stmt, wlimits &wlims)
{
if (!wlims.wmaybe_uninit)
return;
@@ -501,6 +501,10 @@ maybe_warn_pass_by_reference (gimple *stmt, wlimits &wlims)
&& !TYPE_READONLY (TREE_TYPE (argtype)))
continue;
+ /* Ignore args we are not going to read from. */
+ if (gimple_call_arg_flags (stmt, argno - 1) & EAF_UNUSED)
+ continue;
+
if (save_always_executed && access->mode == access_read_only)
/* Attribute read_only arguments imply read access. */
wlims.always_executed = true;
@@ -639,8 +643,8 @@ warn_uninitialized_vars (bool wmaybe_uninit)
if (gimple_vdef (stmt))
wlims.vdef_cnt++;
- if (is_gimple_call (stmt))
- maybe_warn_pass_by_reference (stmt, wlims);
+ if (gcall *call = dyn_cast <gcall *> (stmt))
+ maybe_warn_pass_by_reference (call, wlims);
else if (gimple_assign_load_p (stmt)
&& gimple_has_location (stmt))
{
On Sun, 15 Nov 2020, Jan Hubicka wrote: > > See PR97840. > Thanks, > this is a false positive where we fail to discover that pointed-to type > is empty on non-x86_64 targets. This is triggered by better alias > analysis caused by non-escape discovery. > > While this is not a full fix (I hope someone with more experience on > C++ types and warnings will set up) I think it may be useful to avoid > warning on unused parameter. > > Bootstrapped/regtested x86_64-linux, OK? OK. > PR middle-end/97840 > * tree-ssa-uninit.c (maybe_warn_pass_by_reference): Update prototype; > silence warning on EAF_UNUSED parameters. > (warn_uninitialized_vars): Update. > diff --git a/gcc/tree-ssa-uninit.c b/gcc/tree-ssa-uninit.c > index f23514395e0..1e074793b02 100644 > --- a/gcc/tree-ssa-uninit.c > +++ b/gcc/tree-ssa-uninit.c > @@ -443,7 +443,7 @@ maybe_warn_operand (ao_ref &ref, gimple *stmt, tree lhs, tree rhs, > access implying read access to those objects. */ > > static void > -maybe_warn_pass_by_reference (gimple *stmt, wlimits &wlims) > +maybe_warn_pass_by_reference (gcall *stmt, wlimits &wlims) > { > if (!wlims.wmaybe_uninit) > return; > @@ -501,6 +501,10 @@ maybe_warn_pass_by_reference (gimple *stmt, wlimits &wlims) > && !TYPE_READONLY (TREE_TYPE (argtype))) > continue; > > + /* Ignore args we are not going to read from. */ > + if (gimple_call_arg_flags (stmt, argno - 1) & EAF_UNUSED) > + continue; > + > if (save_always_executed && access->mode == access_read_only) > /* Attribute read_only arguments imply read access. */ > wlims.always_executed = true; > @@ -639,8 +643,8 @@ warn_uninitialized_vars (bool wmaybe_uninit) > if (gimple_vdef (stmt)) > wlims.vdef_cnt++; > > - if (is_gimple_call (stmt)) > - maybe_warn_pass_by_reference (stmt, wlims); > + if (gcall *call = dyn_cast <gcall *> (stmt)) > + maybe_warn_pass_by_reference (call, wlims); > else if (gimple_assign_load_p (stmt) > && gimple_has_location (stmt)) > { >
On Nov 15 2020, Jan Hubicka wrote: >> See PR97840. > Thanks, > this is a false positive where we fail to discover that pointed-to type > is empty on non-x86_64 targets. This is triggered by better alias > analysis caused by non-escape discovery. > > While this is not a full fix (I hope someone with more experience on > C++ types and warnings will set up) I think it may be useful to avoid > warning on unused parameter. > > Bootstrapped/regtested x86_64-linux, OK? That doesn't fix anything. Andreas.
> On Nov 15 2020, Jan Hubicka wrote: > > >> See PR97840. > > Thanks, > > this is a false positive where we fail to discover that pointed-to type > > is empty on non-x86_64 targets. This is triggered by better alias > > analysis caused by non-escape discovery. > > > > While this is not a full fix (I hope someone with more experience on > > C++ types and warnings will set up) I think it may be useful to avoid > > warning on unused parameter. > > > > Bootstrapped/regtested x86_64-linux, OK? > > That doesn't fix anything. It does silence the warning if you remove inline from function declaration (as creduce while minimizing the testcase - the minimized testcase was undefined that is why I did not include it at the end). I now implemented one by hand. The reason is that gimple_call_arg_flags clears EAF_UNUSED on symbols that !binds_to_current_def_p because we are worried that symbol will be interposed by different version of the body with same semantics that will actually read the arg. This is bit paranoid check since we optimize things like *a == *a early but with clang *a will trap if a==NULL. Richi, I think we can add "safe" parameter to gimple_call_arg_flags and bypass this logic when we use it for warnings only (having body that does not use the value is quite strong hint that it is unused by the function). I played with bit more testcases and found that we also want to disable warning for const functions and sometimes EAF_UNUSED flag is dropped becaue of clobber that is not necessary to do. If function only clobber the target it can be considered unused past inlining. I am testing this improved patch and plan to commit if there are no complains, but still we need to handle binds_to_current_def. On the other direction, Martin, I think we may also warn for args that are !EAF_UNUSED and !EAF_NOCLOBBER. This will catch some cases where user did not add "const" specifier to the declaration but parameter is detected to be readonly. I also noticed that we do not detect EAF_UNUSED for fully unused parameters (with no SSA names associated with them). I will fix that incrementally. Honza PR middle-end/97840 * ipa-modref.c (analyze_ssa_name_flags): Skip clobbers if inlining is done. * tree-ssa-uninit.c (maybe_warn_pass_by_reference): Make stmt gcall; skip const calls and unused arguments. (warn_uninitialized_vars): Update prototype. gcc/testsuite/ChangeLog: 2020-11-16 Jan Hubicka <hubicka@ucw.cz> * g++.dg/warn/unit-2.C: New test. diff --git a/gcc/ipa-modref.c b/gcc/ipa-modref.c index 4a43c50aa66..08fcc36a321 100644 --- a/gcc/ipa-modref.c +++ b/gcc/ipa-modref.c @@ -1333,7 +1331,14 @@ analyze_ssa_name_flags (tree name, vec<unsigned char> &known_flags, int depth) /* Handle *name = exp. */ else if (assign && memory_access_to (gimple_assign_lhs (assign), name)) - flags &= ~(EAF_UNUSED | EAF_NOCLOBBER); + { + /* In general we can not ignore clobbers because they are + barriers for code motion, however after inlining it is safe to + do because local optimization passes do not consider clobbers + from other functions. Similar logic is in ipa-pure-const.c. */ + if (!cfun->after_inlining && !gimple_clobber_p (assign)) + flags &= ~(EAF_UNUSED | EAF_NOCLOBBER); + } /* ASM statements etc. */ else if (!assign) { diff --git a/gcc/testsuite/g++.dg/warn/unit-2.C b/gcc/testsuite/g++.dg/warn/unit-2.C new file mode 100644 index 00000000000..30f3ceae191 --- /dev/null +++ b/gcc/testsuite/g++.dg/warn/unit-2.C @@ -0,0 +1,29 @@ +/* { dg-do compile } */ +/* { dg-options "-O2 -Wmaybe-uninitialized" } */ +struct a {int a;}; +__attribute__ ((noinline)) +void +nowarn (const struct a *ptr) +{ + if (ptr) + asm volatile (""); +} +void +test() +{ + struct a ptr; + nowarn (&ptr); +} +__attribute__ ((noinline)) +int +nowarn2 (const struct a *ptr, const struct a ptr2) +{ + return ptr != 0 || ptr2.a; +} +int mem; +int +test2() +{ + struct a ptr,ptr2={0}; + return nowarn2 (&ptr, ptr2); +} diff --git a/gcc/tree-ssa-uninit.c b/gcc/tree-ssa-uninit.c index f23514395e0..c94831bfb75 100644 --- a/gcc/tree-ssa-uninit.c +++ b/gcc/tree-ssa-uninit.c @@ -443,7 +443,7 @@ maybe_warn_operand (ao_ref &ref, gimple *stmt, tree lhs, tree rhs, access implying read access to those objects. */ static void -maybe_warn_pass_by_reference (gimple *stmt, wlimits &wlims) +maybe_warn_pass_by_reference (gcall *stmt, wlimits &wlims) { if (!wlims.wmaybe_uninit) return; @@ -457,6 +457,10 @@ maybe_warn_pass_by_reference (gimple *stmt, wlimits &wlims) if (!fntype) return; + /* Const function do not read their arguments. */ + if (gimple_call_flags (stmt) & ECF_CONST) + return; + const built_in_function fncode = (fndecl && gimple_call_builtin_p (stmt, BUILT_IN_NORMAL) ? DECL_FUNCTION_CODE (fndecl) : (built_in_function)BUILT_IN_LAST); @@ -523,6 +527,10 @@ maybe_warn_pass_by_reference (gimple *stmt, wlimits &wlims) (but not definitive) read access. */ wlims.always_executed = false; + /* Ignore args we are not going to read from. */ + if (gimple_call_arg_flags (stmt, argno - 1) & EAF_UNUSED) + continue; + tree arg = gimple_call_arg (stmt, argno - 1); ao_ref ref; @@ -639,8 +647,8 @@ warn_uninitialized_vars (bool wmaybe_uninit) if (gimple_vdef (stmt)) wlims.vdef_cnt++; - if (is_gimple_call (stmt)) - maybe_warn_pass_by_reference (stmt, wlims); + if (gcall *call = dyn_cast <gcall *> (stmt)) + maybe_warn_pass_by_reference (call, wlims); else if (gimple_assign_load_p (stmt) && gimple_has_location (stmt)) {
On Mon, 16 Nov 2020, Jan Hubicka wrote: > > On Nov 15 2020, Jan Hubicka wrote: > > > > >> See PR97840. > > > Thanks, > > > this is a false positive where we fail to discover that pointed-to type > > > is empty on non-x86_64 targets. This is triggered by better alias > > > analysis caused by non-escape discovery. > > > > > > While this is not a full fix (I hope someone with more experience on > > > C++ types and warnings will set up) I think it may be useful to avoid > > > warning on unused parameter. > > > > > > Bootstrapped/regtested x86_64-linux, OK? > > > > That doesn't fix anything. > > It does silence the warning if you remove inline from function > declaration (as creduce while minimizing the testcase - the minimized > testcase was undefined that is why I did not include it at the end). > I now implemented one by hand. > > The reason is that gimple_call_arg_flags clears EAF_UNUSED > on symbols that !binds_to_current_def_p because we are worried that > symbol will be interposed by different version of the body with > same semantics that will actually read the arg. > This is bit paranoid check since we optimize things like *a == *a early > but with clang *a will trap if a==NULL. > > Richi, I think we can add "safe" parameter to gimple_call_arg_flags and > bypass this logic when we use it for warnings only (having body that > does not use the value is quite strong hint that it is unused by the > function). Eh, please not. > > I played with bit more testcases and found that we also want to disable > warning for const functions and sometimes EAF_UNUSED flag is dropped > becaue of clobber that is not necessary to do. If function only clobber > the target it can be considered unused past inlining. > > I am testing this improved patch and plan to commit if there are no > complains, but still we need to handle binds_to_current_def. > > On the other direction, Martin, I think we may also warn for args > that are !EAF_UNUSED and !EAF_NOCLOBBER. This will catch some cases > where user did not add "const" specifier to the declaration but > parameter is detected to be readonly. > > I also noticed that we do not detect EAF_UNUSED for fully unused > parameters (with no SSA names associated with them). I will fix that > incrementally. Make sure to not apply it based on that reason to aggregates ;) > Honza > > PR middle-end/97840 > * ipa-modref.c (analyze_ssa_name_flags): Skip clobbers if inlining > is done. > * tree-ssa-uninit.c (maybe_warn_pass_by_reference): Make stmt gcall; > skip const calls and unused arguments. > (warn_uninitialized_vars): Update prototype. > > gcc/testsuite/ChangeLog: > > 2020-11-16 Jan Hubicka <hubicka@ucw.cz> > > * g++.dg/warn/unit-2.C: New test. > > diff --git a/gcc/ipa-modref.c b/gcc/ipa-modref.c > index 4a43c50aa66..08fcc36a321 100644 > --- a/gcc/ipa-modref.c > +++ b/gcc/ipa-modref.c > @@ -1333,7 +1331,14 @@ analyze_ssa_name_flags (tree name, vec<unsigned char> &known_flags, int depth) > /* Handle *name = exp. */ > else if (assign > && memory_access_to (gimple_assign_lhs (assign), name)) > - flags &= ~(EAF_UNUSED | EAF_NOCLOBBER); > + { > + /* In general we can not ignore clobbers because they are > + barriers for code motion, however after inlining it is safe to > + do because local optimization passes do not consider clobbers > + from other functions. Similar logic is in ipa-pure-const.c. */ > + if (!cfun->after_inlining && !gimple_clobber_p (assign)) > + flags &= ~(EAF_UNUSED | EAF_NOCLOBBER); > + } > /* ASM statements etc. */ > else if (!assign) > { > diff --git a/gcc/testsuite/g++.dg/warn/unit-2.C b/gcc/testsuite/g++.dg/warn/unit-2.C > new file mode 100644 > index 00000000000..30f3ceae191 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/warn/unit-2.C > @@ -0,0 +1,29 @@ > +/* { dg-do compile } */ > +/* { dg-options "-O2 -Wmaybe-uninitialized" } */ > +struct a {int a;}; > +__attribute__ ((noinline)) > +void > +nowarn (const struct a *ptr) > +{ > + if (ptr) > + asm volatile (""); > +} > +void > +test() > +{ > + struct a ptr; > + nowarn (&ptr); > +} > +__attribute__ ((noinline)) > +int > +nowarn2 (const struct a *ptr, const struct a ptr2) > +{ > + return ptr != 0 || ptr2.a; > +} > +int mem; > +int > +test2() > +{ > + struct a ptr,ptr2={0}; > + return nowarn2 (&ptr, ptr2); > +} > diff --git a/gcc/tree-ssa-uninit.c b/gcc/tree-ssa-uninit.c > index f23514395e0..c94831bfb75 100644 > --- a/gcc/tree-ssa-uninit.c > +++ b/gcc/tree-ssa-uninit.c > @@ -443,7 +443,7 @@ maybe_warn_operand (ao_ref &ref, gimple *stmt, tree lhs, tree rhs, > access implying read access to those objects. */ > > static void > -maybe_warn_pass_by_reference (gimple *stmt, wlimits &wlims) > +maybe_warn_pass_by_reference (gcall *stmt, wlimits &wlims) > { > if (!wlims.wmaybe_uninit) > return; > @@ -457,6 +457,10 @@ maybe_warn_pass_by_reference (gimple *stmt, wlimits &wlims) > if (!fntype) > return; > > + /* Const function do not read their arguments. */ > + if (gimple_call_flags (stmt) & ECF_CONST) > + return; > + > const built_in_function fncode > = (fndecl && gimple_call_builtin_p (stmt, BUILT_IN_NORMAL) > ? DECL_FUNCTION_CODE (fndecl) : (built_in_function)BUILT_IN_LAST); > @@ -523,6 +527,10 @@ maybe_warn_pass_by_reference (gimple *stmt, wlimits &wlims) > (but not definitive) read access. */ > wlims.always_executed = false; > > + /* Ignore args we are not going to read from. */ > + if (gimple_call_arg_flags (stmt, argno - 1) & EAF_UNUSED) > + continue; > + > tree arg = gimple_call_arg (stmt, argno - 1); > > ao_ref ref; > @@ -639,8 +647,8 @@ warn_uninitialized_vars (bool wmaybe_uninit) > if (gimple_vdef (stmt)) > wlims.vdef_cnt++; > > - if (is_gimple_call (stmt)) > - maybe_warn_pass_by_reference (stmt, wlims); > + if (gcall *call = dyn_cast <gcall *> (stmt)) > + maybe_warn_pass_by_reference (call, wlims); > else if (gimple_assign_load_p (stmt) > && gimple_has_location (stmt)) > { >
> > > > Richi, I think we can add "safe" parameter to gimple_call_arg_flags and > > bypass this logic when we use it for warnings only (having body that > > does not use the value is quite strong hint that it is unused by the > > function). > > Eh, please not. OK, I do not care that much as long as we do not have false positives everywhere :) Hadling EAF_UNUSED and CONST functions is necessary so we do not get false positive caused by us optimizing code out. In this case of not trusing EAF_UNUSED flag we will not optimize, so I do not really care. Martin, we collected very many warnings when building with configure --with-build-config=bootstrap-lto.mk This patch fixes some of them, but there are many others, can you take a look? For the testcase in PR I think it is enough to use default_is_empty_type to disable the warning, but it is not clear to me why the code uses default_is_empty_record at first place. > > > > > I played with bit more testcases and found that we also want to disable > > warning for const functions and sometimes EAF_UNUSED flag is dropped > > becaue of clobber that is not necessary to do. If function only clobber > > the target it can be considered unused past inlining. > > > > I am testing this improved patch and plan to commit if there are no > > complains, but still we need to handle binds_to_current_def. > > > > On the other direction, Martin, I think we may also warn for args > > that are !EAF_UNUSED and !EAF_NOCLOBBER. This will catch some cases > > where user did not add "const" specifier to the declaration but > > parameter is detected to be readonly. > > > > I also noticed that we do not detect EAF_UNUSED for fully unused > > parameters (with no SSA names associated with them). I will fix that > > incrementally. > > Make sure to not apply it based on that reason to aggregates ;) Sure, we already have detection of unused params in ipa-prop, so I guess we want is_giple_ref (parm) && !default_def to imply EAF_UNUSED. Honza
On 11/16/20 1:44 PM, Jan Hubicka wrote: > Martin, we collected very many warnings when building with > configure --with-build-config=bootstrap-lto.mk > This patch fixes some of them, but there are many others, can you take a > look? Hello. I guess you mean Martin Jambor, or me? Please CC :) Martin
> On 11/16/20 1:44 PM, Jan Hubicka wrote: > > Martin, we collected very many warnings when building with > > configure --with-build-config=bootstrap-lto.mk > > This patch fixes some of them, but there are many others, can you take a > > look? > > Hello. > > I guess you mean Martin Jambor, or me? Please CC :) Martin Sebor, added to CC (since he did most of work on tree-ssa-uninit recently) I filled some PRs on the isues now. Honza > > Martin
diff --git a/gcc/builtins.c b/gcc/builtins.c index da25343beb1..c55f54e2aef 100644 --- a/gcc/builtins.c +++ b/gcc/builtins.c @@ -12939,16 +12939,16 @@ builtin_fnspec (tree callee) argument. */ case BUILT_IN_STRCAT: case BUILT_IN_STRCAT_CHK: - return "1cW R "; + return "1cW r "; case BUILT_IN_STRNCAT: case BUILT_IN_STRNCAT_CHK: - return "1cW R3"; + return "1cW r3"; case BUILT_IN_STRCPY: case BUILT_IN_STRCPY_CHK: - return "1cO R "; + return "1cO r "; case BUILT_IN_STPCPY: case BUILT_IN_STPCPY_CHK: - return ".cO R "; + return ".cO r "; case BUILT_IN_STRNCPY: case BUILT_IN_MEMCPY: case BUILT_IN_MEMMOVE: @@ -12957,15 +12957,15 @@ builtin_fnspec (tree callee) case BUILT_IN_STRNCPY_CHK: case BUILT_IN_MEMCPY_CHK: case BUILT_IN_MEMMOVE_CHK: - return "1cO3R3"; + return "1cO3r3"; case BUILT_IN_MEMPCPY: case BUILT_IN_MEMPCPY_CHK: - return ".cO3R3"; + return ".cO3r3"; case BUILT_IN_STPNCPY: case BUILT_IN_STPNCPY_CHK: - return ".cO3R3"; + return ".cO3r3"; case BUILT_IN_BCOPY: - return ".cR3O3"; + return ".cr3O3"; case BUILT_IN_BZERO: return ".cO2"; case BUILT_IN_MEMCMP: diff --git a/gcc/gimple.c b/gcc/gimple.c index 1afed88e1f1..72a0d05580a 100644 --- a/gcc/gimple.c +++ b/gcc/gimple.c @@ -46,6 +46,8 @@ along with GCC; see the file COPYING3. If not see #include "asan.h" #include "langhooks.h" #include "attr-fnspec.h" +#include "ipa-modref-tree.h" +#include "ipa-modref.h" /* All the tuples have their operand vector (if present) at the very bottom @@ -1532,24 +1534,45 @@ int gimple_call_arg_flags (const gcall *stmt, unsigned arg) { attr_fnspec fnspec = gimple_call_fnspec (stmt); - - if (!fnspec.known_p ()) - return 0; - int flags = 0; - if (!fnspec.arg_specified_p (arg)) - ; - else if (!fnspec.arg_used_p (arg)) - flags = EAF_UNUSED; - else + if (fnspec.known_p ()) { - if (fnspec.arg_direct_p (arg)) - flags |= EAF_DIRECT; - if (fnspec.arg_noescape_p (arg)) - flags |= EAF_NOESCAPE; - if (fnspec.arg_readonly_p (arg)) - flags |= EAF_NOCLOBBER; + if (!fnspec.arg_specified_p (arg)) + ; + else if (!fnspec.arg_used_p (arg)) + flags = EAF_UNUSED; + else + { + if (fnspec.arg_direct_p (arg)) + flags |= EAF_DIRECT; + if (fnspec.arg_noescape_p (arg)) + flags |= EAF_NOESCAPE; + if (fnspec.arg_readonly_p (arg)) + flags |= EAF_NOCLOBBER; + } + } + tree callee = gimple_call_fndecl (stmt); + if (callee) + { + cgraph_node *node = cgraph_node::get (callee); + modref_summary *summary = node ? get_modref_function_summary (node) + : NULL; + + if (summary && summary->arg_flags.length () > arg) + { + int modref_flags = summary->arg_flags [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; + if ((modref_flags & EAF_DIRECT) && !(flags & EAF_DIRECT)) + modref_flags &= ~EAF_DIRECT; + } + flags |= modref_flags; + } } return flags; } diff --git a/gcc/ipa-modref.c b/gcc/ipa-modref.c index 3f46bebed3c..6df4c8fbf68 100644 --- a/gcc/ipa-modref.c +++ b/gcc/ipa-modref.c @@ -61,6 +61,12 @@ along with GCC; see the file COPYING3. If not see #include "ipa-fnsummary.h" #include "attr-fnspec.h" #include "symtab-clones.h" +#include "gimple-ssa.h" +#include "tree-phinodes.h" +#include "tree-ssa-operands.h" +#include "ssa-iterators.h" +#include "stringpool.h" +#include "tree-ssanames.h" /* We record fnspec specifiers for call edges since they depends on actual gimple statements. */ @@ -186,6 +192,8 @@ modref_summary::useful_p (int ecf_flags) { if (ecf_flags & (ECF_CONST | ECF_NOVOPS)) return false; + if (arg_flags.length ()) + return true; if (loads && !loads->every_base) return true; if (ecf_flags & ECF_PURE) @@ -355,6 +363,22 @@ dump_lto_records (modref_records_lto *tt, FILE *out) } } +/* Dump EAF flags. */ + +static void +dump_eaf_flags (FILE *out, int flags) +{ + if (flags & EAF_DIRECT) + fprintf (out, " direct"); + if (flags & EAF_NOCLOBBER) + fprintf (out, " noclobber"); + if (flags & EAF_NOESCAPE) + fprintf (out, " noescape"); + if (flags & EAF_UNUSED) + fprintf (out, " unused"); + fprintf (out, "\n"); +} + /* Dump summary. */ void @@ -372,6 +396,15 @@ modref_summary::dump (FILE *out) } if (writes_errno) fprintf (out, " Writes errno\n"); + if (arg_flags.length ()) + { + for (unsigned int i = 0; i < arg_flags.length (); i++) + if (arg_flags[i]) + { + fprintf (out, " parm %i flags:", i); + dump_eaf_flags (out, arg_flags[i]); + } + } } /* Dump summary. */ @@ -402,7 +435,8 @@ get_modref_function_summary (cgraph_node *func) function. */ enum availability avail; func = func->function_or_virtual_thunk_symbol - (&avail, cgraph_node::get (current_function_decl)); + (&avail, current_function_decl ? + cgraph_node::get (current_function_decl) : NULL); if (avail <= AVAIL_INTERPOSABLE) return NULL; @@ -1067,6 +1101,266 @@ remove_summary (bool lto, bool nolto, bool ipa) " - modref done with result: not tracked.\n"); } +/* Return true if OP accesses memory pointed to by SSA_NAME. */ + +bool +memory_access_to (tree op, tree ssa_name) +{ + tree base = get_base_address (op); + if (!base) + return false; + if (TREE_CODE (base) != MEM_REF && TREE_CODE (base) != TARGET_MEM_REF) + return false; + return TREE_OPERAND (base, 0) == ssa_name; +} + +/* We have val = *arg. + return EAF flags of ARG that can be determined from EAF flags of VAL + (which are known to be FLAGS). If IGNORE_STORES is true we can ignore + all stores to VAL, i.e. when handling noreturn function. */ + +static int +deref_flags (int flags, bool ignore_stores) +{ + int ret = 0; + if (flags & EAF_UNUSED) + ret |= EAF_DIRECT | EAF_NOCLOBBER | EAF_NOESCAPE; + else + { + if ((flags & EAF_NOCLOBBER) || ignore_stores) + ret |= EAF_NOCLOBBER; + if ((flags & EAF_NOESCAPE) || ignore_stores) + ret |= EAF_NOESCAPE; + } + return ret; +} + +/* Analyze EAF flags for SSA name NAME. + KNOWN_FLAGS is a cache for flags we already determined. */ + +static int +analyze_ssa_name_flags (tree name, vec<int> *known_flags, int depth) +{ + imm_use_iterator ui; + gimple *use_stmt; + int flags = EAF_DIRECT | EAF_NOCLOBBER | EAF_NOESCAPE | EAF_UNUSED; + + /* See if value is already computed. */ + if ((*known_flags)[SSA_NAME_VERSION (name)]) + { + /* Punt on cycles for now, so we do not need dataflow. */ + if ((*known_flags)[SSA_NAME_VERSION (name)] == 1) + { + if (dump_file) + fprintf (dump_file, + "%*sGiving up on a cycle in SSA graph\n", depth * 4, ""); + return 0; + } + return (*known_flags)[SSA_NAME_VERSION (name)] - 2; + } + /* Recursion guard. */ + (*known_flags)[SSA_NAME_VERSION (name)] = 1; + + if (dump_file) + { + fprintf (dump_file, + "%*sAnalyzing flags of ssa name: ", depth * 4, ""); + print_generic_expr (dump_file, name); + fprintf (dump_file, "\n"); + } + + FOR_EACH_IMM_USE_STMT (use_stmt, ui, name) + { + if (flags == 0) + { + BREAK_FROM_IMM_USE_STMT (ui); + } + if (dump_file) + { + fprintf (dump_file, "%*s Analyzing stmt:", depth * 4, ""); + print_gimple_stmt (dump_file, use_stmt, 0); + } + + /* Gimple return may load the return value. */ + if (gimple_code (use_stmt) == GIMPLE_RETURN) + { + if (memory_access_to (gimple_return_retval + (as_a <greturn *> (use_stmt)), name)) + flags &= ~EAF_UNUSED; + } + /* Account for LHS store, arg loads and flags from callee function. */ + else if (is_gimple_call (use_stmt)) + { + tree callee = gimple_call_fndecl (use_stmt); + + /* Recursion would require bit of propagation; give up for now. */ + if (callee && recursive_call_p (current_function_decl, callee)) + flags = 0; + else + { + int ecf_flags = gimple_call_flags (use_stmt); + bool ignore_stores = ignore_stores_p (current_function_decl, + ecf_flags); + + /* Handle *name = func (...). */ + if (gimple_call_lhs (use_stmt) + && memory_access_to (gimple_call_lhs (use_stmt), name)) + flags &= ~(EAF_UNUSED | EAF_NOCLOBBER); + + /* Handle all function parameters. */ + for (unsigned i = 0; i < gimple_call_num_args (use_stmt); i++) + /* Name is directly passed to the callee. */ + if (gimple_call_arg (use_stmt, i) == name + && !(ecf_flags & (ECF_CONST | ECF_NOVOPS))) + { + int call_flags = gimple_call_arg_flags (as_a <gcall *> + (use_stmt), i); + if (ignore_stores) + call_flags |= EAF_NOCLOBBER | EAF_NOESCAPE; + + flags &= call_flags; + } + /* Name is dereferenced and passed to a callee. */ + else if (memory_access_to (gimple_call_arg (use_stmt, i), name)) + { + if (ecf_flags & (ECF_CONST | ECF_NOVOPS)) + flags &= ~EAF_UNUSED; + else + flags &= deref_flags (gimple_call_arg_flags + (as_a <gcall *> (use_stmt), i), + ignore_stores); + } + } + } + else if (gimple_assign_load_p (use_stmt)) + { + /* Memory to memory copy. */ + if (gimple_store_p (use_stmt)) + { + /* Handle *name = *exp. */ + if (memory_access_to (gimple_assign_lhs (use_stmt), name)) + flags &= ~(EAF_UNUSED | EAF_NOCLOBBER); + + /* Handle *lhs = *name. + + We do not track memory locations, so assume that value + is used arbitrarily. */ + if (memory_access_to (gimple_assign_rhs1 (use_stmt), name)) + flags = 0; + } + /* Handle lhs = *name. */ + else if (memory_access_to (gimple_assign_rhs1 (use_stmt), name)) + flags &= deref_flags (analyze_ssa_name_flags + (gimple_assign_lhs (use_stmt), + known_flags, depth + 1), false); + } + else if (gimple_store_p (use_stmt)) + { + /* Handle *lhs = name. */ + if (is_gimple_assign (use_stmt) + && gimple_assign_rhs1 (use_stmt) == name) + { + if (dump_file) + fprintf (dump_file, "%*s ssa name saved to memory\n", + depth * 4, ""); + flags = 0; + } + /* Handle *name = exp. */ + else if (is_gimple_assign (use_stmt) + && memory_access_to (gimple_assign_lhs (use_stmt), name)) + flags &= ~(EAF_UNUSED | EAF_NOCLOBBER); + /* ASM statements etc. */ + else if (!is_gimple_assign (use_stmt)) + { + if (dump_file) + fprintf (dump_file, "%*s Unhandled store\n", + depth * 4, ""); + flags = 0; + } + } + else if (is_gimple_assign (use_stmt)) + { + enum tree_code code = gimple_assign_rhs_code (use_stmt); + + /* See if operation is a merge as considered by + tree-ssa-structalias.c:find_func_aliases. */ + if (!truth_value_p (code) + && code != POINTER_DIFF_EXPR + && (code != POINTER_PLUS_EXPR + || gimple_assign_rhs1 (use_stmt) == name)) + flags &= analyze_ssa_name_flags + (gimple_assign_lhs (use_stmt), known_flags, + depth + 1); + } + else if (gimple_code (use_stmt) == GIMPLE_PHI) + { + flags &= analyze_ssa_name_flags + (gimple_phi_result (use_stmt), known_flags, + depth + 1); + } + /* Conditions are not considered escape points + by tree-ssa-structalias. */ + else if (gimple_code (use_stmt) == GIMPLE_COND) + ; + else + { + if (dump_file) + fprintf (dump_file, "%*s Unhandled stmt\n", depth * 4, ""); + flags = 0; + } + + if (dump_file) + { + fprintf (dump_file, "%*s current flags of ", depth * 4, ""); + print_generic_expr (dump_file, name); + dump_eaf_flags (dump_file, flags); + } + } + if (dump_file) + { + fprintf (dump_file, "%*sflags of ssa name ", depth * 4, ""); + print_generic_expr (dump_file, name); + dump_eaf_flags (dump_file, flags); + } + (*known_flags)[SSA_NAME_VERSION (name)] = flags + 2; + return flags; +} + +/* Determine EAF flags for function parameters. */ + +static void +analyze_parms (modref_summary *summary) +{ + unsigned int parm_index = 0; + unsigned int count = 0; + + for (tree parm = DECL_ARGUMENTS (current_function_decl); parm; + parm = TREE_CHAIN (parm)) + count++; + + if (!count) + return; + + auto_vec<int> known_flags; + known_flags.safe_grow_cleared (num_ssa_names); + + for (tree parm = DECL_ARGUMENTS (current_function_decl); parm; parm_index++, + parm = TREE_CHAIN (parm)) + { + tree name = ssa_default_def (cfun, parm); + if (!name) + continue; + int flags = analyze_ssa_name_flags (name, &known_flags, 0); + + if (flags) + { + if (parm_index >= summary->arg_flags.length ()) + summary->arg_flags.safe_grow_cleared (count); + summary->arg_flags [parm_index] = flags; + } + } +} + /* Analyze function F. IPA indicates whether we're running in local mode (false) or the IPA mode (true). */ @@ -1174,6 +1468,10 @@ analyze_function (function *f, bool ipa) param_modref_max_accesses); summary_lto->writes_errno = false; } + + if (!ipa) + analyze_parms (summary); + int ecf_flags = flags_from_decl_or_type (current_function_decl); auto_vec <gimple *, 32> recursive_calls; @@ -1191,8 +1489,9 @@ analyze_function (function *f, bool ipa) || ((!summary || !summary->useful_p (ecf_flags)) && (!summary_lto || !summary_lto->useful_p (ecf_flags)))) { - remove_summary (lto, nolto, ipa); - return; + collapse_loads (summary, summary_lto); + collapse_stores (summary, summary_lto); + break; } } } diff --git a/gcc/ipa-modref.h b/gcc/ipa-modref.h index 31ceffa8d34..8fa05aaf7fb 100644 --- a/gcc/ipa-modref.h +++ b/gcc/ipa-modref.h @@ -29,6 +29,7 @@ struct GTY(()) modref_summary /* Load and stores in function (transitively closed to all callees) */ modref_records *loads; modref_records *stores; + auto_vec<int> GTY((skip)) arg_flags; modref_summary (); ~modref_summary (); diff --git a/gcc/testsuite/gcc.dg/torture/pta-ptrarith-1.c b/gcc/testsuite/gcc.dg/torture/pta-ptrarith-1.c index 99a548840df..85b68068b12 100644 --- a/gcc/testsuite/gcc.dg/torture/pta-ptrarith-1.c +++ b/gcc/testsuite/gcc.dg/torture/pta-ptrarith-1.c @@ -6,11 +6,14 @@ struct Foo { int *p; }; +struct Foo *ff; + void __attribute__((noinline)) foo (void *p) { struct Foo *f = (struct Foo *)p - 1; *f->p = 0; + ff = f; } int bar (void)