Hello, please find new review comments inline below.
On Wed, Jul 30 2025, Josef Melcr wrote: > Hi, > This is the 3rd version of this patch. > Changes made since v2: > - implemented Martin's suggestions, mostly regarding naming One more naming nit. In various comments and dumping strings, you use the term "offloading" function. I guess the term is somewhat understandable in the context of OpenMP/OpenACC (but there it usually means something offloading computation to a GPU or other such special device, not running GOMP_parallel). I'd suggest "callback-dispatching" or something similar and more general (qsort is not offloading in any way). But this is not particularly important, thanks for the effort to get rid of "parent" and "children," that really helped. > - unintentionally removed lines added back > - unknown arguments are no longer represented with an identity to the > callback-carrying edge, zeroed-out jump functions are used instead > - supporting functions moved from attr-callback.h to a new .cc file > - fixed a bug in cgraph_edge::set_call_stmt where the callback-carrying > edge could be missed > - callback_hash renamed to callback_id, index of the callback is now used > in place of the hash > - each function parameter can now have at most a single callback > attribute > > GOMP_task remains special-cased in the same manner as in v2. Bootstrapped > and regtested on x86_64-linux. > > Best regards, > Josef Melcr > > gcc/ChangeLog: > > * Makefile.in: Add attr-callback.o to OBJS. > * builtin-attrs.def (ATTR_CALLBACK): Callback attr identifier. > (DEF_CALLBACK_ATTRIBUTE): Macro for callback attr creation. > (GOMP): Attrs for libgomp functions. > (OACC): Attrs for oacc functions. > (ATTR_CALLBACK_GOMP_LIST): ATTR_NOTHROW_LIST with GOMP callback > attr added. > (ATTR_CALLBACK_OACC_LIST): ATTR_NOTHROW_LIST with OACC callback > attr added. > * cgraph.cc (cgraph_add_edge_to_call_site_hash): Always hash the > callback-carrying edge. > (cgraph_node::get_edge): Always return the callback-carrying > edge. > (cgraph_edge::set_call_stmt): Add cascade for callback edges, > rename update_speculative to update_derived_edges. > (symbol_table::create_edge): Allow callback edges to share call > statements, initialize new flags. > (cgraph_edge::make_callback): New method, derives a new callback > edge. > (cgraph_edge::get_callback_carrying_edge): New method. > (cgraph_edge::first_callback_edge): Likewise. > (cgraph_edge::next_callback_edge): Likewise. > (cgraph_edge::purge_callback_edges): Likewise. > (cgraph_edge::redirect_callee): When redirecting a callback > edge, redirect its ref as well. > (cgraph_edge::redirect_call_stmt_to_callee): Add callback edge > redirection logic, set update_derived_edges to true if > redirecting callback-carrying edge. > (cgraph_node::remove_callers): Add cascade for callback edges. > (cgraph_edge::dump_edge_flags): Add callback flag printing. > (cgraph_node::verify_node): Add sanity checks for callback > edges. > * cgraph.h: Add new flags and 16 bit callback_id to cgraph_edge > class. > * cgraphclones.cc (cgraph_edge::clone): Copy over callback data. > * ipa-cp.cc (purge_useless_callback_edges): New function, > deletes callback edges when necessary. > (ipcp_decision_stage): Call purge_useless_callback_edges. > * ipa-fnsummary.cc (ipa_call_summary_t::duplicate): Add an > exception for callback edges. > (analyze_function_body): Copy summary from callback-carrying to > callback edge. > * ipa-inline-analysis.cc (do_estimate_growth_1): Skip callback > edges when estimating growth. > * ipa-inline-transform.cc (inline_transform): Add redirection > cascade for callback edges. > * ipa-inline.cc (can_inline_edge_p): Never inline callback > edges. > * ipa-param-manipulation.cc > (drop_decl_attribute_if_params_changed_p): New function. > (ipa_param_adjustments::build_new_function_type): Add > args_modified out parameter. > (ipa_param_adjustments::adjust_decl): Drop callback attrs when > modifying args. > * ipa-param-manipulation.h: Change decl of > build_new_function_type. > * ipa-prop.cc (ipa_duplicate_jump_function): Add declaration. > (init_callback_edge_summary): New function. > (ipa_compute_jump_functions_for_edge): Add callback edge > creation logic. > * lto-cgraph.cc (lto_output_edge): Stream out callback data. > (input_edge): Input callback data. > * omp-builtins.def (BUILT_IN_GOACC_PARALLEL): Use new attr list. > (BUILT_IN_GOMP_PARALLEL_LOOP_STATIC): Likewise. > (BUILT_IN_GOMP_PARALLEL_LOOP_GUIDED): Likewise. > (BUILT_IN_GOMP_PARALLEL_LOOP_NONMONOTONIC_DYNAMIC): Likewise. > (BUILT_IN_GOMP_PARALLEL_LOOP_NONMONOTONIC_RUNTIME): Likewise. > (BUILT_IN_GOMP_PARALLEL): Likewise. > (BUILT_IN_GOMP_PARALLEL_SECTIONS): Likewise. > (BUILT_IN_GOMP_TEAMS_REG): Likewise. > * tree-core.h (ECF_CB_1_2): New constant for callback(1,2). > (ECF_CB_2_4): New constant for callback(2,4). > * tree-inline.cc (copy_bb): Copy callback edges when copying > callback-carrying edge. > (redirect_all_calls): Redirect callback edges. > * tree.cc (set_call_expr_flags): Create callback attrs according > to the ECF_CB constants. > * attr-callback.cc: New file. > * attr-callback.h: New file. > > gcc/c-family/ChangeLog: > > * c-attribs.cc: Define callback attribute. > > gcc/fortran/ChangeLog: > > * f95-lang.cc (ATTR_CALLBACK_GOMP_LIST): New attr list > corresponding to the list in builtin-attrs.def. > (ATTR_CALLBACK_OACC_LIST): Likewise. > > gcc/testsuite/ChangeLog: > > * gcc.dg/ipa/ipcp-cb-spec1.c: New test. > * gcc.dg/ipa/ipcp-cb-spec2.c: New test. > * gcc.dg/ipa/ipcp-cb1.c: New test. It seems you have lost the test in testsuite/gcc.dg/attr-callback.c ? It looked useful. > > Signed-off-by: Josef Melcr <[email protected]> > --- > gcc/Makefile.in | 1 + > gcc/attr-callback.cc | 354 +++++++++++++++++++++++ > gcc/attr-callback.h | 73 +++++ > gcc/builtin-attrs.def | 14 + > gcc/c-family/c-attribs.cc | 3 + > gcc/cgraph.cc | 291 ++++++++++++++++++- > gcc/cgraph.h | 54 +++- > gcc/cgraphclones.cc | 3 + > gcc/fortran/f95-lang.cc | 2 + > gcc/ipa-cp.cc | 73 ++++- > gcc/ipa-fnsummary.cc | 24 +- > gcc/ipa-inline-analysis.cc | 5 + > gcc/ipa-inline-transform.cc | 12 +- > gcc/ipa-inline.cc | 5 + > gcc/ipa-param-manipulation.cc | 37 ++- > gcc/ipa-param-manipulation.h | 3 +- > gcc/ipa-prop.cc | 103 ++++++- > gcc/lto-cgraph.cc | 6 + > gcc/omp-builtins.def | 26 +- > gcc/testsuite/gcc.dg/ipa/ipcp-cb-spec1.c | 19 ++ > gcc/testsuite/gcc.dg/ipa/ipcp-cb-spec2.c | 21 ++ > gcc/testsuite/gcc.dg/ipa/ipcp-cb1.c | 25 ++ > gcc/tree-core.h | 8 + > gcc/tree-inline.cc | 27 +- > gcc/tree.cc | 18 +- > 25 files changed, 1166 insertions(+), 41 deletions(-) > create mode 100644 gcc/attr-callback.cc > create mode 100644 gcc/attr-callback.h > create mode 100644 gcc/testsuite/gcc.dg/ipa/ipcp-cb-spec1.c > create mode 100644 gcc/testsuite/gcc.dg/ipa/ipcp-cb-spec2.c > create mode 100644 gcc/testsuite/gcc.dg/ipa/ipcp-cb1.c > > diff --git a/gcc/Makefile.in b/gcc/Makefile.in > index d7d5cbe7277..542449f29a4 100644 > --- a/gcc/Makefile.in > +++ b/gcc/Makefile.in > @@ -1844,6 +1844,7 @@ OBJS = \ > web.o \ > wide-int.o \ > wide-int-print.o \ > + attr-callback.o \ > $(out_object_file) \ > $(ANALYZER_OBJS) \ > $(EXTRA_OBJS) \ > diff --git a/gcc/attr-callback.cc b/gcc/attr-callback.cc > new file mode 100644 > index 00000000000..10e9afb5fc8 > --- /dev/null > +++ b/gcc/attr-callback.cc > @@ -0,0 +1,354 @@ > +/* Callback attribute handling > + Copyright (C) 2025 Free Software Foundation, Inc. > + Contributed by Josef Melcr <[email protected]> > + > + This file is part of GCC. > + > + GCC is free software; you can redistribute it and/or modify > + under the terms of the GNU General Public License as published by > + the Free Software Foundation; either version 3 of the License, or > + (at your option) any later version. > + > + GCC is distributed in the hope that it will be useful, > + but WITHOUT ANY WARRANTY; without even the implied warranty of > + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + GNU General Public License for more details. > + > + You should have received a copy of the GNU General Public License > + along with GCC; see the file COPYING3. If not see > + <http://www.gnu.org/licenses/>. */ > + > +#include "config.h" > +#include "system.h" > +#include "coretypes.h" > +#include "backend.h" > +#include "tree.h" > +#include "gimple.h" > +#include "alloc-pool.h" > +#include "cgraph.h" > +#include "diagnostic.h" > +#include "builtins.h" > +#include "options.h" > +#include "gimple-range.h" > +#include "attribs.h" > +#include "attr-callback.h" > + > +/* Returns a callback attribute with callback index FN_IDX, and ARG_COUNT > + arguments specified by VA_ARGS. */ > +tree > +callback_build_attr (unsigned fn_idx, unsigned arg_count...) > +{ > + va_list args; > + va_start (args, arg_count); > + > + tree cblist = NULL_TREE; > + tree *pp = &cblist; > + unsigned i; > + for (i = 0; i < arg_count; i++) > + { > + int num = va_arg (args, int); > + tree tnum = build_int_cst (integer_type_node, num); > + *pp = build_tree_list (NULL, tnum PASS_MEM_STAT); > + pp = &TREE_CHAIN (*pp); > + } > + cblist > + = tree_cons (NULL_TREE, build_int_cst (integer_type_node, fn_idx), > cblist); > + tree attr > + = tree_cons (get_identifier (CALLBACK_ATTR_IDENT), cblist, NULL_TREE); > + return attr; > +} > + > +/* Returns TRUE if a function should be treated as if it had a callback > + attribute despite the DECL not having it. */ Please describe the stmt parameter. I am a bit surprised we can afford to be optimistic when it is NULL - though in the contexts where it is used from WPA it might be OK. But it would be nice to describe when it is OK to pass NULL and when not. > +bool > +callback_is_special_cased (tree decl, gcall *stmt) > +{ > + if (fndecl_built_in_p (decl, BUILT_IN_GOMP_TASK)) > + { > + if (stmt) > + { > + return gimple_call_arg (stmt, 2) == null_pointer_node; > + } Superfluous braces. > + return true; > + } > + return false; > +} > + > +/* Returns an attribute for a special cased function. */ > +tree > +callback_special_case_attr (tree decl) > +{ > + if (fndecl_built_in_p (decl, BUILT_IN_GOMP_TASK)) > + return callback_build_attr (1, 1, 2); > + gcc_unreachable (); > +} > + > +/* Given an instance of callback attribute, return the 0-based > + index of the called function in question. */ > +int > +callback_get_fn_index (tree cb_attr) > +{ > + tree args = TREE_VALUE (cb_attr); > + int idx = TREE_INT_CST_LOW (TREE_VALUE (args)) - 1; > + return idx; > +} > + > +/* For a given callback pair, retrieves the callback attribute used > + to create E from the callee of CARRYING. */ > +tree > +callback_fetch_attr_by_edge (cgraph_edge *e, cgraph_edge *carrying) > +{ > + gcc_checking_assert (e->call_stmt == carrying->call_stmt > + && e->lto_stmt_uid == carrying->lto_stmt_uid); > + > + if (callback_is_special_cased (carrying->callee->decl, e->call_stmt)) > + return callback_special_case_attr (carrying->callee->decl); > + > + tree cb_attr = lookup_attribute (CALLBACK_ATTR_IDENT, > + DECL_ATTRIBUTES (carrying->callee->decl)); > + gcc_checking_assert (cb_attr); > + tree res = NULL_TREE; > + for (; cb_attr; > + cb_attr = lookup_attribute (CALLBACK_ATTR_IDENT, TREE_CHAIN > (cb_attr))) > + { > + unsigned id = callback_get_fn_index (cb_attr); > + if (id == e->callback_id) > + { > + res = cb_attr; > + break; > + } > + } > + gcc_checking_assert (res != NULL_TREE); > + return res; > +} > + > +/* Given an instance of callback attribute, return the 0-base indices > + of arguments passed to the callback. For a callback function taking > + n parameters, returns a vector of n indices of their values in the > parameter > + list of it's caller. Indices with unknown positions contain -1. */ > +auto_vec<int> > +callback_get_arg_mapping (cgraph_edge *e, cgraph_edge *carrying) > +{ > + tree attr = callback_fetch_attr_by_edge (e, carrying); > + gcc_checking_assert (attr); > + tree args = TREE_VALUE (attr); > + auto_vec<int> res; > + tree it; > + > + /* Skip over the first argument, which denotes > + which argument is the called function. */ > + for (it = TREE_CHAIN (args); it != NULL_TREE; it = TREE_CHAIN (it)) > + { > + int idx = TREE_INT_CST_LOW (TREE_VALUE (it)); > + /* Subtract 1 to account for 1-based indexing. If the value is > unknown, > + use constant -1 instead. */ > + idx = idx == CB_UNKNOWN_POS ? -1 : idx - 1; > + res.safe_push (idx); > + } > + > + return res; > +} > + > +/* For a callback pair, returns the 0-based index of the address of > + E's callee in the argument list of CARRYING's callee decl. */ > +int > +callback_fetch_fn_position (cgraph_edge *e, cgraph_edge *carrying) > +{ > + tree attr = callback_fetch_attr_by_edge (e, carrying); > + return callback_get_fn_index (attr); > +} > + > +/* Returns the element at index idx in the list or NULL_TREE if > + the list isn't long enough. NULL_TREE is used as the endpoint. */ > +static tree > +get_nth_list_elem (tree list, unsigned idx) > +{ > + tree res = NULL_TREE; > + unsigned i = 0; > + tree it; > + for (it = list; it != NULL_TREE; it = TREE_CHAIN (it), i++) > + { > + if (i == idx) > + { > + res = TREE_VALUE (it); > + break; > + } > + } > + return res; > +} > + > +/* Handle a "callback" attribute; arguments as in > + struct attribute_spec.handler. */ > +tree > +handle_callback_attribute (tree *node, tree name, tree args, > + int ARG_UNUSED (flags), bool *no_add_attrs) > +{ > + tree decl = *node; > + if (TREE_CODE (decl) != FUNCTION_DECL) > + { > + error_at (DECL_SOURCE_LOCATION (decl), > + "%qE attribute can only be used on functions", name); > + *no_add_attrs = true; > + } > + > + tree cb_fn_idx_node = TREE_VALUE (args); > + if (TREE_CODE (cb_fn_idx_node) != INTEGER_CST) > + { > + error_at (DECL_SOURCE_LOCATION (decl), > + "argument specifying callback function position is not an " > + "integer constant"); > + *no_add_attrs = true; > + return NULL_TREE; > + } > + /* We have to use the function type for validation, as > + DECL_ARGUMENTS returns NULL at this point. */ > + int callback_fn_idx = TREE_INT_CST_LOW (cb_fn_idx_node); > + tree decl_type_args = TYPE_ARG_TYPES (TREE_TYPE (decl)); > + tree it; > + int decl_nargs = list_length (decl_type_args); > + for (it = decl_type_args; it != NULL_TREE; it = TREE_CHAIN (it)) > + if (it == void_list_node) > + { > + --decl_nargs; > + break; > + } > + if (callback_fn_idx == CB_UNKNOWN_POS) > + { > + error_at (DECL_SOURCE_LOCATION (decl), > + "callback function position cannot be marked as unknown"); > + *no_add_attrs = true; > + return NULL_TREE; > + } > + --callback_fn_idx; > + if (callback_fn_idx >= decl_nargs) > + { > + error_at (DECL_SOURCE_LOCATION (decl), > + "callback function position out of range"); > + *no_add_attrs = true; > + return NULL_TREE; > + } > + > + /* Search for the type of the callback function > + in parameters of the original function. */ > + tree cfn = get_nth_list_elem (decl_type_args, callback_fn_idx); > + if (cfn == NULL_TREE) > + { > + error_at (DECL_SOURCE_LOCATION (decl), > + "could not retrieve callback function from arguments"); > + *no_add_attrs = true; > + return NULL_TREE; > + } > + tree cfn_pointee_type = TREE_TYPE (cfn); > + if (TREE_CODE (cfn) != POINTER_TYPE > + || TREE_CODE (cfn_pointee_type) != FUNCTION_TYPE) > + { > + error_at (DECL_SOURCE_LOCATION (decl), > + "argument no. %d is not an address of a function", > + callback_fn_idx + 1); > + *no_add_attrs = true; > + return NULL_TREE; > + } > + > + tree type_args = TYPE_ARG_TYPES (cfn_pointee_type); > + /* Compare the length of the list of argument indices > + and the real number of parameters the callback takes. */ > + unsigned cfn_nargs = list_length (TREE_CHAIN (args)); > + unsigned type_nargs = list_length (type_args); > + for (it = type_args; it != NULL_TREE; it = TREE_CHAIN (it)) > + if (it == void_list_node) > + { > + --type_nargs; > + break; > + } > + if (cfn_nargs != type_nargs) > + { > + error_at (DECL_SOURCE_LOCATION (decl), > + "argument number mismatch, %d expected, got %d", type_nargs, > + cfn_nargs); > + *no_add_attrs = true; > + return NULL_TREE; > + } > + > + unsigned curr = 0; > + tree cfn_it; > + /* Validate type compatibility of the arguments passed > + from caller function to callback. "it" is used to step > + through the parameters of the caller, "cfn_it" is > + stepping through the parameters of the callback. */ > + for (it = type_args, cfn_it = TREE_CHAIN (args); curr < type_nargs; > + it = TREE_CHAIN (it), cfn_it = TREE_CHAIN (cfn_it), curr++) > + { > + if (TREE_CODE (TREE_VALUE (cfn_it)) != INTEGER_CST) > + { > + error_at (DECL_SOURCE_LOCATION (decl), > + "argument no. %d is not an integer constant", curr + 1); > + *no_add_attrs = true; > + continue; > + } > + > + int arg_idx = TREE_INT_CST_LOW (TREE_VALUE (cfn_it)); > + > + /* No need to check for type compatibility, > + if we don't know what we are passing. */ > + if (arg_idx == CB_UNKNOWN_POS) > + { > + continue; > + } Superfluous braces. > + > + arg_idx -= 1; > + /* Report an error if the position is out of bounds, > + but we can still check the rest of the arguments. */ > + if (arg_idx >= decl_nargs) > + { > + error_at (DECL_SOURCE_LOCATION (decl), > + "callback argument index %d is out of range", arg_idx + 1); > + *no_add_attrs = true; > + continue; > + } > + > + tree arg_type = get_nth_list_elem (decl_type_args, arg_idx); > + tree expected_type = TREE_VALUE (it); > + /* Check the type of the value we are about to pass ("arg_type") > + for compatibility with the actual type the callback function > + expects ("expected_type"). */ > + if (!types_compatible_p (expected_type, arg_type)) > + { > + error_at (DECL_SOURCE_LOCATION (decl), > + "argument type at index %d is not compatible with callback " > + "argument type at index %d", > + arg_idx + 1, curr + 1); > + *no_add_attrs = true; > + continue; > + } > + } > + > + /* Check that the decl does not already have a callback attribute > describing > + the same argument. */ > + it = lookup_attribute (CALLBACK_ATTR_IDENT, DECL_ATTRIBUTES (decl)); > + for (; it; it = lookup_attribute (CALLBACK_ATTR_IDENT, TREE_CHAIN (it))) > + if (callback_get_fn_index (it) == callback_fn_idx) > + { > + error_at (DECL_SOURCE_LOCATION (decl), > + "function declaration has multiple callback attributes " > + "describing argument no. %d", > + callback_fn_idx + 1); > + *no_add_attrs = true; > + break; > + } > + > + return NULL_TREE; > +} > + > +/* Returns TRUE if E is considered useful in the callgraph, FALSE otherwise. > If > + this predicate returns FALSE, then E wasn't used to optimize its callee > and > + can be safely removed from the callgraph. */ > +bool > +callback_edge_useful_p (cgraph_edge *e) > +{ > + gcc_checking_assert (e->callback); > + /* If the edge is not pointing towards a clone, it is no longer useful as > its > + entire purpose is to produce clones of callbacks. */ > + if (!e->callee->clone_of) > + return false; > + return true; > +} > diff --git a/gcc/attr-callback.h b/gcc/attr-callback.h > new file mode 100644 > index 00000000000..39255b690fb > --- /dev/null > +++ b/gcc/attr-callback.h > @@ -0,0 +1,73 @@ > +/* Callback attribute handling > + Copyright (C) 2025 Free Software Foundation, Inc. > + Contributed by Josef Melcr <[email protected]> > + > + This file is part of GCC. > + > + GCC is free software; you can redistribute it and/or modify > + under the terms of the GNU General Public License as published by > + the Free Software Foundation; either version 3 of the License, or > + (at your option) any later version. > + > + GCC is distributed in the hope that it will be useful, > + but WITHOUT ANY WARRANTY; without even the implied warranty of > + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + GNU General Public License for more details. > + > + You should have received a copy of the GNU General Public License > + along with GCC; see the file COPYING3. If not see > + <http://www.gnu.org/licenses/>. */ > + > +#ifndef ATTR_CALLBACK_H > +#define ATTR_CALLBACK_H > + > +enum callback_position > +{ > + /* Value used when an argument of a callback function > + is unknown or when multiple values may be used. */ > + CB_UNKNOWN_POS = 0 > +}; > + > +#define CALLBACK_ATTR_IDENT " callback" > + > +/* Returns a callback attribute with callback index FN_IDX, and ARG_COUNT > + arguments specified by VA_ARGS. */ > +tree callback_build_attr (unsigned fn_idx, unsigned arg_count...); > + > +/* Returns TRUE if a function should be treated as if it had a callback > + attribute despite the DECL not having it. */ > +bool callback_is_special_cased (tree decl, gcall *stmt); > + > +/* Returns an attribute for a special cased function. */ > +tree callback_special_case_attr (tree decl); > + > +/* Given an instance of callback attribute, return the 0-based > + index of the called function in question. */ > +int callback_get_fn_index (tree cb_attr); > + > +/* For a given callback pair, retrieves the callback attribute used > + to create E from the callee of CARRYING. */ > +tree callback_fetch_attr_by_edge (cgraph_edge *e, cgraph_edge *carrying); > + > +/* Given an instance of callback attribute, return the 0-base indices > + of arguments passed to the callback. For a callback function taking > + n parameters, returns a vector of n indices of their values in the > parameter > + list of it's caller. Indices with unknown positions will be filled with > + an identity. */ This comment has not been updated. > +auto_vec<int> callback_get_arg_mapping (cgraph_edge *e, cgraph_edge > *carrying); > + > +/* For a callback pair, returns the 0-based index of the address of > + E's callee in the argument list of CARRYING's callee decl. */ > +int callback_fetch_fn_position (cgraph_edge *e, cgraph_edge *carrying); > + > +/* Handle a "callback" attribute; arguments as in > + struct attribute_spec.handler. */ > +tree handle_callback_attribute (tree *node, tree name, tree args, int flags, > + bool *no_add_attrs); > + > +/* Returns TRUE if E is considered useful in the callgraph, FALSE otherwise. > If > + this predicate returns FALSE, then E wasn't used to optimize its callee > and > + can be safely removed from the callgraph. */ > +bool callback_edge_useful_p (cgraph_edge *e); > + > +#endif /* ATTR_CALLBACK_H */ > diff --git a/gcc/builtin-attrs.def b/gcc/builtin-attrs.def > index 2b82fc2326e..3c0197e87bc 100644 > --- a/gcc/builtin-attrs.def > +++ b/gcc/builtin-attrs.def > @@ -130,6 +130,7 @@ DEF_ATTR_IDENT (ATTR_TM_TMPURE, "transaction_pure") > DEF_ATTR_IDENT (ATTR_RETURNS_TWICE, "returns_twice") > DEF_ATTR_IDENT (ATTR_RETURNS_NONNULL, "returns_nonnull") > DEF_ATTR_IDENT (ATTR_WARN_UNUSED_RESULT, "warn_unused_result") > +DEF_ATTR_IDENT (ATTR_CALLBACK, " callback") > > DEF_ATTR_TREE_LIST (ATTR_NOVOPS_LIST, ATTR_NOVOPS, ATTR_NULL, ATTR_NULL) > > @@ -430,6 +431,19 @@ DEF_FORMAT_ATTRIBUTE_NOTHROW(STRFMON,3,3_4) > #undef DEF_FORMAT_ATTRIBUTE_NOTHROW > #undef DEF_FORMAT_ATTRIBUTE_BOTH > > +/* Construct callback attributes for GOMP builtins. */ > +#define DEF_CALLBACK_ATTRIBUTE(TYPE, CA, VALUES) \ > + DEF_ATTR_TREE_LIST (ATTR_CALLBACK_##TYPE##_##CA##_##VALUES, ATTR_CALLBACK,\ > + ATTR_##CA, ATTR_LIST_##VALUES) > + > +DEF_CALLBACK_ATTRIBUTE(GOMP, 1, 2) > +DEF_CALLBACK_ATTRIBUTE(OACC, 2, 4) > +DEF_ATTR_TREE_LIST(ATTR_CALLBACK_GOMP_LIST, ATTR_CALLBACK, > + ATTR_CALLBACK_GOMP_1_2, ATTR_NOTHROW_LIST) > +DEF_ATTR_TREE_LIST(ATTR_CALLBACK_OACC_LIST, ATTR_CALLBACK, > + ATTR_CALLBACK_OACC_2_4, ATTR_NOTHROW_LIST) > +#undef DEF_CALLBACK_ATTRIBUTE > + > /* Transactional memory variants of the above. */ > > DEF_ATTR_TREE_LIST (ATTR_TM_NOTHROW_LIST, > diff --git a/gcc/c-family/c-attribs.cc b/gcc/c-family/c-attribs.cc > index 1f4a0df1205..5a1bcecce14 100644 > --- a/gcc/c-family/c-attribs.cc > +++ b/gcc/c-family/c-attribs.cc > @@ -49,6 +49,7 @@ along with GCC; see the file COPYING3. If not see > #include "tree-pretty-print.h" > #include "gcc-rich-location.h" > #include "gcc-urlifier.h" > +#include "attr-callback.h" > > static tree handle_packed_attribute (tree *, tree, tree, int, bool *); > static tree handle_nocommon_attribute (tree *, tree, tree, int, bool *); > @@ -465,6 +466,8 @@ const struct attribute_spec c_common_gnu_attributes[] = > handle_tm_attribute, NULL }, > { "transaction_may_cancel_outer", 0, 0, false, true, false, false, > handle_tm_attribute, NULL }, > + { CALLBACK_ATTR_IDENT, 1, -1, true, false, false, false, > + handle_callback_attribute, NULL }, > /* ??? These two attributes didn't make the transition from the > Intel language document to the multi-vendor language document. */ > { "transaction_pure", 0, 0, false, true, false, false, > diff --git a/gcc/cgraph.cc b/gcc/cgraph.cc > index 32071a84bac..c6fcd4375c1 100644 > --- a/gcc/cgraph.cc > +++ b/gcc/cgraph.cc > @@ -69,6 +69,7 @@ along with GCC; see the file COPYING3. If not see > #include "tree-nested.h" > #include "symtab-thunks.h" > #include "symtab-clones.h" > +#include "attr-callback.h" > > /* FIXME: Only for PROP_loops, but cgraph shouldn't have to know about this. > */ > #include "tree-pass.h" > @@ -870,11 +871,22 @@ cgraph_add_edge_to_call_site_hash (cgraph_edge *e) > one indirect); always hash the direct one. */ > if (e->speculative && e->indirect_unknown_callee) > return; > + /* We always want to hash the carrying edge of a callback, not the edges > + pointing to the callbacks themselves, as their call statement doesn't > + exist. */ > + if (e->callback) > + return; > cgraph_edge **slot = e->caller->call_site_hash->find_slot_with_hash > (e->call_stmt, cgraph_edge_hasher::hash (e->call_stmt), INSERT); > if (*slot) > { > - gcc_assert (((cgraph_edge *)*slot)->speculative); > + cgraph_edge *edge = (cgraph_edge *) *slot; > + gcc_assert (edge->speculative || edge->has_callback); > + if (edge->has_callback) > + /* If the slot is already occupied, then the hashed edge is the > + callback-carrying edge, which is desired behavior, so we can safely > + return. */ > + return; > if (e->callee && (!e->prev_callee > || !e->prev_callee->speculative > || e->prev_callee->call_stmt != e->call_stmt)) I still don't understand why the early exit is not enough. In one of your previous emails you wrote: > In some cases, the callback edge would be added before the > callback-carrying edge, occupying its slot and leading to a crash on the > assert when adding the callback-carrying edge. The early return > prevents that. Sadly, I am afraid I no longer have the code snippet to > reproduce the crash. ...and I'd think that the early return would be enough and the second condition is not necessary. But if it is not, I don't have an issue with the code as it is. > @@ -918,6 +930,13 @@ cgraph_node::get_edge (gimple *call_stmt) > n++; > } > > + /* We want to work with the callback-carrying edge whenever possible. > When it > + 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) > + e = e->get_callback_carrying_edge (); > + > if (n > 100) > { > call_site_hash = hash_table<cgraph_edge_hasher>::create_ggc (120); > @@ -931,14 +950,16 @@ cgraph_node::get_edge (gimple *call_stmt) > } > > > -/* Change field call_stmt of edge E to NEW_STMT. If UPDATE_SPECULATIVE and E > +/* Change field call_stmt of edge E to NEW_STMT. If UPDATE_DERIVED_EDGES > and E > is any component of speculative edge, then update all components. > Speculations can be resolved in the process and EDGE can be removed and > - deallocated. Return the edge that now represents the call. */ > + deallocated. If UPDATE_DERIVED_EDGES and E is a part of a callback pair, > + update every callback edge and their callback-carrying edge. Return the > edge > + that now represents the call. */ > > cgraph_edge * > cgraph_edge::set_call_stmt (cgraph_edge *e, gcall *new_stmt, > - bool update_speculative) > + bool update_derived_edges) > { > tree decl; > > @@ -954,7 +975,7 @@ cgraph_edge::set_call_stmt (cgraph_edge *e, gcall > *new_stmt, > > /* Speculative edges has three component, update all of them > when asked to. */ > - if (update_speculative && e->speculative > + if (update_derived_edges && e->speculative > /* If we are about to resolve the speculation by calling make_direct > below, do not bother going over all the speculative edges now. */ > && !new_direct_callee) > @@ -990,6 +1011,27 @@ cgraph_edge::set_call_stmt (cgraph_edge *e, gcall > *new_stmt, > if (new_direct_callee) > e = make_direct (e, new_direct_callee); > > + /* When updating a callback or a callback-carrying edge, update every edge > + involved. */ > + if (update_derived_edges && (e->callback || e->has_callback)) > + { > + cgraph_edge *current, *next, *carrying; > + carrying = e->has_callback ? e : e->get_callback_carrying_edge (); > + > + current = e->first_callback_edge (); > + if (current) > + { > + for (cgraph_edge *d = current; d; d = next) > + { > + next = d->next_callback_edge (); > + cgraph_edge *d2 = set_call_stmt (d, new_stmt, false); > + gcc_assert (d2 == d); > + } > + } > + carrying = set_call_stmt (carrying, new_stmt, false); > + return carrying; > + } > + I'm not sure that always returning the carrying edge is the expected thing to do. But it does not look like it ever matters that the original callback edge is returned, so perhaps it is OK to just document the behavior in the function comment - ah, it actually is done in the header file, so please just copy it to the .c file description too. (Though perhaps Honza will have a different opinion about this.) > /* Only direct speculative edges go to call_site_hash. */ > if (e->caller->call_site_hash > && (!e->speculative || !e->indirect_unknown_callee) > @@ -1035,7 +1077,7 @@ symbol_table::create_edge (cgraph_node *caller, > cgraph_node *callee, > construction of call stmt hashtable. */ > cgraph_edge *e; > gcc_checking_assert (!(e = caller->get_edge (call_stmt)) > - || e->speculative); > + || e->speculative || e->has_callback || e->callback); > > gcc_assert (is_gimple_call (call_stmt)); > } > @@ -1062,6 +1104,9 @@ symbol_table::create_edge (cgraph_node *caller, > cgraph_node *callee, > edge->indirect_info = NULL; > edge->indirect_inlining_edge = 0; > edge->speculative = false; > + edge->has_callback = false; > + edge->callback = false; > + edge->callback_id = 0; > edge->indirect_unknown_callee = indir_unknown_callee; > if (call_stmt && caller->call_site_hash) > cgraph_add_edge_to_call_site_hash (edge); > @@ -1285,6 +1330,123 @@ cgraph_edge::make_speculative (cgraph_node *n2, > profile_count direct_count, > return e2; > } > > +/* Create a callback edge calling N2. Callback edges > + never get turned into actual calls, they are just used > + as clues and allow for optimizing functions which do not > + have any callsites during compile time, e.g. functions > + passed to standard library functions. > + > + The edge will be attached to the same call statement as > + the callback-carrying edge, which is the instance this method > + is called on. > + > + callback_id is used to pair the returned edge with the attribute that > + originated it. > + > + Return the resulting callback edge. */ > + > +cgraph_edge * > +cgraph_edge::make_callback (cgraph_node *n2, unsigned int callback_id) > +{ > + cgraph_node *n = caller; > + cgraph_edge *e2; > + > + has_callback = true; > + e2 = n->create_edge (n2, call_stmt, count); > + if (dump_file) > + fprintf ( > + dump_file, > + "Created callback edge %s -> %s belonging to carrying edge %s -> %s\n", > + e2->caller->dump_name (), e2->callee->dump_name (), caller->dump_name > (), > + callee->dump_name ()); > + initialize_inline_failed (e2); > + e2->callback = true; > + e2->callback_id = callback_id; > + if (TREE_NOTHROW (n2->decl)) > + e2->can_throw_external = false; > + else > + e2->can_throw_external = can_throw_external; > + e2->lto_stmt_uid = lto_stmt_uid; > + n2->mark_address_taken (); > + return e2; > +} > + > +/* Returns the callback_carrying edge of a callback edge on which > + it is called on or NULL when no such edge can be found. > + > + An edge is taken to be the callback-carrying if it has it's has_callback > + flag set and the edges share their call statements. */ > + > +cgraph_edge * > +cgraph_edge::get_callback_carrying_edge () > +{ > + gcc_checking_assert (callback); > + cgraph_edge *e; > + for (e = caller->callees; e; e = e->next_callee) > + { > + if (e->has_callback && e->call_stmt == call_stmt > + && e->lto_stmt_uid == lto_stmt_uid) > + break; > + } > + return e; > +} > + > +/* Returns the first callback edge in the list of callees of the caller node. > + Note that the edges might be in arbitrary order. Must be called on a > + callback or callback-carrying edge. */ > + > +cgraph_edge * > +cgraph_edge::first_callback_edge () > +{ > + gcc_checking_assert (has_callback || callback); > + cgraph_edge *e = NULL; > + for (e = caller->callees; e; e = e->next_callee) > + { > + if (e->callback && e->call_stmt == call_stmt > + && e->lto_stmt_uid == lto_stmt_uid) > + { > + break; > + } Superfluous braces. > + } > + return e; > +} > + > +/* Given a callback edge, returns the next callback edge belonging to the > same > + carrying edge. Must be called on a callback edge, not the > callback-carrying > + edge. */ > + > +cgraph_edge * > +cgraph_edge::next_callback_edge () > +{ > + gcc_checking_assert (callback); > + cgraph_edge *e = NULL; > + for (e = next_callee; e; e = e->next_callee) > + { > + if (e->callback && e->call_stmt == call_stmt > + && e->lto_stmt_uid == lto_stmt_uid) > + { > + break; > + } Likewise. > + } > + return e; > +} > + > +/* When called on a callback-carrying edge, removes all of its attached > callback > + edges and sets has_callback to FALSE. */ > + > +void > +cgraph_edge::purge_callback_edges () > +{ > + gcc_checking_assert (has_callback); > + cgraph_edge *e, *next; > + for (e = first_callback_edge (); e; e = next) > + { > + next = e->next_callback_edge (); > + cgraph_edge::remove (e); > + } > + has_callback = false; > +} > + > /* Speculative call consists of an indirect edge and one or more > direct edge+ref pairs. > > @@ -1522,12 +1684,27 @@ void > cgraph_edge::redirect_callee (cgraph_node *n) > { > bool loc = callee->comdat_local_p (); > + cgraph_node *old_callee = callee; > + > /* Remove from callers list of the current callee. */ > remove_callee (); > > /* Insert to callers list of the new callee. */ > set_callee (n); > > + if (callback) > + { > + /* When redirecting a callback callee, redirect its ref as well. */ > + ipa_ref *old_ref = caller->find_reference (old_callee, call_stmt, > + lto_stmt_uid, IPA_REF_ADDR); > + gcc_checking_assert(old_ref); > + old_ref->remove_reference (); > + ipa_ref *new_ref = caller->create_reference (n, IPA_REF_ADDR, > call_stmt); > + new_ref->lto_stmt_uid = lto_stmt_uid; > + if (!old_callee->referred_to_p ()) > + old_callee->address_taken = 0; > + } > + > if (!inline_failed) > return; > if (!loc && n->comdat_local_p ()) > @@ -1644,6 +1821,27 @@ cgraph_edge::redirect_call_stmt_to_callee (cgraph_edge > *e, > || decl == e->callee->decl) > return e->call_stmt; > > + /* 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) > + { > + cgraph_edge *carrying = e->get_callback_carrying_edge (); > + if (!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 offloading 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; It might be nicer to drop the edges at the same places where we drop the attribute but it can be done later, I guess. > + int fn_idx = callback_fetch_fn_position (e, carrying); > + tree previous_arg = gimple_call_arg (e->call_stmt, 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, fn_idx, new_addr); > + return e->call_stmt; > + } > + > if (decl && ipa_saved_clone_sources) > { > tree *p = ipa_saved_clone_sources->get (e->callee); > @@ -1753,7 +1951,9 @@ cgraph_edge::redirect_call_stmt_to_callee (cgraph_edge > *e, > maybe_remove_unused_call_args (DECL_STRUCT_FUNCTION (e->caller->decl), > new_stmt); > > - e->caller->set_call_stmt_including_clones (e->call_stmt, new_stmt, false); > + /* Update callback edges if setting the carrying edge's statement, or else > + their pairing would fall apart. */ > + e->caller->set_call_stmt_including_clones (e->call_stmt, new_stmt, > e->has_callback); > > if (symtab->dump_file) > { > @@ -1945,6 +2145,17 @@ cgraph_node::remove_callers (void) > for (e = callers; e; e = f) > { > f = e->next_caller; > + /* When removing a callback-carrying edge, remove all its attached > edges > + as well. */ > + if (e->has_callback) > + { > + cgraph_edge *cbe, *next_cbe = NULL; > + for (cbe = e->first_callback_edge (); cbe; cbe = next_cbe) > + { > + next_cbe = cbe->next_callback_edge (); > + cgraph_edge::remove (cbe); > + } > + } > symtab->call_edge_removal_hooks (e); > e->remove_caller (); > symtab->free_edge (e); > @@ -2254,6 +2465,10 @@ cgraph_edge::dump_edge_flags (FILE *f) > { > if (speculative) > fprintf (f, "(speculative) "); > + if (callback) > + fprintf (f, "(callback) "); > + if (has_callback) > + fprintf (f, "(has_callback) "); > if (!inline_failed) > fprintf (f, "(inlined) "); > if (call_stmt_cannot_inline_p) > @@ -3859,6 +4074,8 @@ cgraph_node::verify_node (void) > if (gimple_has_body_p (e->caller->decl) > && !e->caller->inlined_to > && !e->speculative > + && !e->callback > + && !e->has_callback > /* Optimized out calls are redirected to __builtin_unreachable. */ > && (e->count.nonzero_p () > || ! e->callee->decl > @@ -4064,7 +4281,12 @@ cgraph_node::verify_node (void) > } > if (!e->indirect_unknown_callee) > { > - if (e->verify_corresponds_to_fndecl (decl)) > + /* Callback edges violate this assertion > + because their call statement doesn't exist, > + their associated statement belongs to the > + offloading function. */ > + if (!e->callback > + && e->verify_corresponds_to_fndecl (decl)) > { > error ("edge points to wrong declaration:"); > debug_tree (e->callee->decl); > @@ -4106,7 +4328,58 @@ cgraph_node::verify_node (void) > > for (e = callees; e; e = e->next_callee) > { > - if (!e->aux && !e->speculative) > + if (!e->callback && e->callback_id) > + { > + error ("non-callback edge has callback_id set"); > + error_found = true; > + } > + > + if (e->callback && e->has_callback) > + { > + error ("edge has both callback and has_callback set"); > + error_found = true; > + } > + > + 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->has_callback > + && !callback_is_special_cased (e->callee->decl, e->call_stmt)) > + { > + int ncallbacks = 0; > + int nfound_edges = 0; > + for (tree cb = lookup_attribute (CALLBACK_ATTR_IDENT, > DECL_ATTRIBUTES ( > + e->callee->decl)); > + cb; cb = lookup_attribute (CALLBACK_ATTR_IDENT, TREE_CHAIN > (cb)), > + ncallbacks++) > + ; > + for (cgraph_edge *cbe = callees; cbe; cbe = cbe->next_callee) > + { > + if (cbe->callback && cbe->call_stmt == e->call_stmt > + && cbe->lto_stmt_uid == e->lto_stmt_uid) > + { > + nfound_edges++; > + } > + } > + if (ncallbacks < nfound_edges) > + { > + error ("callback edge %s->%s callback edge count mismatch, " > + "expected at most %d, found %d", > + identifier_to_locale (e->caller->name ()), > + identifier_to_locale (e->callee->name ()), ncallbacks, > + nfound_edges); > + } > + } > + > + if (!e->aux && !e->speculative && !e->callback && !e->has_callback) > { > error ("edge %s->%s has no corresponding call_stmt", > identifier_to_locale (e->caller->name ()), > diff --git a/gcc/cgraph.h b/gcc/cgraph.h > index deca564a8e3..2ca30f9e229 100644 > --- a/gcc/cgraph.h > +++ b/gcc/cgraph.h > @@ -1725,12 +1725,14 @@ public: > /* Remove EDGE from the cgraph. */ > static void remove (cgraph_edge *edge); > > - /* Change field call_stmt of edge E to NEW_STMT. If UPDATE_SPECULATIVE > and E > - is any component of speculative edge, then update all components. > + /* Change field call_stmt of edge E to NEW_STMT. If UPDATE_DERIVED_EDGES > and > + E is any component of speculative edge, then update all components. > Speculations can be resolved in the process and EDGE can be removed and > - deallocated. Return the edge that now represents the call. */ > + deallocated. Return the edge that now represents the call. If > + UPDATE_DERIVED_EDGES and E is a part of a callback edge, update all > + callback edges and the callback-carrying edge. */ > static cgraph_edge *set_call_stmt (cgraph_edge *e, gcall *new_stmt, > - bool update_speculative = true); > + bool update_derived_edges = true); > > /* Redirect callee of the edge to N. The function does not update > underlying > call expression. */ > @@ -1756,6 +1758,32 @@ public: > cgraph_edge *make_speculative (cgraph_node *n2, profile_count direct_count, > unsigned int speculative_id = 0); > > + /* Create a callback edge, representing an indirect call to n2 > + passed to a function by argument. Sets has_callback flag of the > original > + edge. Both edges are attached to the same call statement. Returns > created > + callback edge. */ > + cgraph_edge *make_callback (cgraph_node *n2, unsigned int callback_hash); > + > + /* 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 > + this method is caled on. */ > + cgraph_edge *get_callback_carrying_edge (); > + > + /* Returns the first callback edge in the list of callees of the caller > node. > + Note that the edges might be in arbitrary order. Must be called on a > + callback or callback-carrying edge. */ > + cgraph_edge *first_callback_edge (); > + > + /* Given a callback edge, returns the next callback edge belonging to the > same > + callback-carrying edge. Must be called on a callback edge, not the > + callback-carrying edge. */ > + cgraph_edge *next_callback_edge (); > + > + /* When called on a callback-carrying edge, removes all of its attached > + callback edges and sets has_callback to FALSE. */ > + void purge_callback_edges (); > + > /* Speculative call consists of an indirect edge and one or more > direct edge+ref pairs. Speculative will expand to the following > sequence: > > @@ -1977,6 +2005,24 @@ public: > Optimizers may later redirect direct call to clone, so 1) and 3) > do not need to necessarily agree with destination. */ > unsigned int speculative : 1; > + /* Edges with CALLBACK flag represent indirect calls to functions passed > + to their callers by argument. This is useful in cases, where the body > + of these caller functions is not known, e. g. qsort in glibc or > + GOMP_parallel in libgomp. These edges are never made into real calls, > + but are used instead to optimize these callback functions and later > replace > + their addresses with their optimized versions. Edges with this flag set > + share their call statement with their callback-carrying edge. */ > + unsigned int callback : 1; > + /* Edges with this flag set have one or more callback edges attached. They > + share their call statements with this edge. This flag represents the > fact > + that the callee of this edge takes a function and it's parameters by > + argument and calls it at a later time. */ > + unsigned int has_callback : 1; > + /* Used to pair callback edges and the attributes that originated them > + together. Needed in order to get ipa-icf to work with callbacks. I think the sentence about ICF is outdated? In any case it seems quite unnecessary. > + Currently the index of the callback argument, retrieved from the > + attribute. */ > + unsigned int callback_id : 16; > /* Set to true when caller is a constructor or destructor of polymorphic > type. */ > unsigned in_polymorphic_cdtor : 1; > diff --git a/gcc/cgraphclones.cc b/gcc/cgraphclones.cc > index c160e8b6985..c765a33c43a 100644 > --- a/gcc/cgraphclones.cc > +++ b/gcc/cgraphclones.cc > @@ -144,6 +144,9 @@ cgraph_edge::clone (cgraph_node *n, gcall *call_stmt, > unsigned stmt_uid, > new_edge->can_throw_external = can_throw_external; > new_edge->call_stmt_cannot_inline_p = call_stmt_cannot_inline_p; > new_edge->speculative = speculative; > + new_edge->callback = callback; > + new_edge->has_callback = has_callback; > + new_edge->callback_id = callback_id; > new_edge->in_polymorphic_cdtor = in_polymorphic_cdtor; > > /* Update IPA profile. Local profiles need no updating in original. */ > diff --git a/gcc/fortran/f95-lang.cc b/gcc/fortran/f95-lang.cc > index bb4ce6d8288..496798a572f 100644 > --- a/gcc/fortran/f95-lang.cc > +++ b/gcc/fortran/f95-lang.cc > @@ -580,6 +580,8 @@ gfc_builtin_function (tree decl) > #define ATTR_COLD_NORETURN_NOTHROW_LEAF_LIST \ > (ECF_COLD | ECF_NORETURN | \ > ECF_NOTHROW | ECF_LEAF) > +#define ATTR_CALLBACK_GOMP_LIST (ECF_CB_1_2 | ATTR_NOTHROW_LIST) > +#define ATTR_CALLBACK_OACC_LIST (ECF_CB_2_4 | ATTR_NOTHROW_LIST) > #define ATTR_PURE_NOTHROW_LIST (ECF_PURE | ECF_NOTHROW) > > static void > diff --git a/gcc/ipa-cp.cc b/gcc/ipa-cp.cc > index 480cf48786c..d90dcbf1860 100644 > --- a/gcc/ipa-cp.cc > +++ b/gcc/ipa-cp.cc > @@ -131,7 +131,7 @@ along with GCC; see the file COPYING3. If not see > #include "dbgcnt.h" > #include "symtab-clones.h" > #include "gimple-range.h" > - > +#include "attr-callback.h" > > /* Allocation pools for values and their sources in ipa-cp. */ > > @@ -6202,6 +6202,72 @@ identify_dead_nodes (struct cgraph_node *node) > } > } > > +/* Removes all useless callback edges from the callgraph. Useless callback > + edges might mess up the callgraph, because they might be impossible to > + redirect and so on, leading to crashes. Their usefulness is evaluated > + through callback_edge_useful_p. */ > + > +static void > +purge_useless_callback_edges () > +{ > + if (dump_file) > + fprintf (dump_file, "\nPurging useless callback edges:\n"); > + > + cgraph_edge *e; > + cgraph_node *node; > + FOR_EACH_FUNCTION_WITH_GIMPLE_BODY (node) > + { > + for (e = node->callees; e; e = e->next_callee) > + { > + if (e->has_callback) > + { > + if (dump_file) > + fprintf (dump_file, "\tExamining callbacks of edge %s -> %s:\n", > + e->caller->dump_name (), e->callee->dump_name ()); > + if (!lookup_attribute (CALLBACK_ATTR_IDENT, > + DECL_ATTRIBUTES (e->callee->decl)) > + && !callback_is_special_cased (e->callee->decl, e->call_stmt)) > + { > + if (dump_file) > + fprintf ( > + dump_file, > + "\t\tPurging callbacks, because the offloading " > + "function no longer has any callback attributes.\n"); > + e->purge_callback_edges (); > + continue; > + } > + cgraph_edge *cbe, *next; > + for (cbe = e->first_callback_edge (); cbe; cbe = next) > + { > + next = cbe->next_callback_edge (); > + if (!callback_edge_useful_p (cbe)) > + { > + if (dump_file) > + fprintf (dump_file, > + "\t\tCallback edge %s -> %s not deemed " > + "useful, removing.\n", > + cbe->caller->dump_name (), > + cbe->callee->dump_name ()); > + cgraph_edge::remove (cbe); > + } > + else > + { > + if (dump_file) > + fprintf (dump_file, > + "\t\tKept callback edge %s -> %s " > + "because it looks useful.\n", > + cbe->caller->dump_name (), > + cbe->callee->dump_name ()); > + } > + } > + } > + } > + } > + > + if (dump_file) > + fprintf (dump_file, "\n"); > +} > + > /* The decision stage. Iterate over the topological order of call graph > nodes > TOPO and make specialized clones if deemed beneficial. */ > > @@ -6232,6 +6298,11 @@ ipcp_decision_stage (class ipa_topo_info *topo) > if (change) > identify_dead_nodes (node); > } > + > + /* Currently, the primary use of callback edges is constant propagation. > + Constant propagation is now over, so we have to remove unused callback > + edges. */ > + purge_useless_callback_edges (); > } > > /* Look up all VR and bits information that we have discovered and copy it > diff --git a/gcc/ipa-fnsummary.cc b/gcc/ipa-fnsummary.cc > index 924a54b498b..2bb5ca0fa87 100644 > --- a/gcc/ipa-fnsummary.cc > +++ b/gcc/ipa-fnsummary.cc > @@ -990,7 +990,10 @@ ipa_call_summary_t::duplicate (struct cgraph_edge *src, > info->predicate = NULL; > edge_set_predicate (dst, srcinfo->predicate); > info->param = srcinfo->param.copy (); > - if (!dst->indirect_unknown_callee && src->indirect_unknown_callee) > + if (!dst->indirect_unknown_callee && src->indirect_unknown_callee > + /* Don't subtract the size when dealing with callback pairs, since the > + edge has no real size. */ > + && !src->has_callback && !dst->callback) > { > info->call_stmt_size -= (eni_size_weights.indirect_call_cost > - eni_size_weights.call_cost); > @@ -3106,6 +3109,25 @@ analyze_function_body (struct cgraph_node *node, bool > early) > es, es3); > } > } > + > + /* If dealing with a carrying edge, copy its summary over to its > + attached edges as well. */ > + if (edge->has_callback) > + { > + cgraph_edge *cbe; > + for (cbe = edge->first_callback_edge (); cbe; > + cbe = cbe->next_callback_edge ()) > + { > + ipa_call_summary *es2 = ipa_call_summaries->get (cbe); > + es2 = ipa_call_summaries->get_create (cbe); > + ipa_call_summaries->duplicate (edge, cbe, es, es2); > + /* Unlike speculative edges, callback edges have no real > + size or time; the call doesn't exist. Reflect that in > + their summaries. */ > + es2->call_stmt_size = 0; > + es2->call_stmt_time = 0; > + } > + } > } > > /* TODO: When conditional jump or switch is known to be constant, but > diff --git a/gcc/ipa-inline-analysis.cc b/gcc/ipa-inline-analysis.cc > index c5472cb0ff0..c6ab256859d 100644 > --- a/gcc/ipa-inline-analysis.cc > +++ b/gcc/ipa-inline-analysis.cc > @@ -417,6 +417,11 @@ do_estimate_growth_1 (struct cgraph_node *node, void > *data) > { > gcc_checking_assert (e->inline_failed); > > + /* Don't count callback edges into growth, since they are never inlined > + anyway. */ > + if (e->callback) > + continue; > + > if (cgraph_inline_failed_type (e->inline_failed) == CIF_FINAL_ERROR > || !opt_for_fn (e->caller->decl, optimize)) > { > diff --git a/gcc/ipa-inline-transform.cc b/gcc/ipa-inline-transform.cc > index 9d759d218b5..d58fe5492c3 100644 > --- a/gcc/ipa-inline-transform.cc > +++ b/gcc/ipa-inline-transform.cc > @@ -780,7 +780,17 @@ inline_transform (struct cgraph_node *node) > if (!e->inline_failed) > has_inline = true; > next = e->next_callee; > - cgraph_edge::redirect_call_stmt_to_callee (e); > + if (e->has_callback) > + { > + /* Redirect callback edges when redirecting their carrying edge. */ > + cgraph_edge *cbe; > + cgraph_edge::redirect_call_stmt_to_callee (e); > + for (cbe = e->first_callback_edge (); cbe; > + cbe = cbe->next_callback_edge ()) > + cgraph_edge::redirect_call_stmt_to_callee (cbe); > + } > + else > + cgraph_edge::redirect_call_stmt_to_callee (e); > } > node->remove_all_references (); > > diff --git a/gcc/ipa-inline.cc b/gcc/ipa-inline.cc > index 0cf97a80687..b0f7394e7a4 100644 > --- a/gcc/ipa-inline.cc > +++ b/gcc/ipa-inline.cc > @@ -373,6 +373,11 @@ can_inline_edge_p (struct cgraph_edge *e, bool report, > { > gcc_checking_assert (e->inline_failed); > > + /* Never inline callback edges, since the call doesn't exist in > + reality. */ > + if (e->callback) > + return false; > + > if (cgraph_inline_failed_type (e->inline_failed) == CIF_FINAL_ERROR) > { > if (report) > diff --git a/gcc/ipa-param-manipulation.cc b/gcc/ipa-param-manipulation.cc > index 9b74fe24cc4..bcf2b820294 100644 > --- a/gcc/ipa-param-manipulation.cc > +++ b/gcc/ipa-param-manipulation.cc > @@ -50,6 +50,7 @@ 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" > > /* Actual prefixes of different newly synthetized parameters. Keep in sync > with IPA_PARAM_PREFIX_* defines. */ > @@ -308,6 +309,16 @@ drop_type_attribute_if_params_changed_p (tree name) > return false; > } > > +/* Return TRUE if the attribute should be dropped in the decl it is sitting > on > + changes. Primarily affects attributes working with the decls arguments. > */ > +static bool > +drop_decl_attribute_if_params_changed_p (tree name) > +{ > + if (is_attribute_p (CALLBACK_ATTR_IDENT, name)) > + return true; > + return false; > +} > + > /* Build and return a function type just like ORIG_TYPE but with parameter > types given in NEW_PARAM_TYPES - which can be NULL if, but only if, > ORIG_TYPE itself has NULL TREE_ARG_TYPEs. If METHOD2FUNC is true, also > make > @@ -488,11 +499,12 @@ ipa_param_adjustments::method2func_p (tree orig_type) > performing all atored modifications. TYPE_ORIGINAL_P should be true when > OLD_TYPE refers to the type before any IPA transformations, as opposed to > a > type that can be an intermediate one in between various IPA > - transformations. */ > + transformations. Set pointee of ARGS_MODIFIED (if provided) to TRUE if > the > + type's arguments were changed. */ > > tree > -ipa_param_adjustments::build_new_function_type (tree old_type, > - bool type_original_p) > +ipa_param_adjustments::build_new_function_type ( > + tree old_type, bool type_original_p, bool *args_modified /* = NULL */) > { > auto_vec<tree,16> new_param_types, *new_param_types_p; > if (prototype_p (old_type)) > @@ -518,6 +530,8 @@ ipa_param_adjustments::build_new_function_type (tree > old_type, > || get_original_index (index) != (int)index) > modified = true; > > + if (args_modified) > + *args_modified = modified; > > return build_adjusted_function_type (old_type, new_param_types_p, > method2func_p (old_type), m_skip_return, > @@ -536,10 +550,11 @@ ipa_param_adjustments::adjust_decl (tree orig_decl) > { > tree new_decl = copy_node (orig_decl); > tree orig_type = TREE_TYPE (orig_decl); > + bool args_modified = false; > if (prototype_p (orig_type) > || (m_skip_return && !VOID_TYPE_P (TREE_TYPE (orig_type)))) > { > - tree new_type = build_new_function_type (orig_type, false); > + tree new_type = build_new_function_type (orig_type, false, > &args_modified); > TREE_TYPE (new_decl) = new_type; > } > if (method2func_p (orig_type)) > @@ -556,6 +571,20 @@ ipa_param_adjustments::adjust_decl (tree orig_decl) > if (m_skip_return) > DECL_IS_MALLOC (new_decl) = 0; > > + /* If the decl's arguments changed, we might need to drop some attributes. > */ > + if (args_modified && DECL_ATTRIBUTES (new_decl)) > + { > + tree t = DECL_ATTRIBUTES (new_decl); > + tree *last = &DECL_ATTRIBUTES (new_decl); > + DECL_ATTRIBUTES (new_decl) = NULL; > + for (; t; t = TREE_CHAIN (t)) > + if (!drop_decl_attribute_if_params_changed_p (get_attribute_name (t))) > + { > + *last = copy_node (t); > + TREE_CHAIN (*last) = NULL; > + last = &TREE_CHAIN (*last); > + } > + } > return new_decl; > } > > diff --git a/gcc/ipa-param-manipulation.h b/gcc/ipa-param-manipulation.h > index 7c7661c1b4a..8121ad68526 100644 > --- a/gcc/ipa-param-manipulation.h > +++ b/gcc/ipa-param-manipulation.h > @@ -229,7 +229,8 @@ public: > /* Return if the first parameter is left intact. */ > bool first_param_intact_p (); > /* Build a function type corresponding to the modified call. */ > - tree build_new_function_type (tree old_type, bool type_is_original_p); > + tree build_new_function_type (tree old_type, bool type_is_original_p, > + bool *args_modified = NULL); > /* Build a declaration corresponding to the target of the modified call. > */ > tree adjust_decl (tree orig_decl); > /* Fill a vector marking which parameters are intact by the described > diff --git a/gcc/ipa-prop.cc b/gcc/ipa-prop.cc > index 84d4fb5db67..62911118de9 100644 > --- a/gcc/ipa-prop.cc > +++ b/gcc/ipa-prop.cc > @@ -61,6 +61,8 @@ along with GCC; see the file COPYING3. If not see > #include "value-range-storage.h" > #include "vr-values.h" > #include "lto-streamer.h" > +#include "attribs.h" > +#include "attr-callback.h" > > /* Function summary where the parameter infos are actually stored. */ > ipa_node_params_t *ipa_node_params_sum = NULL; > @@ -324,6 +326,10 @@ ipa_get_param_decl_index (class ipa_node_params *info, > tree ptree) > return ipa_get_param_decl_index_1 (info->descriptors, ptree); > } > > +static void > +ipa_duplicate_jump_function (cgraph_edge *src, cgraph_edge *dst, > + ipa_jump_func *src_jf, ipa_jump_func *dst_jf); > + > /* Populate the param_decl field in parameter DESCRIPTORS that correspond to > NODE. */ > > @@ -2416,6 +2422,19 @@ skip_a_safe_conversion_op (tree t) > return t; > } > > +/* Initializes ipa_edge_args summary of CBE given its callback-carrying edge. > + This primarily means allocating the correct amount of jump functions. */ > + > +static inline void > +init_callback_edge_summary (struct cgraph_edge *carrying, > + struct cgraph_edge *cbe) > +{ > + ipa_edge_args *carrying_args = ipa_edge_args_sum->get (carrying); > + ipa_edge_args *cb_args = ipa_edge_args_sum->get_create (cbe); > + vec_safe_grow_cleared (cb_args->jump_functions, > + carrying_args->jump_functions->length (), true); why is carrying_args->jump_functions->length () the correct number of jump functions? You need the lenght of the vector callback_get_arg_mapping would return, no? (I'm not suggesting that we construct such a vector here but I think the length needs to be computed from the attribute and not from the number of parameters the dispatching function has.) Generally, this version is a clear improvement over the previous versions and (with at least the last issue described above fixed), I'd be happy to have the cgraph and ipa bits committed. Someone else needs to approve the rest though - and Honza may want to chime in. I also hope that soon (at Cauldron?) we'll be able to come to some kind of conclusion what to do about the attribute name and behavior. We may also consider enabling the new behavior with a compiler switch (enabled by default at -O2 and higher). But that is also a detail. Thanks a lot again for working on this. Martin
