On Fri, 29 Oct 2021, Jan Hubicka wrote:
> Hi,
> this patch extends modref and tree-ssa-structalias to handle retslot flags.
> Since retslot it essentially a hidden argument that is known to be write-only
> we can do pretty much the same stuff as we do for regular parameters.
> I plan to add static chain handling similar way.
>
> We do not handle IPA propagation of retslot flags (where return slot is
> initialized via return slot of other function). For this ipa-prop needs
> to be extended to understand retslot as well.
>
> Martin, I wonder if we could look into the ipa-prop bits as well as
> dropping ancessor functions and adding jump functions for return
> functions (which initially does not need to be used by ipa-cp avoiding
> your problem with forming non-trivial SCCS on acyclic callgraph).
> They would be immediatly useful for modref and work on ipa-cp can be
> done incrementally.
>
> Bootstrapped/regtested x86_64-linux, OK for the gimple bits?
OK.
Thanks,
Richard.
> Honz
>
> gcc/ChangeLog:
>
> * gimple.c (gimple_call_retslot_flags): New function.
> * gimple.h (gimple_call_retslot_flags): Declare.
> * ipa-modref.c: Include tree-cfg.h.
> (struct escape_entry): Turn parm_index to signed.
> (modref_summary_lto::modref_summary_lto): Add retslot_flags.
> (modref_summary::modref_summary): Initialize retslot_flags.
> (struct modref_summary_lto): Likewise.
> (modref_summary::useful_p): Check retslot_flags.
> (modref_summary_lto::useful_p): Likewise.
> (modref_summary::dump): Dump retslot_flags.
> (modref_summary_lto::dump): Likewise.
> (struct escape_point): Add hidden_args enum.
> (analyze_ssa_name_flags): Ignore return slot return;
> use gimple_call_retslot_flags.
> (record_escape_points): Break out from ...
> (analyze_parms): ... here; handle retslot_flags.
> (modref_summaries::duplicate): Duplicate retslot_flags.
> (modref_summaries_lto::duplicate): Likewise.
> (modref_write_escape_summary): Stream parm_index as signed.
> (modref_read_escape_summary): Likewise.
> (modref_write): Stream retslot_flags.
> (read_section): Likewise.
> (struct escape_map): Fix typo in comment.
> (update_escape_summary_1): Fix whitespace.
> (ipa_merge_modref_summary_after_inlining): Drop retslot_flags.
> (modref_merge_call_site_flags): Merge retslot_flags.
> * ipa-modref.h (struct modref_summary): Add retslot_flags.
> * tree-ssa-structalias.c (handle_rhs_call): Handle retslot_flags.
>
> diff --git a/gcc/gimple.c b/gcc/gimple.c
> index cc7a88e822b..22dd6417d19 100644
> --- a/gcc/gimple.c
> +++ b/gcc/gimple.c
> @@ -1608,6 +1613,40 @@ gimple_call_arg_flags (const gcall *stmt, unsigned arg)
> return flags;
> }
>
> +/* Detects argument flags for return slot on call STMT. */
> +
> +int
> +gimple_call_retslot_flags (const gcall *stmt)
> +{
> + int flags = EAF_DIRECT | EAF_NOREAD;
> +
> + 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)
> + {
> + int modref_flags = summary->retslot_flags;
> +
> + /* We have possibly optimized out load. Be conservative here. */
> + if (!node->binds_to_current_def_p ())
> + {
> + if ((modref_flags & EAF_UNUSED) && !(flags & EAF_UNUSED))
> + {
> + modref_flags &= ~EAF_UNUSED;
> + modref_flags |= EAF_NOESCAPE;
> + }
> + }
> + if (dbg_cnt (ipa_mod_ref_pta))
> + flags |= modref_flags;
> + }
> + }
> + return flags;
> +}
> +
> /* Detects return flags for the call STMT. */
>
> int
> diff --git a/gcc/gimple.h b/gcc/gimple.h
> index 303623b3ced..23a124ec769 100644
> --- a/gcc/gimple.h
> +++ b/gcc/gimple.h
> @@ -1589,6 +1589,7 @@ gimple_seq gimple_seq_copy (gimple_seq);
> bool gimple_call_same_target_p (const gimple *, const gimple *);
> int gimple_call_flags (const gimple *);
> int gimple_call_arg_flags (const gcall *, unsigned);
> +int gimple_call_retslot_flags (const gcall *);
> int gimple_call_return_flags (const gcall *);
> bool gimple_call_nonnull_result_p (gcall *);
> tree gimple_call_nonnull_arg (gcall *);
> diff --git a/gcc/ipa-modref.c b/gcc/ipa-modref.c
> index 0bbec8df0a2..4c59194c521 100644
> --- a/gcc/ipa-modref.c
> +++ b/gcc/ipa-modref.c
> @@ -86,6 +86,7 @@ along with GCC; see the file COPYING3. If not see
> #include "stringpool.h"
> #include "tree-ssanames.h"
> #include "attribs.h"
> +#include "tree-cfg.h"
>
>
> namespace {
> @@ -133,7 +134,7 @@ static fnspec_summaries_t *fnspec_summaries = NULL;
> struct escape_entry
> {
> /* Parameter that escapes at a given call. */
> - unsigned int parm_index;
> + int parm_index;
> /* Argument it escapes to. */
> unsigned int arg;
> /* Minimal flags known about the argument. */
> @@ -269,7 +270,7 @@ static GTY(()) fast_function_summary <modref_summary_lto
> *, va_gc>
> /* Summary for a single function which this pass produces. */
>
> modref_summary::modref_summary ()
> - : loads (NULL), stores (NULL), writes_errno (NULL)
> + : loads (NULL), stores (NULL), retslot_flags (0), writes_errno (false)
> {
> }
>
> @@ -322,6 +323,8 @@ modref_summary::useful_p (int ecf_flags, bool check_flags)
> if (check_flags && eaf_flags_useful_p (arg_flags, ecf_flags))
> return true;
> arg_flags.release ();
> + if (check_flags && remove_useless_eaf_flags (retslot_flags, ecf_flags,
> false))
> + return true;
> if (ecf_flags & ECF_CONST)
> return false;
> if (loads && !loads->every_base)
> @@ -363,6 +366,7 @@ struct GTY(()) modref_summary_lto
> modref_records_lto *loads;
> modref_records_lto *stores;
> auto_vec<eaf_flags_t> GTY((skip)) arg_flags;
> + eaf_flags_t retslot_flags;
> bool writes_errno;
>
> modref_summary_lto ();
> @@ -374,7 +378,7 @@ struct GTY(()) modref_summary_lto
> /* Summary for a single function which this pass produces. */
>
> modref_summary_lto::modref_summary_lto ()
> - : loads (NULL), stores (NULL), writes_errno (NULL)
> + : loads (NULL), stores (NULL), retslot_flags (0), writes_errno (false)
> {
> }
>
> @@ -400,6 +404,8 @@ modref_summary_lto::useful_p (int ecf_flags, bool
> check_flags)
> if (check_flags && eaf_flags_useful_p (arg_flags, ecf_flags))
> return true;
> arg_flags.release ();
> + if (check_flags && remove_useless_eaf_flags (retslot_flags, ecf_flags,
> false))
> + return true;
> if (ecf_flags & ECF_CONST)
> return false;
> if (loads && !loads->every_base)
> @@ -608,6 +614,11 @@ modref_summary::dump (FILE *out)
> dump_eaf_flags (out, arg_flags[i]);
> }
> }
> + if (retslot_flags)
> + {
> + fprintf (out, " Retslot flags:");
> + dump_eaf_flags (out, retslot_flags);
> + }
> }
>
> /* Dump summary. */
> @@ -630,6 +641,11 @@ modref_summary_lto::dump (FILE *out)
> dump_eaf_flags (out, arg_flags[i]);
> }
> }
> + if (retslot_flags)
> + {
> + fprintf (out, " Retslot flags:\n");
> + dump_eaf_flags (out, retslot_flags);
> + }
> }
>
> /* Get function summary for FUNC if it exists, return NULL otherwise. */
> @@ -1396,6 +1412,11 @@ namespace {
>
> struct escape_point
> {
> + /* Extra hidden args we keep track of. */
> + enum hidden_args
> + {
> + retslot_arg = -1
> + };
> /* Value escapes to this call. */
> gcall *call;
> /* Argument it escapes to. */
> @@ -1705,7 +1726,11 @@ analyze_ssa_name_flags (tree name, vec<modref_lattice>
> &lattice, int depth,
> Returning name counts as an use by tree-ssa-structalias.c */
> if (greturn *ret = dyn_cast <greturn *> (use_stmt))
> {
> - if (gimple_return_retval (ret) == name)
> + /* Returning through return slot is seen as memory write earlier. */
> + if (DECL_RESULT (current_function_decl)
> + && DECL_BY_REFERENCE (DECL_RESULT (current_function_decl)))
> + ;
> + else if (gimple_return_retval (ret) == name)
> lattice[index].merge (~(EAF_UNUSED | EAF_NOT_RETURNED));
> else if (memory_access_to (gimple_return_retval (ret), name))
> {
> @@ -1748,7 +1773,7 @@ analyze_ssa_name_flags (tree name, vec<modref_lattice>
> &lattice, int depth,
> may make LHS to escape. See PR 98499. */
> if (gimple_call_return_slot_opt_p (call)
> && TREE_ADDRESSABLE (TREE_TYPE (gimple_call_lhs (call))))
> - lattice[index].merge (EAF_NOREAD | EAF_DIRECT);
> + lattice[index].merge (gimple_call_retslot_flags (call));
> }
>
> /* We do not track accesses to the static chain (we could)
> @@ -1777,7 +1802,7 @@ analyze_ssa_name_flags (tree name, vec<modref_lattice>
> &lattice, int depth,
> lattice[index].merge (call_flags);
> else
> lattice[index].add_escape_point (call, i,
> - call_flags, true);
> + call_flags, true);
> }
> if (!ignore_retval)
> merge_call_lhs_flags (call, i, index, false,
> @@ -1912,6 +1937,29 @@ analyze_ssa_name_flags (tree name, vec<modref_lattice>
> &lattice, int depth,
> lattice[index].known = true;
> }
>
> +/* Record escape points of PARM_INDEX according to LATTICE. */
> +
> +static void
> +record_escape_points (modref_lattice &lattice, int parm_index, int flags)
> +{
> + if (lattice.escape_points.length ())
> + {
> + escape_point *ep;
> + unsigned int ip;
> + cgraph_node *node = cgraph_node::get (current_function_decl);
> +
> + FOR_EACH_VEC_ELT (lattice.escape_points, ip, ep)
> + if ((ep->min_flags & flags) != flags)
> + {
> + cgraph_edge *e = node->get_edge (ep->call);
> + struct escape_entry ee = {parm_index, ep->arg,
> + ep->min_flags, ep->direct};
> +
> + escape_summaries->get_create (e)->esc.safe_push (ee);
> + }
> + }
> +}
> +
> /* Determine EAF flags for function parameters. */
>
> static void
> @@ -1921,16 +1969,22 @@ analyze_parms (modref_summary *summary,
> modref_summary_lto *summary_lto,
> unsigned int parm_index = 0;
> unsigned int count = 0;
> int ecf_flags = flags_from_decl_or_type (current_function_decl);
> + tree retslot = NULL;
>
> /* For novops functions we have nothing to gain by EAF flags. */
> if (ecf_flags & ECF_NOVOPS)
> return;
>
> + /* If there is return slot, look up its SSA name. */
> + if (DECL_RESULT (current_function_decl)
> + && DECL_BY_REFERENCE (DECL_RESULT (current_function_decl)))
> + retslot = ssa_default_def (cfun, DECL_RESULT (current_function_decl));
> +
> for (tree parm = DECL_ARGUMENTS (current_function_decl); parm;
> parm = TREE_CHAIN (parm))
> count++;
>
> - if (!count)
> + if (!count && !retslot)
> return;
>
> auto_vec<modref_lattice> lattice;
> @@ -1984,24 +2038,24 @@ analyze_parms (modref_summary *summary,
> modref_summary_lto *summary_lto,
> summary_lto->arg_flags.safe_grow_cleared (count, true);
> summary_lto->arg_flags[parm_index] = flags;
> }
> - if (lattice[SSA_NAME_VERSION (name)].escape_points.length ())
> - {
> - escape_point *ep;
> - unsigned int ip;
> - cgraph_node *node = cgraph_node::get (current_function_decl);
> -
> - gcc_checking_assert (ipa);
> - FOR_EACH_VEC_ELT
> - (lattice[SSA_NAME_VERSION (name)].escape_points, ip, ep)
> - if ((ep->min_flags & flags) != flags)
> - {
> - cgraph_edge *e = node->get_edge (ep->call);
> - struct escape_entry ee = {parm_index, ep->arg,
> - ep->min_flags, ep->direct};
> + record_escape_points (lattice[SSA_NAME_VERSION (name)],
> + parm_index, flags);
> + }
> + }
> + if (retslot)
> + {
> + analyze_ssa_name_flags (retslot, lattice, 0, ipa);
> + int flags = lattice[SSA_NAME_VERSION (retslot)].flags;
>
> - escape_summaries->get_create (e)->esc.safe_push (ee);
> - }
> - }
> + flags = remove_useless_eaf_flags (flags, ecf_flags, false);
> + if (flags)
> + {
> + if (summary)
> + summary->retslot_flags = flags;
> + if (summary_lto)
> + summary_lto->retslot_flags = flags;
> + record_escape_points (lattice[SSA_NAME_VERSION (retslot)],
> + escape_point::retslot_arg, flags);
> }
> }
> if (ipa)
> @@ -2287,6 +2341,7 @@ modref_summaries::duplicate (cgraph_node *, cgraph_node
> *dst,
> dst_data->writes_errno = src_data->writes_errno;
> if (src_data->arg_flags.length ())
> dst_data->arg_flags = src_data->arg_flags.copy ();
> + dst_data->retslot_flags = src_data->retslot_flags;
> }
>
> /* Called when new clone is inserted to callgraph late. */
> @@ -2312,6 +2367,7 @@ modref_summaries_lto::duplicate (cgraph_node *,
> cgraph_node *,
> dst_data->writes_errno = src_data->writes_errno;
> if (src_data->arg_flags.length ())
> dst_data->arg_flags = src_data->arg_flags.copy ();
> + dst_data->retslot_flags = src_data->retslot_flags;
> }
>
> namespace
> @@ -2551,7 +2607,7 @@ modref_write_escape_summary (struct bitpack_d *bp,
> escape_summary *esum)
> escape_entry *ee;
> FOR_EACH_VEC_ELT (esum->esc, i, ee)
> {
> - bp_pack_var_len_unsigned (bp, ee->parm_index);
> + bp_pack_var_len_int (bp, ee->parm_index);
> bp_pack_var_len_unsigned (bp, ee->arg);
> bp_pack_var_len_unsigned (bp, ee->min_flags);
> bp_pack_value (bp, ee->direct, 1);
> @@ -2571,7 +2627,7 @@ modref_read_escape_summary (struct bitpack_d *bp,
> cgraph_edge *e)
> for (unsigned int i = 0; i < n; i++)
> {
> escape_entry ee;
> - ee.parm_index = bp_unpack_var_len_unsigned (bp);
> + ee.parm_index = bp_unpack_var_len_int (bp);
> ee.arg = bp_unpack_var_len_unsigned (bp);
> ee.min_flags = bp_unpack_var_len_unsigned (bp);
> ee.direct = bp_unpack_value (bp, 1);
> @@ -2628,6 +2684,7 @@ modref_write ()
> streamer_write_uhwi (ob, r->arg_flags.length ());
> for (unsigned int i = 0; i < r->arg_flags.length (); i++)
> streamer_write_uhwi (ob, r->arg_flags[i]);
> + streamer_write_uhwi (ob, r->retslot_flags);
>
> write_modref_records (r->loads, ob);
> write_modref_records (r->stores, ob);
> @@ -2724,6 +2781,11 @@ read_section (struct lto_file_decl_data *file_data,
> const char *data,
> if (modref_sum_lto)
> modref_sum_lto->arg_flags.quick_push (flags);
> }
> + eaf_flags_t flags = streamer_read_uhwi (&ib);
> + if (modref_sum)
> + modref_sum->retslot_flags = flags;
> + if (modref_sum_lto)
> + modref_sum_lto->retslot_flags = flags;
> read_modref_records (&ib, data_in,
> modref_sum ? &modref_sum->loads : NULL,
> modref_sum_lto ? &modref_sum_lto->loads : NULL);
> @@ -3098,7 +3160,7 @@ struct escape_map
> bool direct;
> };
>
> -/* Update escape map fo E. */
> +/* Update escape map for E. */
>
> static void
> update_escape_summary_1 (cgraph_edge *e,
> @@ -3117,7 +3179,10 @@ update_escape_summary_1 (cgraph_edge *e,
> {
> unsigned int j;
> struct escape_map *em;
> - if (ee->parm_index >= map.length ())
> + /* TODO: We do not have jump functions for return slots, so we
> + never propagate them to outer function. */
> + if (ee->parm_index >= (int)map.length ()
> + || ee->parm_index < 0)
> continue;
> FOR_EACH_VEC_ELT (map[ee->parm_index], j, em)
> {
> @@ -3125,7 +3190,7 @@ update_escape_summary_1 (cgraph_edge *e,
> if (ee->direct && !em->direct)
> min_flags = deref_flags (min_flags, ignore_stores);
> struct escape_entry entry = {em->parm_index, ee->arg,
> - ee->min_flags,
> + ee->min_flags,
> ee->direct & em->direct};
> sum->esc.safe_push (entry);
> }
> @@ -3245,7 +3310,11 @@ ipa_merge_modref_summary_after_inlining (cgraph_edge
> *edge)
> FOR_EACH_VEC_ELT (sum->esc, i, ee)
> {
> bool needed = false;
> - if (to_info && to_info->arg_flags.length () > ee->parm_index)
> + /* TODO: We do not have jump functions for return slots, so we
> + never propagate them to outer function. */
> + if (ee->parm_index < 0)
> + continue;
> + if (to_info && (int)to_info->arg_flags.length () > ee->parm_index)
> {
> int flags = callee_info
> && callee_info->arg_flags.length () > ee->arg
> @@ -3259,7 +3328,7 @@ ipa_merge_modref_summary_after_inlining (cgraph_edge
> *edge)
> if (to_info->arg_flags[ee->parm_index])
> needed = true;
> }
> - if (to_info_lto && to_info_lto->arg_flags.length () > ee->parm_index)
> + if (to_info_lto && (int)to_info_lto->arg_flags.length () >
> ee->parm_index)
> {
> int flags = callee_info_lto
> && callee_info_lto->arg_flags.length () > ee->arg
> @@ -3798,29 +3867,31 @@ modref_merge_call_site_flags (escape_summary *sum,
> if (flags_lto & EAF_NOESCAPE)
> flags_lto |= EAF_NODIRECTESCAPE;
> if (!(flags & EAF_UNUSED)
> - && cur_summary && ee->parm_index < cur_summary->arg_flags.length ())
> + && cur_summary && ee->parm_index < (int)cur_summary->arg_flags.length
> ())
> {
> - int f = cur_summary->arg_flags[ee->parm_index];
> + eaf_flags_t &f = ee->parm_index == escape_point::retslot_arg
> + ? cur_summary->retslot_flags
> + : cur_summary->arg_flags[ee->parm_index];
> if ((f & flags) != f)
> {
> f = remove_useless_eaf_flags
> (f & flags, ecf_flags,
> VOID_TYPE_P (TREE_TYPE (TREE_TYPE (caller))));
> - cur_summary->arg_flags[ee->parm_index] = f;
> changed = true;
> }
> }
> if (!(flags_lto & EAF_UNUSED)
> && cur_summary_lto
> - && ee->parm_index < cur_summary_lto->arg_flags.length ())
> + && ee->parm_index < (int)cur_summary_lto->arg_flags.length ())
> {
> - int f = cur_summary_lto->arg_flags[ee->parm_index];
> + eaf_flags_t &f = ee->parm_index == escape_point::retslot_arg
> + ? cur_summary_lto->retslot_flags
> + : cur_summary_lto->arg_flags[ee->parm_index];
> if ((f & flags_lto) != f)
> {
> f = remove_useless_eaf_flags
> (f & flags_lto, ecf_flags,
> VOID_TYPE_P (TREE_TYPE (TREE_TYPE (caller))));
> - cur_summary_lto->arg_flags[ee->parm_index] = f;
> changed = true;
> }
> }
> diff --git a/gcc/ipa-modref.h b/gcc/ipa-modref.h
> index 5afa3aa439f..a4db27471eb 100644
> --- a/gcc/ipa-modref.h
> +++ b/gcc/ipa-modref.h
> @@ -31,6 +31,7 @@ struct GTY(()) modref_summary
> modref_records *loads;
> modref_records *stores;
> auto_vec<eaf_flags_t> GTY((skip)) arg_flags;
> + eaf_flags_t retslot_flags;
> bool writes_errno;
>
> modref_summary ();
> diff --git a/gcc/tree-ssa-structalias.c b/gcc/tree-ssa-structalias.c
> index 35971a54e02..99072df0768 100644
> --- a/gcc/tree-ssa-structalias.c
> +++ b/gcc/tree-ssa-structalias.c
> @@ -4254,17 +4254,28 @@ handle_rhs_call (gcall *stmt, vec<ce_s> *results,
> && gimple_call_lhs (stmt) != NULL_TREE
> && TREE_ADDRESSABLE (TREE_TYPE (gimple_call_lhs (stmt))))
> {
> - auto_vec<ce_s> tmpc;
> - struct constraint_expr *c;
> - unsigned i;
> + int flags = gimple_call_retslot_flags (stmt);
> + if ((flags & (EAF_NOESCAPE | EAF_NOT_RETURNED))
> + != (EAF_NOESCAPE | EAF_NOT_RETURNED))
> + {
> + auto_vec<ce_s> tmpc;
>
> - get_constraint_for_address_of (gimple_call_lhs (stmt), &tmpc);
> + get_constraint_for_address_of (gimple_call_lhs (stmt), &tmpc);
>
> - make_constraints_to (callescape->id, tmpc);
> - if (writes_global_memory)
> - make_constraints_to (escaped_id, tmpc);
> - FOR_EACH_VEC_ELT (tmpc, i, c)
> - results->safe_push (*c);
> + if (!(flags & (EAF_NOESCAPE | EAF_NODIRECTESCAPE)))
> + {
> + make_constraints_to (callescape->id, tmpc);
> + if (writes_global_memory)
> + make_constraints_to (escaped_id, tmpc);
> + }
> + if (!(flags & EAF_NOT_RETURNED))
> + {
> + struct constraint_expr *c;
> + unsigned i;
> + FOR_EACH_VEC_ELT (tmpc, i, c)
> + results->safe_push (*c);
> + }
> + }
> }
> }
>
>
--
Richard Biener <rguent...@suse.de>
SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
Germany; GF: Felix Imendörffer; HRB 36809 (AG Nuernberg)
