Hi,
On Wed, Jun 17 2026, Josef Melcr wrote:
> Last version:
> (https://patchwork.sourceware.org/project/gcc/patch/[email protected]/)
>
> Hi,
> I've made the following changes since the last time I posted this patch:
> - Implemented Martin's suggestions: removed the id field, replaced it
> with a get_id method and a comment.
> - Fixed the formatting a little.
> - Me and Martin discussed removing the arg_mapping vector, but
> ultimately decided to keep it for ease of use.
>
> Best regards,
> Josef
>
> gcc/ChangeLog:
>
> * Makefile.in: Add callback-info.o to OBJS.
> * attr-callback.cc (callback_fetch_attr_by_edge): Use fn_idx from
> the summary.
> (callback_get_arg_mapping): Replaced by
> callback_get_arg_mapping_from_attr and the summary field.
> (callback_get_arg_mapping_from_attr): New function, parses the
> attr and returns the computed argument mapping.
> (callback_fetch_fn_position): Delete, obsoleted by the summary.
> (callback_edge_useful_p): Check for the redirected flag in the
> summary instead of the cgraph_node fields.
> * attr-callback.h (enum callback_position): Add
> CB_UNKNOWN_POS_IDX for better readability.
> (callback_get_arg_mapping): Delete.
> (callback_fetch_fn_position): Delete.
> (callback_get_arg_mapping_from_attr): Add decl.
> * cgraph.cc (symbol_table::create_edge): Remove callback_id
> initializer.
> (cgraph_edge::make_callback): Likewise.
> (cgraph_edge::redirect_callee): Set the redirected flag when
> redirecting a callback edge.
> (cgraph_edge::redirect_call_stmt_to_callee): Use the summary.
> (cgraph_node::verify_node): Remove callback_id checks.
> (cgraph_cc_finalize): Free the summaries.
> * cgraph.h: Remove callback_id from cgraph_edge.
> * cgraphclones.cc (cgraph_edge::clone): Remove callback_id.
> * ipa-cp.cc (ipcp_driver): Initialize the summary sum.
> * ipa-prop.cc (init_callback_edge_summary): Initialize the summary.
> (ipa_compute_jump_functions_for_edge): Remove callback_id, use
> the summary.
> (ipa_analyze_node): Initialize the summary sum.
> (ipa_register_cgraph_hooks): Likewise.
> * lto-cgraph.cc (lto_output_edge): Remove callback_id, stream
> out the summary.
> (output_symtab): Initialize the summary sum.
> (input_edge): Remove callback_id, stream in the summary.
> (input_cgraph_1): Initialize the summary sum.
> * callback-info.cc: New file.
> * callback-info.h: New file.
>
This patch needs rebasing - at least the function
init_callback_edge_summary you modify here no longer exists. But after
that I think it is basically OK to be pushed to master. The only thing
that I dislike is...
[...]
> diff --git a/gcc/attr-callback.h b/gcc/attr-callback.h
> index b2c1c3c09c5..8f649006b8c 100644
> --- a/gcc/attr-callback.h
> +++ b/gcc/attr-callback.h
> @@ -25,7 +25,10 @@ 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
> + CB_UNKNOWN_POS = 0,
> + /* Value representing an unknown argument position in the arg_mapping
> + vector. */
> + CB_UNKNOWN_POS_IDX = -1
> };
...that within this single enum we define identifiers that are to be
used in quite different contexts (IIUC CB_UNKNOWN_POS is being looked at
in the attribute itself, whereas CB_UNKNOWN_POS_IDX is something we put
into parameter remapping vectors). I don't know (or particularly care)
whether we want a different enum with just one item or a define or just
resort to using -1. If we want an identifier, lets change the CB_
prefix too.
Also, the new constant is not used when the remapping is constructed, if
we keep it, we should use it there too.
Thanks!
Martin
