Hi,
On Mon, Sep 21 2026, Josef Melcr wrote:
> This patch makes necessary adjustments to ipa-sra to make it work on
> outlined assume bodies.
>
> Some functions try to handle internal calls, these needed to have their
> checks altered to let IFN_ASSUME calls pass through. Additionaly, code
> examining call statements or argument flows needed to have it's indices
> remapped when looking at the callee side of the edge.
>
> I also special-cased assume bodies in code which tries to make decisions
> about a function's return value. The return value of assumes is never
> used directly, but it must not be removed, as the return value is what
> is used later in vrp.
>
> The patch also includes a check in ipa_sra_preliminary_function_checks
> which prevents the body from being optimized if a callback edge is not
> pointing at it during summary generation. Even if an edge is created at
> a later stage, it's call statement is not analyzed, meaning that it's
> unusable for an optimization.
>
> gcc/ChangeLog:
>
> * ipa-sra.cc (ipa_sra_preliminary_function_checks): Exclude
> assume bodies without callback edges.
> (ptr_parm_has_nonarg_uses): Adjust internal call guard.
> (scan_function): Likewise.
> (isra_analyze_call): Special case m_return_ignored for assume bodies.
> (ipa_sra_generate_summary): Initialize callback_info sum.
> (ipa_sra_ipa_function_checks): Don't disqualify assume bodies
> when checking if a node is local.
> (param_removal_cross_scc_edge): Remap param descriptors when
> dealing with callback edges.
> (propagate_param_hints_accross_call): Likewise.
> (propagate_used_across_scc_edge): Likewise.
> (param_splitting_across_edge): Replace assert with a special
> case condition for callback edges, remap param flows for
> callback edges.
> (retval_used_p): Return true for assume bodies.
>
> Signed-off-by: Josef Melcr <[email protected]>
> ---
> gcc/ipa-sra.cc | 107 +++++++++++++++++++++++++++++++++++++++++--------
> 1 file changed, 91 insertions(+), 16 deletions(-)
>
> diff --git a/gcc/ipa-sra.cc b/gcc/ipa-sra.cc
> index 422bf367b97..a663900fc1d 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 *);
>
> @@ -689,6 +691,19 @@ ipa_sra_preliminary_function_checks (cgraph_node *node)
> return false;
> }
>
> + /* A special case. If the callback edge pointing to the assume body is not
> + created before ipa-sra analysis is run, the newly created edge won't be
> + analyzed and will thus lack the appropriate summaries. Be conservative
> and
> + skip the function entirely. We aren't missing anything, because we
> can't
> + use an unsummarized call anyway. */
> + if (node->assume_function && !node->callers)
> + {
> + if (dump_file)
> + fprintf (dump_file,
> + "Assume body without a callback edge, skipping. \n");
> + return false;
> + }
> +
Well, OK. But after you commit this, please file a bug that IPA-SRA of
assume does not work when IPA-CP is switched off (because ipa-fnsummary
analysis then happens too late). While I do not want this to prevent
the patches to go in, it is something we should aim to fix.
> return true;
> }
>
> @@ -1120,10 +1135,10 @@ ptr_parm_has_nonarg_uses (cgraph_node *node, function
> *fun, tree parm,
> }
> else if (is_gimple_call (stmt))
> {
> - gcall *call = as_a <gcall *> (stmt);
> - unsigned arg_count;
> - if (gimple_call_internal_p (call)
> - || (arg_count = gimple_call_num_args (call)) == 0)
> + gcall *call = as_a<gcall *> (stmt);
> + unsigned arg_count = gimple_call_num_args (call);
> + if (!callback_ifn_assume_call_with_edge_p (stmt)
> + && (gimple_call_internal_p (call) || arg_count == 0))
Should this have been
if ((gimple_call_internal_p (call) &&
!callback_ifn_assume_call_with_edge_p (stmt))
|| arg_count == 0)
?
(Not that it matters in practice but well...)
> {
> ret = true;
> break;
> @@ -2056,7 +2071,8 @@ 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))
> + if (gimple_call_internal_p (stmt)
> + && !callback_ifn_assume_call_with_edge_p (stmt))
> {
> call_info_p = NULL;
> ctx = ISRA_CTX_LOAD;
> @@ -2248,6 +2264,11 @@ isra_analyze_call (cgraph_edge *cs)
> BITMAP_FREE (analyzed);
> }
> }
> +
> + /* The return value of an ASSUME body is not ignored, but we cannot detect
> + this, as the function is never called directly. */
> + else if (cs->callee && cs->callee->assume_function)
> + csum->m_return_ignored = false;
> /* Don't set m_return_ignored for musttail calls. The tailc/musttail
> passes
> compare the returned value against the IPA-VRP return value range if
> it is a singleton, but if the call is changed to something which doesn't
> @@ -2778,6 +2799,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);
> @@ -3120,7 +3142,9 @@ ipa_sra_dump_all_summaries (FILE *f, bool hints)
> static bool
> ipa_sra_ipa_function_checks (cgraph_node *node)
> {
> - if (!node->can_be_local_p ())
> + /* Assume bodies cannot be local, because their address is taken, but we
> want
> + to consider them anyway. */
> + if (!node->can_be_local_p () && !node->assume_function)
> {
> if (dump_file)
> fprintf (dump_file, "Function %s disqualified because it cannot be "
> @@ -3395,11 +3419,21 @@ param_removal_cross_scc_edge (cgraph_edge *cs)
>
> for (unsigned i = 0; i < args_count; i++)
> {
> - bool unused_in_callee;
> - if (i < param_count)
> - unused_in_callee = (*to_ifs->m_parameters)[i].locally_unused;
> - else
> - unused_in_callee = false;
> + bool unused_in_callee = false;
> +
> + isra_param_desc *descriptor = NULL;
> + if (cs->callback)
> + {
> + callback_info *ci = callback_info_sum->get (cs);
> + int remapped_idx = ci->inverted_mapping[i];
> + if (remapped_idx != ARG_MAPPING_UNKNOWN_IDX)
> + descriptor = &(*to_ifs->m_parameters)[remapped_idx];
> + }
> + else if (i < param_count)
> + descriptor = &(*to_ifs->m_parameters)[i];
> +
> + if (descriptor)
> + unused_in_callee = descriptor->locally_unused;
>
> if (!unused_in_callee)
> {
> @@ -3514,7 +3548,16 @@ propagate_param_hints_accross_call (cgraph_edge *cs,
> isra_func_summary *to_ifs)
>
> if (desc->by_ref)
> {
> - isra_param_flow *ipf = &csum->m_arg_flow[i];
> + unsigned idx = i;
> + if (cs->callback)
> + {
> + callback_info *ci = callback_info_sum->get (cs);
> + int remapped = ci->arg_mapping[i];
> + if (remapped == ARG_MAPPING_UNKNOWN_IDX)
> + continue;
I have spent a lot of time thinking about the safety of the continue
statement in this and the other similar mappings below. I believe they
are safe but it is not obvious. I think that we should either simply
assert here that callee is an assume_function because it really probably
never makes sense to try an IPA-SRA a different kind of callback or we
can add a comment (to all of them) saying something like:
In the case of assume outlined functions, the only skipped argument is
the address to be called, which si fine. In other cases, is they ever
arise, processing of the call to callback dispatching functions will
unmapped arguments as necessary.
Does it make sense?
> + idx = (unsigned) remapped;
> + }
> + isra_param_flow *ipf = &csum->m_arg_flow[idx];
>
> if (!ipf->constructed_for_calls)
> desc->not_specially_constructed = true;
> @@ -3589,8 +3632,19 @@ propagate_used_across_scc_edge (cgraph_edge *cs,
> vec<cgraph_node *> *stack)
> ? vec_safe_length (to_ifs->m_parameters) : 0;
> for (unsigned i = 0; i < args_count; i++)
> {
> - if (i < param_count
> - && (*to_ifs->m_parameters)[i].locally_unused)
> + unsigned idx = i;
> +
> + if (cs->callback)
> + {
> + callback_info *ci = callback_info_sum->get (cs);
> + int remapped = ci->inverted_mapping[i];
> + if (remapped == ARG_MAPPING_UNKNOWN_IDX)
> + continue;
Likewise.
> + idx = (unsigned) remapped;
> + }
> +
> + if (idx < param_count
> + && (*to_ifs->m_parameters)[idx].locally_unused)
> continue;
>
> /* The argument is needed in the callee it, we must mark the parameter
> as
> @@ -3810,7 +3864,14 @@ param_splitting_across_edge (cgraph_edge *cs)
> = ipcp_get_transformation_summary (cs->caller);
>
> isra_call_summary *csum = call_sums->get (cs);
> - gcc_checking_assert (csum);
> + /* The summary may not exist if the edge is a callback edge created after
> SRA
> + summaries are generated (when ipa-cp is disabled for example). Bail if
> + that happens. */
> + if (!csum)
> + {
> + gcc_checking_assert (callback_ifn_assume_call_p (cs->call_stmt));
> + return false;
> + }
OK but see the comment at the top about creating a bug for this.
> unsigned args_count = csum->m_arg_flow.length ();
> isra_func_summary *to_ifs = func_sums->get (callee);
> unsigned param_count
> @@ -3826,7 +3887,16 @@ param_splitting_across_edge (cgraph_edge *cs)
> for (i = 0; (i < args_count) && (i < param_count); i++)
> {
> isra_param_desc *arg_desc = &(*to_ifs->m_parameters)[i];
> - isra_param_flow *ipf = &csum->m_arg_flow[i];
> + unsigned ipf_idx = i;
> + if (cs->callback)
> + {
> + callback_info *ci = callback_info_sum->get (cs);
> + int remapped = ci->arg_mapping[i];
> + if (remapped == ARG_MAPPING_UNKNOWN_IDX)
> + continue;
See the comment above about the continue statements in remapping
arguments.
> + ipf_idx = (unsigned) remapped;
> + }
> + isra_param_flow *ipf = &csum->m_arg_flow[ipf_idx];
>
> if (arg_desc->locally_unused)
> {
> @@ -4028,6 +4098,11 @@ retval_used_p (cgraph_node *node, void *)
> return true;
> }
>
> + /* Keep the return value, even though it's not used in the code directly,
> + because it's used by vrp later. */
> + if (node->assume_function)
> + return true;
> +
> return false;
> }
Otherwise this patch is OK
Thanks,
Martin
