Hello, On Wed, Jun 17 2026, Josef Melcr wrote: > Last version: > https://patchwork.sourceware.org/project/gcc/patch/[email protected]/ > > Hi, > this patch got a review by Martin off the mailing list, so I will try to > summarize it here. I've made the following changes: > - Renamed the wrapper functions: Renamed gimple_call_arg_summary to > callback_call_arg_summary, likewise for gimple_call_num_args_summary. > - Martin pointed out that I was using `struct function` in WPA to > identify the .ASSUME bodies, which of course wouldn't work. I > ultimately decided to solve this by adding a one bit flag to cgraph_node > containing the information. It is duplicate without LTO, but we would > have to resort to some pattern matching without it, which wouldn't be > too reliable or readable IMHO. > - The dont_disqualify flag is now included in ipa-sra dumps. > - Added a LTO testcase.
Thanks a lot for keeping working on this, I still have some comments/concerns though, see below. > > Best regards, > Josef > > PR c++/122779 > PR ipa/109112 > > gcc/ChangeLog: > > * attr-callback.cc (callback_redirect_edge): New function, > redirects a callback edge. > * attr-callback.h (callback_redirect_edge): Add decl. > * callback-info.cc (callback_info::initialize_internal_summary): > New method, initializes the summary for internal calls. > (callback_call_arg_summary): New function, wrapper around > gimple_call_arg. Remaps arg indices for callbacks if necessary. > (callback_call_num_args_summary): New function, wrapper around > gimple_call_num_args. Returns the correct number of args for > callback calls. > * callback-info.h (callback_call_arg_summary): Add decl. > (callback_call_num_args_summary): Likewise. > * cgraph.cc (cgraph_node::get_edge): Don't return the > dispatching edge for assume callback edges. > (cgraph_edge::set_call_stmt): Don't cascade for assume callback > edges. > (cgraph_edge::make_callback_internal): New method, creates a > callback edge attached to an internal call. > (cgraph_edge::redirect_call_stmt_to_callee): Implement assume > callback edge redirection. > (cgraph_node::dump): Dump assume_function flag. > (cgraph_node::verify_node): Adjust sanity checks. > * cgraph.h (struct cgraph_node): Add assume_function flag to > cgraph_node. > * cgraphclones.cc (cgraph_node::create_clone): Copy > assume_function flag. > * gimple-low.cc (create_assumption_fn): Don't add attrs blocking > ipa optimizations to the new function. > (lower_assumption): Unset stdarg, set assume_function. > * ipa-fnsummary.cc (analyze_function_body): Implement summary > modifications for assume callback calls. > * ipa-param-manipulation.cc (ipa_param_adjustments::modify_call): > Implement call stmt modifications for IFN_ASSUME. > * ipa-sra.cc (struct isra_param_desc): Add dont_disqualify flag. > (struct gensum_param_desc): Likewise. > (ipa_sra_function_summaries::duplicate): Copy dont_disqualify. > (dump_gensum_param_descriptor): Dump dont_disqualify. > (ptr_parm_has_nonarg_uses): Use callback_call_arg_summary. > (create_parameter_descriptors): Initialize dont_disqualify. > (disqualify_split_candidate): Early exit if dont_disqualify. > (scan_function): Add logic for assume callback edge creation. > (isra_analyze_call): Remap args for callback edges. > (process_scan_results): Likewise. > (ipa_sra_generate_summary): Check-create callback_info_sum. > (isra_write_node_summary): Stream out dont_disqualify. > (isra_read_node_info): Stream in dont_disqualify. > (can_be_local_p): New function, returns TRUE iff > node->can_be_local_p or if the function is an .ASSUME body. > (ipa_sra_ipa_function_checks): Use can_be_local_p. > (size_would_violate_limit_p): Use dont_disqualify. > (retval_used_p): Return TRUE for .ASSUME bodies. > * lto-cgraph.cc (lto_output_node): Stream out assume_function flag. > (input_overwrite_node): Stream in assume_function flag. > * tree-inline.cc (initialize_cfun): Copy assume_function flag (struct > function, not cgraph_node). > > gcc/testsuite/ChangeLog: > > * g++.dg/ipa/ipa-sra-assume-2.C: New test. > * g++.dg/ipa/ipa-sra-assume-3.C: New test. > * g++.dg/ipa/ipa-sra-assume.C: New test. > * g++.dg/lto/ipa-sra-assume_0.C: New test. > * g++.dg/lto/ipa-sra-assume_1.C: New test. > > Signed-off-by: Josef Melcr <[email protected]> > --- > gcc/attr-callback.cc | 13 +++ > gcc/attr-callback.h | 4 + > gcc/callback-info.cc | 43 ++++++++++ > gcc/callback-info.h | 14 +++- > gcc/cgraph.cc | 79 ++++++++++++++---- > gcc/cgraph.h | 15 +++- > gcc/cgraphclones.cc | 1 + > gcc/gimple-low.cc | 17 ++-- > gcc/ipa-fnsummary.cc | 22 ++++- > gcc/ipa-param-manipulation.cc | 27 ++++-- > gcc/ipa-sra.cc | 92 +++++++++++++++++---- > gcc/lto-cgraph.cc | 2 + > gcc/testsuite/g++.dg/ipa/ipa-sra-assume-2.C | 33 ++++++++ > gcc/testsuite/g++.dg/ipa/ipa-sra-assume-3.C | 30 +++++++ > gcc/testsuite/g++.dg/ipa/ipa-sra-assume.C | 37 +++++++++ > gcc/testsuite/g++.dg/lto/ipa-sra-assume_0.C | 28 +++++++ > gcc/testsuite/g++.dg/lto/ipa-sra-assume_1.C | 11 +++ > gcc/tree-inline.cc | 1 + > 18 files changed, 416 insertions(+), 53 deletions(-) > create mode 100644 gcc/testsuite/g++.dg/ipa/ipa-sra-assume-2.C > create mode 100644 gcc/testsuite/g++.dg/ipa/ipa-sra-assume-3.C > create mode 100644 gcc/testsuite/g++.dg/ipa/ipa-sra-assume.C > create mode 100644 gcc/testsuite/g++.dg/lto/ipa-sra-assume_0.C > create mode 100644 gcc/testsuite/g++.dg/lto/ipa-sra-assume_1.C > > diff --git a/gcc/attr-callback.cc b/gcc/attr-callback.cc > index ec89974532c..e1367da37f1 100644 > --- a/gcc/attr-callback.cc > +++ b/gcc/attr-callback.cc > @@ -24,6 +24,7 @@ > #include "backend.h" > #include "tree.h" > #include "gimple.h" > +#include "fold-const.h" > #include "alloc-pool.h" > #include "cgraph.h" > #include "diagnostic.h" > @@ -362,3 +363,15 @@ callback_num_args (tree attr) > ; > return res; > } > + > +/* Redirect call statement of E to the callee of E. Used in > + cgraph_edge::redirect_call_stmt_to_callee. */ > +void > +callback_redirect_edge (cgraph_edge *e) > +{ > + callback_info *ci = callback_info_sum->get (e); > + tree previous_arg = gimple_call_arg (e->call_stmt, ci->fn_idx); > + location_t loc = EXPR_LOCATION (previous_arg); > + tree new_addr = build_fold_addr_expr_loc (loc, e->callee->decl); > + gimple_call_set_arg (e->call_stmt, ci->fn_idx, new_addr); > +} Does it really make sense to put this into a special function if the use is constrained to one particular place?a > diff --git a/gcc/attr-callback.h b/gcc/attr-callback.h > index 8f649006b8c..088509d2221 100644 > --- a/gcc/attr-callback.h > +++ b/gcc/attr-callback.h > @@ -75,4 +75,8 @@ bool callback_edge_useful_p (cgraph_edge *e); > takes. */ > size_t callback_num_args (tree attr); > > +/* Redirect call statement of E to the callee of E. Used in > + cgraph_edge::redirect_call_stmt_to_callee. */ > +void callback_redirect_edge (cgraph_edge *e); > + > #endif /* ATTR_CALLBACK_H */ > diff --git a/gcc/callback-info.cc b/gcc/callback-info.cc > index 5309d763936..764ad75a66f 100644 > --- a/gcc/callback-info.cc > +++ b/gcc/callback-info.cc > @@ -78,6 +78,29 @@ callback_info::get_id () const > return fn_idx; > } > > +/* Initialize the summary for a callback call CBE of internal function IFN. > */ > +callback_info * > +callback_info::initialize_internal_summary (cgraph_edge *cbe, internal_fn > ifn) > +{ > + callback_info_sum_t::check_create_info_sum (); > + callback_info *ci = callback_info_sum->get_create (cbe); > + switch (ifn) > + { > + case IFN_ASSUME: > + { > + ci->fn_idx = 0; > + unsigned n = gimple_call_num_args (cbe->call_stmt); > + for (unsigned i = 0; i < n - 1; i++) > + ci->arg_mapping.safe_push (i + 1); > + break; > + } > + default: > + gcc_unreachable (); > + } > + ci->redirected = false; > + return ci; > +} > + > /* Populates the callback_info_sum if it's NULL. */ > void > callback_info_sum_t::check_create_info_sum (void) > @@ -106,3 +129,23 @@ callback_info_sum_t::duplicate (cgraph_edge *, > cgraph_edge *, > dst_s->arg_mapping = src_s->arg_mapping.copy (); > dst_s->redirected = src_s->redirected; > } > + > +/* Return the argument at IDX if CI is null, otherwise use the mapping in CI > and > + return the remapped argument. */ > +tree > +callback_call_arg_summary (gimple *stmt, unsigned idx, callback_info *ci) > +{ > + if (ci) > + idx = ci->arg_mapping[idx]; > + return gimple_call_arg (stmt, idx); > +} > + > +/* Return the number of arguments of STMT if CI is null, > + ci->arg_mapping.length() otherwise. */ > +unsigned > +callback_call_num_args_summary (const gcall *stmt, callback_info *ci) > +{ > + if (ci) > + return ci->arg_mapping.length (); > + return gimple_call_num_args (stmt); > +} > diff --git a/gcc/callback-info.h b/gcc/callback-info.h > index 2443fa426e6..0d41eb8dbe1 100644 > --- a/gcc/callback-info.h > +++ b/gcc/callback-info.h > @@ -40,7 +40,7 @@ public: > auto_vec<int> arg_mapping; > > /* TRUE iff the associated callback edge was redirected. */ > - bool redirected; > + bool redirected : 1; Is this intentional? If so, it is not referenced in the ChangeLog. > > /* Stream in callback_info. */ > void stream_in (lto_input_block *ib); > @@ -50,6 +50,10 @@ public: > > /* Returns the id of the associated callback attribute. */ > unsigned get_id () const; > + > + /* Initialize the summary for a callback call CBE of internal function > IFN. */ > + static callback_info *initialize_internal_summary (cgraph_edge *cbe, > + internal_fn ifn); > }; > > class callback_info_sum_t : public call_summary<callback_info *> > @@ -72,4 +76,12 @@ public: > > extern callback_info_sum_t *callback_info_sum; > > +/* Return the argument at IDX if CI is null, otherwise use the mapping in CI > and > + return the remapped argument. */ > +tree callback_call_arg_summary (gimple *stmt, unsigned idx, callback_info > *ci); > + > +/* Return the number of arguments of STMT if CI is null, > + ci->arg_mapping.length() otherwise. */ > +unsigned callback_call_num_args_summary (const gcall *stmt, callback_info > *ci); > + > #endif /* CALLBACK_INFO_H */ > diff --git a/gcc/cgraph.cc b/gcc/cgraph.cc > index b43a249938d..66b0af0f5f1 100644 > --- a/gcc/cgraph.cc > +++ b/gcc/cgraph.cc > @@ -937,7 +937,7 @@ cgraph_node::get_edge (gimple *call_stmt) > comes to callback edges, a call statement might have multiple callback > edges attached to it. These can be easily obtained from the carrying > edge > instead. */ > - if (e && e->callback) > + if (e && e->callback && !e->callee->assume_function) The various tests for e->callback && !e->callee->assume_function need a comment, or perhaps it would be bettor to make it an appropriately named predicate, something like cgraph_edge::callback_with_carrying_edge_p(). > e = e->get_callback_carrying_edge (); > > if (n > 100) > @@ -1015,7 +1015,8 @@ cgraph_edge::set_call_stmt (cgraph_edge *e, gcall > *new_stmt, > > /* When updating a callback or a callback-carrying edge, update every edge > involved. */ > - if (update_derived_edges && (e->callback || e->has_callback)) > + if (update_derived_edges && (e->callback || e->has_callback) > + && !e->callee->assume_function) > { > cgraph_edge *current, *next, *carrying; > carrying = e->has_callback ? e : e->get_callback_carrying_edge (); > @@ -1383,6 +1384,37 @@ cgraph_edge::make_callback (cgraph_node *n2) > return e2; > } > > + > +/* Create a callback edge, representing an indirect call to n2 > + passed to an internal function by argument. As internal calls are not > + represented in the callgraph, the created edge won't have any associated > + dispatching edge. The created callback edge will be attached to the > + internal call statement STMT. Returns created callback edge. */ > + > +cgraph_edge * > +cgraph_edge::make_callback_internal (cgraph_node *caller, cgraph_node > *target, > + gcall *call_stmt) > +{ > + cgraph_edge *ret; > + > + ret > + = caller->create_edge (target, call_stmt, profile_count::uninitialized > ()); > + if (dump_file) > + { > + fprintf (dump_file, > + "Created callback edge %s -> %s belonging to internal call ", > + ret->caller->dump_name (), ret->callee->dump_name ()); > + print_gimple_stmt (dump_file, call_stmt, 0); > + } > + ret->inline_failed = CIF_CALLBACK_EDGE; > + ret->callback = true; > + if (TREE_NOTHROW (target->decl)) > + ret->can_throw_external = false; > + caller->create_reference (target, IPA_REF_ADDR, call_stmt); > + target->mark_address_taken (); > + return ret; > +} > + > /* Returns the callback_carrying edge of a callback edge on which > it is called on or NULL when no such edge can be found. > > @@ -1837,21 +1869,18 @@ cgraph_edge::redirect_call_stmt_to_callee > (cgraph_edge *e, > /* When redirecting a callback edge, all we need to do is replace > the original address with the address of the function we are > redirecting to. */ > - if (e->callback) > + if (e->callback && !e->callee->assume_function) > { > cgraph_edge *carrying = e->get_callback_carrying_edge (); > - if (!callback_is_special_cased (carrying->callee->decl, e->call_stmt) > + if (carrying > + && !callback_is_special_cased (carrying->callee->decl, e->call_stmt) > && !lookup_attribute (CALLBACK_ATTR_IDENT, > DECL_ATTRIBUTES (carrying->callee->decl))) > /* Callback attribute is removed if the dispatching function changes > signature, as the indices wouldn't be correct anymore. These edges > will get cleaned up later, ignore their redirection for now. */ > return e->call_stmt; > - callback_info *ci = callback_info_sum->get (e); > - tree previous_arg = gimple_call_arg (e->call_stmt, ci->fn_idx); > - location_t loc = EXPR_LOCATION (previous_arg); > - tree new_addr = build_fold_addr_expr_loc (loc, e->callee->decl); > - gimple_call_set_arg (e->call_stmt, ci->fn_idx, new_addr); > + callback_redirect_edge (e); > return e->call_stmt; > } I'd prefer to have an early exist here for all types of callback edges, rather than continuing, but only somewhat, with those that don't have a carrying edge and then returning early for those too. I think the function would otherwise become unreadable very quickly. So please add whatever is necessary for assume call here together with another return, even if some code needs to be duplicated (but does it?). > > @@ -1904,6 +1933,9 @@ cgraph_edge::redirect_call_stmt_to_callee (cgraph_edge > *e, > && old_fntype == TREE_TYPE (origin->former_clone_of)) > || old_fntype == TREE_TYPE (origin->decl)) > gimple_call_set_fntype (new_stmt, TREE_TYPE (e->callee->decl)); > + /* Don't set fntype on internal calls. */ > + else if (gimple_call_internal_p (new_stmt)) > + ; The empty statement instead of negating the condition seems totally unnecessary here? > else > { > tree new_fntype = padjs->build_new_function_type (old_fntype, true); > @@ -1920,10 +1952,20 @@ cgraph_edge::redirect_call_stmt_to_callee > (cgraph_edge *e, > BUILT_IN_UNREACHABLE_TRAP)) > ipa_verify_edge_has_no_modifications (e); > new_stmt = e->call_stmt; > - gimple_call_set_fndecl (new_stmt, e->callee->decl); > + /* Redirects callback edges attached to internal calls. */ > + if (e->callback) > + callback_redirect_edge (e); > + else > + gimple_call_set_fndecl (new_stmt, e->callee->decl); > update_stmt_fn (DECL_STRUCT_FUNCTION (e->caller->decl), new_stmt); > } > > + if (e->callback && e->callee->assume_function) > + { > + e->caller->set_call_stmt_including_clones (e->call_stmt, new_stmt, > false); > + return new_stmt; > + } > + > /* If changing the call to __cxa_pure_virtual or similar noreturn function, > adjust gimple_call_fntype too. */ > if (gimple_call_noreturn_p (new_stmt) > @@ -2580,6 +2622,8 @@ cgraph_node::dump (FILE *f) > fprintf (f, " calls_comdat_local"); > if (icf_merged) > fprintf (f, " icf_merged"); > + if (assume_function) > + fprintf (f, " assume_function"); > if (merged_comdat) > fprintf (f, " merged_comdat"); > if (merged_extern_inline) > @@ -4396,13 +4440,14 @@ cgraph_node::verify_node (void) > > if (e->callback) > { > - if (!e->get_callback_carrying_edge ()) > - { > - error ("callback edge %s->%s has no callback-carrying", > - identifier_to_locale (e->caller->name ()), > - identifier_to_locale (e->callee->name ())); > - error_found = true; > - } > + if (!e->get_callback_carrying_edge () > + && (!e->call_stmt || !gimple_call_internal_p (e->call_stmt))) > + { > + error ("callback edge %s->%s has no callback-carrying edge", > + identifier_to_locale (e->caller->name ()), > + identifier_to_locale (e->callee->name ())); > + error_found = true; > + } > } > > if (e->has_callback > diff --git a/gcc/cgraph.h b/gcc/cgraph.h > index ec7112452da..7afe4e0d4c3 100644 > --- a/gcc/cgraph.h > +++ b/gcc/cgraph.h > @@ -939,8 +939,8 @@ struct GTY((tag ("SYMTAB_FUNCTION"))) cgraph_node : > public symtab_node > only_called_at_startup (false), only_called_at_exit (false), > tm_clone (false), dispatcher_function (false), > dispatcher_resolver_function (false), is_target_clone (false), > - calls_comdat_local (false), > - icf_merged (false), nonfreeing_fn (false), merged_comdat (false), > + calls_comdat_local (false), icf_merged (false), assume_function(false), Space before opening bracket :-) > + nonfreeing_fn (false), merged_comdat (false), > merged_extern_inline (false), parallelized_function (false), > split_part (false), indirect_call_target (false), local (false), > versionable (false), can_change_signature (false), > @@ -1536,6 +1536,8 @@ struct GTY((tag ("SYMTAB_FUNCTION"))) cgraph_node : > public symtab_node > unsigned calls_comdat_local : 1; > /* True if node has been created by merge operation in IPA-ICF. */ > unsigned icf_merged: 1; > + /* True if node has been created by lowering an [[assume(...)]] statement. > */ > + unsigned assume_function : 1; > /* True if call to node can't result in a call to free, munmap or > other operation that could make previously non-trapping memory > accesses trapping. */ > @@ -1928,6 +1930,15 @@ public: > callback edge. */ > cgraph_edge *make_callback (cgraph_node *n2); > > + /* Create a callback edge, representing an indirect call to n2 > + passed to an internal function by argument. As internal calls are not > + represented in the callgraph, the created edge won't have any associated > + dispatching edge. edge. The created callback edge will be attached to > the Edge repeated here. > + internal call statement STMT. Returns created callback edge. */ > + static cgraph_edge *make_callback_internal (cgraph_node *caller, > + cgraph_node *target, > + gcall *call_stmt); > + > /* Returns the callback-carrying edge of a callback edge or NULL, if such > edge > cannot be found. An edge is considered callback-carrying, if it has > it's > has_callback flag set and shares it's call statement with the edge > diff --git a/gcc/cgraphclones.cc b/gcc/cgraphclones.cc > index b5f2ce5ff2f..c5aead0c3b8 100644 > --- a/gcc/cgraphclones.cc > +++ b/gcc/cgraphclones.cc > @@ -452,6 +452,7 @@ cgraph_node::create_clone (tree new_decl, profile_count > prof_count, > new_node->tp_first_run = tp_first_run; > new_node->tm_clone = tm_clone; > new_node->icf_merged = icf_merged; > + new_node->assume_function = assume_function; > new_node->thunk = thunk; > new_node->unit_id = unit_id; > new_node->merged_comdat = merged_comdat; > diff --git a/gcc/gimple-low.cc b/gcc/gimple-low.cc > index 5915d7234e0..6f0f5bedd02 100644 > --- a/gcc/gimple-low.cc > +++ b/gcc/gimple-low.cc > @@ -302,18 +302,7 @@ create_assumption_fn (location_t loc) > DECL_EXTERNAL (decl) = 0; > DECL_CONTEXT (decl) = NULL_TREE; > DECL_INITIAL (decl) = make_node (BLOCK); > - tree attributes = DECL_ATTRIBUTES (current_function_decl); > - if (lookup_attribute ("noipa", attributes) == NULL) > - { > - attributes = tree_cons (get_identifier ("noipa"), NULL, attributes); > - if (lookup_attribute ("noinline", attributes) == NULL) > - attributes = tree_cons (get_identifier ("noinline"), NULL, attributes); > - if (lookup_attribute ("noclone", attributes) == NULL) > - attributes = tree_cons (get_identifier ("noclone"), NULL, attributes); > - if (lookup_attribute ("no_icf", attributes) == NULL) > - attributes = tree_cons (get_identifier ("no_icf"), NULL, attributes); > - } > - DECL_ATTRIBUTES (decl) = attributes; > + DECL_ATTRIBUTES (decl) = DECL_ATTRIBUTES (current_function_decl); > BLOCK_SUPERCONTEXT (DECL_INITIAL (decl)) = decl; > DECL_FUNCTION_SPECIFIC_OPTIMIZATION (decl) > = DECL_FUNCTION_SPECIFIC_OPTIMIZATION (current_function_decl); > @@ -673,8 +662,12 @@ lower_assumption (gimple_stmt_iterator *gsi, struct > lower_data *data) > } > DECL_ARGUMENTS (lad.id.dst_fn) = parms; > TREE_TYPE (lad.id.dst_fn) = build_function_type (boolean_type_node, parmt); > + /* The body function no longer has var. args, unset stdarg. */ > + DECL_STRUCT_FUNCTION (lad.id.dst_fn)->stdarg = 0; This is intriguing and I don't really understand it. Can these two lines be submitted and committed separately? (I'm not sure I can approve this.) This also suggests that the last element of TREE_TYPE (lad.id.dst_fn) built here should be void_type_node, is it? > > cgraph_node::add_new_function (lad.id.dst_fn, false); > + cgraph_node *dst_node = cgraph_node::get (lad.id.dst_fn); > + dst_node->assume_function = 1; > > for (unsigned i = 0; i < sz; ++i) > { > diff --git a/gcc/ipa-fnsummary.cc b/gcc/ipa-fnsummary.cc > index a6ccdb1852a..1c253aadd21 100644 > --- a/gcc/ipa-fnsummary.cc > +++ b/gcc/ipa-fnsummary.cc > @@ -3045,9 +3045,20 @@ analyze_function_body (struct cgraph_node *node, bool > early) > } > > if (is_gimple_call (stmt) > - && !gimple_call_internal_p (stmt)) > + && (!gimple_call_internal_p (stmt) > + || gimple_call_internal_fn (stmt) == IFN_ASSUME)) > { > struct cgraph_edge *edge = node->get_edge (stmt); > + if (!edge) > + { > + /* The edge for .ASSUME outlined bodies is created when > + generating ipa-sra summaries. Ignore the call statement > + for now. */ > + gcc_checking_assert (gimple_call_internal_p (stmt) > + && gimple_call_internal_fn (stmt) > + == IFN_ASSUME); I still wonder whether we want to create the edge here (too). But for now, I think this is OK. > + continue; > + } > ipa_call_summary *es = ipa_call_summaries->get_create (edge); > > /* Special case: results of BUILT_IN_CONSTANT_P will be always > @@ -3134,6 +3145,15 @@ analyze_function_body (struct cgraph_node *node, bool > early) > es2->call_stmt_time = 0; > } > } > + /* Internal calls are not in the callgraph, so edges pointing to > + .ASSUME bodies are not covered by the above branch, as the > + dispatching edge doesn't exist. */ > + else if (edge->callback) > + { > + gcc_checking_assert (edge->callee->assume_function); > + es->call_stmt_size = 0; > + es->call_stmt_time = 0; > + } > } > > /* TODO: When conditional jump or switch is known to be constant, but > diff --git a/gcc/ipa-param-manipulation.cc b/gcc/ipa-param-manipulation.cc > index 6f85d10a29e..9a2e83bf851 100644 > --- a/gcc/ipa-param-manipulation.cc > +++ b/gcc/ipa-param-manipulation.cc > @@ -50,7 +50,9 @@ along with GCC; see the file COPYING3. If not see > #include "sreal.h" > #include "ipa-cp.h" > #include "ipa-prop.h" > +#include "data-streamer.h" > #include "attr-callback.h" > +#include "callback-info.h" > > /* Actual prefixes of different newly synthesized parameters. Keep in sync > with IPA_PARAM_PREFIX_* defines. */ > @@ -719,7 +721,8 @@ ipa_param_adjustments::modify_call (cgraph_edge *cs, > > unsigned len = vec_safe_length (m_adj_params); > auto_vec<tree, 16> vargs (len); > - unsigned old_nargs = gimple_call_num_args (stmt); > + callback_info *ci = callback_info_sum->get (cs); > + unsigned old_nargs = callback_call_num_args_summary (stmt, ci); > unsigned orig_nargs = mod_info ? mod_info->index_map.length () : old_nargs; > auto_vec<bool, 16> kept (old_nargs); > kept.quick_grow_cleared (old_nargs); > @@ -728,6 +731,11 @@ ipa_param_adjustments::modify_call (cgraph_edge *cs, > if (update_references) > current_node->remove_stmt_references (stmt); > > + if (cs->callback && cs->callee->assume_function) > + { > + vargs.safe_push (build_fold_addr_expr (callee_decl)); > + } No need to have braces here. > + > gimple_stmt_iterator gsi = gsi_for_stmt (stmt); > gimple_stmt_iterator prev_gsi = gsi; > gsi_prev (&prev_gsi); > @@ -747,7 +755,7 @@ ipa_param_adjustments::modify_call (cgraph_edge *cs, > gcc_assert (index >= 0); > } > > - tree arg = gimple_call_arg (stmt, index); > + tree arg = callback_call_arg_summary (stmt, index, ci); > > vargs.quick_push (arg); > kept[index] = true; > @@ -773,7 +781,7 @@ ipa_param_adjustments::modify_call (cgraph_edge *cs, > { > int repl_idx = mod_info->pass_through_map[j].new_index; > gcc_assert (repl_idx >= 0); > - repl = gimple_call_arg (stmt, repl_idx); > + repl = callback_call_arg_summary (stmt, repl_idx, ci); > break; > } > if (repl) > @@ -797,7 +805,7 @@ ipa_param_adjustments::modify_call (cgraph_edge *cs, > index = mod_info->index_map[apm->base_index]; > gcc_assert (index >= 0); > } > - tree base = gimple_call_arg (stmt, index); > + tree base = callback_call_arg_summary (stmt, index, ci); > > /* We create a new parameter out of the value of the old one, we can > do the following kind of transformations: > @@ -920,7 +928,7 @@ ipa_param_adjustments::modify_call (cgraph_edge *cs, > gcc_assert (always_copy_start >= 0); > } > for (unsigned i = always_copy_start; i < old_nargs; i++) > - vargs.safe_push (gimple_call_arg (stmt, i)); > + vargs.safe_push (callback_call_arg_summary (stmt, i, ci)); > } > > /* For optimized away parameters, add on the caller side > @@ -1005,7 +1013,14 @@ ipa_param_adjustments::modify_call (cgraph_edge *cs, > print_gimple_stmt (dump_file, gsi_stmt (gsi), 0); > } > > - gcall *new_stmt = gimple_build_call_vec (callee_decl, vargs); > + gcall *new_stmt; > + if (cs->callback && cs->callee->assume_function) > + { > + /* Has to be special-cased, as we have to create an internal call. */ > + new_stmt = gimple_build_call_internal_vec (IFN_ASSUME, vargs); I think the comment is not really needed. Slightly more importantly, the braces should not be there (either way). > + } > + else > + new_stmt = gimple_build_call_vec (callee_decl, vargs); > > hash_set <tree> *ssas_to_remove = NULL; > if (tree lhs = gimple_call_lhs (stmt)) > diff --git a/gcc/ipa-sra.cc b/gcc/ipa-sra.cc > index 422bf367b97..282f4658279 100644 > --- a/gcc/ipa-sra.cc > +++ b/gcc/ipa-sra.cc > @@ -87,6 +87,8 @@ along with GCC; see the file COPYING3. If not see > #include "sreal.h" > #include "ipa-cp.h" > #include "ipa-prop.h" > +#include "attr-callback.h" > +#include "callback-info.h" > > static void ipa_sra_summarize_function (cgraph_node *); > > @@ -185,6 +187,10 @@ struct GTY(()) isra_param_desc > unsigned locally_unused : 1; > /* An aggregate that is a candidate for breaking up or complete removal. > */ > unsigned split_candidate : 1; > + /* True iff the parameter should not be disqualified as a split candidate. > + Only useful for forcing transformations, such as when optimizing .ASSUME > + bodies. */ > + unsigned dont_disqualify : 1; If we want to keep this, it would logically belong to isra_func_summary, it describes the whole function and cannot have different value for different candidate parameters, which is what isra_param_desc describes. If we want to keep this, the flag would need to have name like "ignore_size_constraints" or similar. I don't believe we can simply never disqualify candidates, at least in the case of overlapping accesses or variable-sized accesses we most certainly do and quite likely for other cases as well. But if the only case when we want to disregard limits on parameter growth are assume functions, I think we can just test node->assume_function - or create an appropriately named predicate doing that. > /* Is this a parameter passing stuff by reference? */ > unsigned by_ref : 1; > /* If set, this parameter can only be a candidate for removal if the > function > @@ -233,6 +239,10 @@ struct gensum_param_desc > by reference that is a candidate for being converted to a set of > parameters passing those data by value. */ > bool split_candidate; > + /* True iff the parameter should not be disqualified as a split candidate. > + Only useful for forcing transformations, such as when optimizing .ASSUME > + bodies. */ > + bool dont_disqualify; > /* Is this a parameter passing stuff by reference (either a pointer or a > source language reference type)? */ > bool by_ref; > @@ -461,6 +471,7 @@ ipa_sra_function_summaries::duplicate (cgraph_node *, > cgraph_node *, > d->safe_size = s->safe_size; > d->locally_unused = s->locally_unused; > d->split_candidate = s->split_candidate; > + d->dont_disqualify = s->dont_disqualify; > d->by_ref = s->by_ref; > d->remove_only_when_retval_removed = > s->remove_only_when_retval_removed; > d->split_only_when_retval_removed = s->split_only_when_retval_removed; > @@ -754,7 +765,9 @@ dump_gensum_param_descriptor (FILE *f, gensum_param_desc > *desc) > fprintf (f, " unused with %i call_uses%s\n", desc->call_uses, > desc->remove_only_when_retval_removed ? > " remove_only_when_retval_removed" : ""); > - if (!desc->split_candidate) > + if (desc->dont_disqualify) > + fprintf (f, " dont_disqualify\n"); > + else if (!desc->split_candidate) > { > fprintf (f, " not a candidate\n"); > return; > @@ -1131,12 +1144,13 @@ ptr_parm_has_nonarg_uses (cgraph_node *node, function > *fun, tree parm, > > cgraph_edge *cs = node->get_edge (stmt); > gcc_checking_assert (cs); > + callback_info *ci = callback_info_sum->get (cs); > isra_call_summary *csum = call_sums->get_create (cs); > csum->init_inputs (arg_count); > > for (unsigned i = 0; i < arg_count; ++i) > { > - tree arg = gimple_call_arg (stmt, i); > + tree arg = callback_call_arg_summary (stmt, i, ci); I don't think the use of callback_call_arg_summary is correct here. Why invoke it for every parameter in the original call. What would happen if the i-th parameter is mapped to -1 in the summary? Moreover, this code would simply never get executed because there is an early break above this hunk if gimple_call_internal_p(call). To have this path - and the case it covers - tested, I think we need a testcase with a "pass-through" split propagating into the assume function from the caller of the real one through the real one. And another reason why the change is moot is that this function gets called before you create the assume callback edges with their summaries. More generally, I'm afraid that this function needs more work because even when thee is a callback, it has to work also when there are more callback edges and it also has to correctly set up the call summary for the callback carrying-edge if there is one, otherwise things will break horribly once you expose the attribute to the user. > > if (arg == name) > { > @@ -1216,6 +1230,8 @@ create_parameter_descriptors (cgraph_node *node, > gensum_param_desc *desc = &(*param_descriptions)[num]; > /* param_descriptions vector is grown cleared in the caller. */ > desc->param_number = num; > + /* dont_disqualify is only used for outlined .ASSUME bodies. */ > + desc->dont_disqualify = node->assume_function; > decl2desc->put (parm, desc); > > if (dump_file && (dump_flags & TDF_DETAILS)) > @@ -1387,7 +1403,7 @@ get_gensum_param_desc (tree decl) > static void > disqualify_split_candidate (gensum_param_desc *desc, const char *reason) > { > - if (!desc->split_candidate) > + if (!desc->split_candidate || desc->dont_disqualify) > return; Like I already implied above, the check to allow splitting for assumes even when we would otherwise allow it cannot be done here. We still want to disqualify at least overlapping accesses or variable-sized accesses and perhaps more. I'm afraid all calls to disqualify_split_candidate will need to be examine individually and the check done at the side of the caller. > > if (dump_file && (dump_flags & TDF_DETAILS)) > @@ -2056,13 +2072,36 @@ scan_function (cgraph_node *node, struct function > *fun) > unsigned argument_count = gimple_call_num_args (stmt); > isra_scan_context ctx = ISRA_CTX_ARG; > scan_call_info call_info, *call_info_p = &call_info; > + > if (gimple_call_internal_p (stmt)) > { > - call_info_p = NULL; > - ctx = ISRA_CTX_LOAD; > - internal_fn ifn = gimple_call_internal_fn (stmt); > - if (internal_store_fn_p (ifn)) > - ctx = ISRA_CTX_STORE; > + if (gimple_call_internal_fn (stmt) == IFN_ASSUME) > + { > + tree callee_decl > + = TREE_OPERAND (gimple_call_arg (stmt, 0), 0); > + if (!lookup_attribute ("noipa", > + DECL_ATTRIBUTES (callee_decl))) > + { > + cgraph_node *target > + = cgraph_node::get_create (callee_decl); > + cgraph_edge *cbei > + = cgraph_edge::make_callback_internal ( > + node, target, as_a<gcall *> (stmt)); > + callback_info *ci > + = callback_info::initialize_internal_summary ( > + cbei, IFN_ASSUME); > + argument_count = ci->arg_mapping.length (); > + } > + } > + else > + { > + call_info_p = NULL; > + call_info.cs = NULL; > + ctx = ISRA_CTX_LOAD; > + internal_fn ifn = gimple_call_internal_fn (stmt); > + if (internal_store_fn_p (ifn)) > + ctx = ISRA_CTX_STORE; > + } > } > else > { > @@ -2073,8 +2112,11 @@ scan_function (cgraph_node *node, struct function *fun) > for (unsigned i = 0; i < argument_count; i++) > { > call_info.arg_idx = i; > - scan_expr_access (gimple_call_arg (stmt, i), stmt, > - ctx, bb, call_info_p); > + callback_info *ci = NULL; > + if (call_info.cs) > + ci = callback_info_sum->get (call_info.cs); > + scan_expr_access (callback_call_arg_summary (stmt, i, ci), > + stmt, ctx, bb, call_info_p); In the loop, i iterates from 0 to the number of arguments in the call - not th number of parameters of the callback, so the use of callback_call_arg_summary seems wrong here too. Moreover, again we must update summaries of all call graph edges associated with the call statement and there may be more than one. So this function and its callees have a similar problem like ptr_parm_has_nonarg_uses. (OTOH this is only important for record_nonregister_call_use, so perhaps we could ignore this for assumes, but if we are going to do IPA-SRA across other callbacks...) > } > > tree lhs = gimple_call_lhs (stmt); > @@ -2183,13 +2225,14 @@ ssa_name_only_returned_p (function *fun, tree name, > bitmap analyzed) > static void > isra_analyze_call (cgraph_edge *cs) > { > + callback_info *ci = callback_info_sum->get (cs); > gcall *call_stmt = cs->call_stmt; > - unsigned count = gimple_call_num_args (call_stmt); > + unsigned count = callback_call_num_args_summary (call_stmt, ci); > isra_call_summary *csum = call_sums->get_create (cs); > > for (unsigned i = 0; i < count; i++) > { > - tree arg = gimple_call_arg (call_stmt, i); > + tree arg = callback_call_arg_summary (call_stmt, i, ci); > if (TREE_CODE (arg) == ADDR_EXPR) > { > poly_int64 poffset, psize, pmax_size; > @@ -2607,7 +2650,8 @@ process_scan_results (cgraph_node *node, struct > function *fun, > const attribute is wrong) and then we just don't care. */ > bool uses_memory_as_obtained = vuse && SSA_NAME_IS_DEFAULT_DEF (vuse); > > - unsigned count = gimple_call_num_args (call_stmt); > + callback_info *ci = callback_info_sum->get (cs); > + unsigned count = callback_call_num_args_summary (call_stmt, ci); Likewise. > isra_call_summary *csum = call_sums->get_create (cs); > csum->init_inputs (count); > csum->m_before_any_store = uses_memory_as_obtained; > @@ -2690,6 +2734,7 @@ process_scan_results (cgraph_node *node, struct > function *fun, > d->size_reached = s->nonarg_acc_size; > d->locally_unused = s->locally_unused; > d->split_candidate = s->split_candidate; > + d->dont_disqualify = s->dont_disqualify; > d->by_ref = s->by_ref; > d->remove_only_when_retval_removed = > s->remove_only_when_retval_removed; > d->split_only_when_retval_removed = s->split_only_when_retval_removed; > @@ -2778,6 +2823,7 @@ ipa_sra_generate_summary (void) > = (new (ggc_alloc_no_dtor <ipa_sra_function_summaries> ()) > ipa_sra_function_summaries (symtab, true)); > call_sums = new ipa_sra_call_summaries (symtab); > + callback_info_sum_t::check_create_info_sum (); > > FOR_EACH_FUNCTION_WITH_GIMPLE_BODY (node) > ipa_sra_summarize_function (node); > @@ -2852,6 +2898,7 @@ isra_write_node_summary (output_block *ob, cgraph_node > *node) > bitpack_d bp = bitpack_create (ob->main_stream); > bp_pack_value (&bp, desc->locally_unused, 1); > bp_pack_value (&bp, desc->split_candidate, 1); > + bp_pack_value (&bp, desc->dont_disqualify, 1); > bp_pack_value (&bp, desc->by_ref, 1); > gcc_assert (!desc->not_specially_constructed); > bp_pack_value (&bp, desc->remove_only_when_retval_removed, 1); > @@ -2978,6 +3025,7 @@ isra_read_node_info (struct lto_input_block *ib, > cgraph_node *node, > bitpack_d bp = streamer_read_bitpack (ib); > desc->locally_unused = bp_unpack_value (&bp, 1); > desc->split_candidate = bp_unpack_value (&bp, 1); > + desc->dont_disqualify = bp_unpack_value (&bp, 1); > desc->by_ref = bp_unpack_value (&bp, 1); > desc->not_specially_constructed = 0; > desc->remove_only_when_retval_removed = bp_unpack_value (&bp, 1); > @@ -3114,13 +3162,21 @@ ipa_sra_dump_all_summaries (FILE *f, bool hints) > fprintf (f, "\n\n"); > } > > +/* Return TRUE iff the node can be made local or if it's an artificial > .ASSUME > + body (addressable, but only visible in the current TU). */ > +static bool > +can_be_local_p (cgraph_node *n) > +{ > + return n->can_be_local_p () || n->assume_function; > +} > + > /* Perform function-scope viability tests that can be only made at IPA level > and return false if the function is deemed unsuitable for IPA-SRA. */ > > static bool > ipa_sra_ipa_function_checks (cgraph_node *node) > { > - if (!node->can_be_local_p ()) > + if (!can_be_local_p (node)) I'm afraid I don't see the benefit of the IPA-SRA-special predicate here while it hides what is going on. Either we want to allow assume functions also in cgraph::can_be_local_p (it looks like we could?) or let's just simply openly test the cgraph_node flag here too. Thanks again for working on this and sorry that it took so much time to review. Martin
