From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp-out1.suse.de (smtp-out1.suse.de [195.135.220.28]) by sourceware.org (Postfix) with ESMTPS id 76FE23858C39 for ; Fri, 29 Oct 2021 09:38:53 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 76FE23858C39 Received: from relay2.suse.de (relay2.suse.de [149.44.160.134]) by smtp-out1.suse.de (Postfix) with ESMTP id 5145121973; Fri, 29 Oct 2021 09:38:52 +0000 (UTC) Received: from murzim.suse.de (murzim.suse.de [10.160.4.192]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by relay2.suse.de (Postfix) with ESMTPS id 49C04A3B85; Fri, 29 Oct 2021 09:38:52 +0000 (UTC) Date: Fri, 29 Oct 2021 11:38:52 +0200 (CEST) From: Richard Biener To: Jan Hubicka cc: gcc-patches@gcc.gnu.org, mjambor@suse.cz Subject: Re: Handle retslot_flags in ipa-modref and PTA In-Reply-To: <20211029093443.GD91231@kam.mff.cuni.cz> Message-ID: <751q0s6-s7p4-2952-q629-q38898qqq1s@fhfr.qr> References: <20211029093443.GD91231@kam.mff.cuni.cz> MIME-Version: 1.0 X-Spam-Status: No, score=-11.1 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org Content-Type: text/plain; charset=ISO-8859-15 Content-Transfer-Encoding: 8BIT X-Content-Filtered-By: Mailman/MimeDel 2.1.29 X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 29 Oct 2021 09:38:56 -0000 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 > /* 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 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 &lattice, int depth, > Returning name counts as an use by tree-ssa-structalias.c */ > if (greturn *ret = dyn_cast (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 &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 &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 &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 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 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 *results, > && gimple_call_lhs (stmt) != NULL_TREE > && TREE_ADDRESSABLE (TREE_TYPE (gimple_call_lhs (stmt)))) > { > - auto_vec 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 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 SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg, Germany; GF: Felix Imendörffer; HRB 36809 (AG Nuernberg)