Hi,
ping, please.
Martin
On Thu, Jan 21 2021, Martin Jambor wrote:
> Hi,
>
> in the PR 98078 testcase, speculative call-graph edges which were
> created by IPA-CP are confirmed during inlining but
> cgraph_edge::set_call_stmt does not take it very well.
>
> The function enters the update_speculative branch and updates the edges
> in the speculation bundle separately (by a recursive call), but when it
> processes the first direct edge, most of the bundle actually ceases to
> exist because it is devirtualized. It nevertheless goes on to attempt
> to update the indirect edge (that has just been removed), which
> surprisingly gets as far as adding the edge to the call_site_hash, the
> same devirtualized edge for the second time, and that triggers an
> assert.
>
> Fixed by this patch which makes the function aware that it is about to
> resolve a speculation and do so instead of updating components of
> speculation. Also, it does so before dealing with the hash because
> the speculation resolution code needs the hash to point to the first
> speculative direct edge and also cleans the hash up by calling
> update_call_stmt_hash_for_removing_direct_edge.
>
> I don't have a testcase, at least not yet, the one in BZ does not link
> when it does not ICE. I can try to find some time to make it link it is
> deemed very important.
>
> Bootstrapped and tested on x86_64-linux, also profile-LTO-bootstrapped
> on the same system. OK for trunk? What about gcc10, where we cannot
> trigger it but I suppose the bug is there?
>
> Thanks,
>
> Martin
>
>
> gcc/ChangeLog:
>
> 2021-01-20 Martin Jambor <mjam...@suse.cz>
>
> PR ipa/98078
> * cgraph.c (cgraph_edge::set_call_stmt): Do not update all
> corresponding speculative edges if we are about to resolve
> sepculation. Make edge direct (and so resolve speculations) before
> removing it from call_site_hash.
> (cgraph_edge::make_direct): Relax the initial assert to allow calling
> the function on speculative direct edges.
> ---
> gcc/cgraph.c | 37 ++++++++++++++++++++++---------------
> 1 file changed, 22 insertions(+), 15 deletions(-)
>
> diff --git a/gcc/cgraph.c b/gcc/cgraph.c
> index db038306e19..80140757d16 100644
> --- a/gcc/cgraph.c
> +++ b/gcc/cgraph.c
> @@ -789,9 +789,22 @@ cgraph_edge::set_call_stmt (cgraph_edge *e, gcall
> *new_stmt,
> {
> tree decl;
>
> + cgraph_node *new_direct_callee = NULL;
> + if ((e->indirect_unknown_callee || e->speculative)
> + && (decl = gimple_call_fndecl (new_stmt)))
> + {
> + /* Constant propagation and especially inlining can turn an indirect
> call
> + into a direct one. */
> + new_direct_callee = cgraph_node::get (decl);
> + gcc_checking_assert (new_direct_callee);
> + }
> +
> /* Speculative edges has three component, update all of them
> when asked to. */
> - if (update_speculative && e->speculative)
> + if (update_speculative && 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)
> {
> cgraph_edge *direct, *indirect, *next;
> ipa_ref *ref;
> @@ -821,6 +834,9 @@ cgraph_edge::set_call_stmt (cgraph_edge *e, gcall
> *new_stmt,
> return e_indirect ? indirect : direct;
> }
>
> + if (new_direct_callee)
> + e = make_direct (e, new_direct_callee);
> +
> /* Only direct speculative edges go to call_site_hash. */
> if (e->caller->call_site_hash
> && (!e->speculative || !e->indirect_unknown_callee)
> @@ -831,16 +847,6 @@ cgraph_edge::set_call_stmt (cgraph_edge *e, gcall
> *new_stmt,
> (e->call_stmt, cgraph_edge_hasher::hash (e->call_stmt));
>
> e->call_stmt = new_stmt;
> - if (e->indirect_unknown_callee
> - && (decl = gimple_call_fndecl (new_stmt)))
> - {
> - /* Constant propagation (and possibly also inlining?) can turn an
> - indirect call into a direct one. */
> - cgraph_node *new_callee = cgraph_node::get (decl);
> -
> - gcc_checking_assert (new_callee);
> - e = make_direct (e, new_callee);
> - }
>
> function *fun = DECL_STRUCT_FUNCTION (e->caller->decl);
> e->can_throw_external = stmt_can_throw_external (fun, new_stmt);
> @@ -1279,14 +1285,15 @@ cgraph_edge::speculative_call_for_target (cgraph_node
> *target)
> return NULL;
> }
>
> -/* Make an indirect edge with an unknown callee an ordinary edge leading to
> - CALLEE. Speculations can be resolved in the process and EDGE can be
> removed
> - and deallocated. Return the edge that now represents the call. */
> +/* Make an indirect or speculative EDGE with an unknown callee an ordinary
> edge
> + leading to CALLEE. Speculations can be resolved in the process and EDGE
> can
> + be removed and deallocated. Return the edge that now represents the
> + call. */
>
> cgraph_edge *
> cgraph_edge::make_direct (cgraph_edge *edge, cgraph_node *callee)
> {
> - gcc_assert (edge->indirect_unknown_callee);
> + gcc_assert (edge->indirect_unknown_callee || edge->speculative);
>
> /* If we are redirecting speculative call, make it non-speculative. */
> if (edge->speculative)
> --
> 2.29.2
