* [PATCH] ipa: Fix ICE when speculating calls from inlined functions (PR 102388)
@ 2021-09-23 17:26 Martin Jambor
2021-09-27 12:36 ` Martin Liška
0 siblings, 1 reply; 2+ messages in thread
From: Martin Jambor @ 2021-09-23 17:26 UTC (permalink / raw)
To: GCC Patches; +Cc: Jan Hubicka
Hi,
The code handling various cases which lead to call graph edge
duplication (in order to update reference descriptions used to track
and remove no-longer needed references) has missed one important case.
When edge duplication is an effect of creating a speculative edge for
an indirect edge which carries a constant jump function which had been
created from a pass-through function when the edge caller has was
inlined into one of its callers, the reference description attached to
the function describes an edge higher up in the "inlined" clone tree
and so even the new speculative edge will. Therefore we should not
try to duplicate the reference description itself but rather just bump
the refcount of the existing one.
Creating a small testcase unfortunately is not very straightforward, I
have not attempted to trigger just the right speculation after inlining.
Bootstrapped and tested on an x86_64-linux. OK for trunk?
Thanks,
Martin
gcc/ChangeLog:
2021-09-22 Martin Jambor <mjambor@suse.cz>
PR ipa/102388
* ipa-prop.c (ipa_edge_args_sum_t::duplicate): Also handle the
case when the source reference description corresponds to a
referance taken in a function src->caller is inlined to.
---
gcc/ipa-prop.c | 40 +++++++++++++++++++++++++++-------------
1 file changed, 27 insertions(+), 13 deletions(-)
diff --git a/gcc/ipa-prop.c b/gcc/ipa-prop.c
index 1c69d9766c5..443f21ce61b 100644
--- a/gcc/ipa-prop.c
+++ b/gcc/ipa-prop.c
@@ -4428,19 +4428,33 @@ ipa_edge_args_sum_t::duplicate (cgraph_edge *src, cgraph_edge *dst,
dst_jf->value.constant.rdesc = NULL;
else if (src->caller == dst->caller)
{
- struct ipa_ref *ref;
- symtab_node *n = symtab_node_for_jfunc (src_jf);
- gcc_checking_assert (n);
- ref = src->caller->find_reference (n, src->call_stmt,
- src->lto_stmt_uid);
- gcc_checking_assert (ref);
- dst->caller->clone_reference (ref, ref->stmt);
-
- struct ipa_cst_ref_desc *dst_rdesc = ipa_refdesc_pool.allocate ();
- dst_rdesc->cs = dst;
- dst_rdesc->refcount = src_rdesc->refcount;
- dst_rdesc->next_duplicate = NULL;
- dst_jf->value.constant.rdesc = dst_rdesc;
+ /* Creation of a speculative edge. If the source edge is the one
+ grabbing a reference, we must create a new (duplicate)
+ reference description. Otherwise they refer to the same
+ description corresponding to a reference taken in a function
+ src->caller is inlined to. In that case we just must
+ increment the refcount. */
+ if (src_rdesc->cs == src)
+ {
+ symtab_node *n = symtab_node_for_jfunc (src_jf);
+ gcc_checking_assert (n);
+ ipa_ref *ref
+ = src->caller->find_reference (n, src->call_stmt,
+ src->lto_stmt_uid);
+ gcc_checking_assert (ref);
+ dst->caller->clone_reference (ref, ref->stmt);
+
+ ipa_cst_ref_desc *dst_rdesc = ipa_refdesc_pool.allocate ();
+ dst_rdesc->cs = dst;
+ dst_rdesc->refcount = src_rdesc->refcount;
+ dst_rdesc->next_duplicate = NULL;
+ dst_jf->value.constant.rdesc = dst_rdesc;
+ }
+ else
+ {
+ src_rdesc->refcount++;
+ dst_jf->value.constant.rdesc = src_rdesc;
+ }
}
else if (src_rdesc->cs == src)
{
--
2.33.0
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [PATCH] ipa: Fix ICE when speculating calls from inlined functions (PR 102388)
2021-09-23 17:26 [PATCH] ipa: Fix ICE when speculating calls from inlined functions (PR 102388) Martin Jambor
@ 2021-09-27 12:36 ` Martin Liška
0 siblings, 0 replies; 2+ messages in thread
From: Martin Liška @ 2021-09-27 12:36 UTC (permalink / raw)
To: Martin Jambor, GCC Patches; +Cc: Jan Hubicka
On 9/23/21 19:26, Martin Jambor wrote:
> Hi,
>
> The code handling various cases which lead to call graph edge
> duplication (in order to update reference descriptions used to track
> and remove no-longer needed references) has missed one important case.
>
> When edge duplication is an effect of creating a speculative edge for
> an indirect edge which carries a constant jump function which had been
> created from a pass-through function when the edge caller has was
> inlined into one of its callers, the reference description attached to
> the function describes an edge higher up in the "inlined" clone tree
> and so even the new speculative edge will. Therefore we should not
> try to duplicate the reference description itself but rather just bump
> the refcount of the existing one.
>
> Creating a small testcase unfortunately is not very straightforward, I
> have not attempted to trigger just the right speculation after inlining.
>
> Bootstrapped and tested on an x86_64-linux. OK for trunk?
Approved.
Martin
>
> Thanks,
>
> Martin
>
>
> gcc/ChangeLog:
>
> 2021-09-22 Martin Jambor <mjambor@suse.cz>
>
> PR ipa/102388
> * ipa-prop.c (ipa_edge_args_sum_t::duplicate): Also handle the
> case when the source reference description corresponds to a
> referance taken in a function src->caller is inlined to.
> ---
> gcc/ipa-prop.c | 40 +++++++++++++++++++++++++++-------------
> 1 file changed, 27 insertions(+), 13 deletions(-)
>
> diff --git a/gcc/ipa-prop.c b/gcc/ipa-prop.c
> index 1c69d9766c5..443f21ce61b 100644
> --- a/gcc/ipa-prop.c
> +++ b/gcc/ipa-prop.c
> @@ -4428,19 +4428,33 @@ ipa_edge_args_sum_t::duplicate (cgraph_edge *src, cgraph_edge *dst,
> dst_jf->value.constant.rdesc = NULL;
> else if (src->caller == dst->caller)
> {
> - struct ipa_ref *ref;
> - symtab_node *n = symtab_node_for_jfunc (src_jf);
> - gcc_checking_assert (n);
> - ref = src->caller->find_reference (n, src->call_stmt,
> - src->lto_stmt_uid);
> - gcc_checking_assert (ref);
> - dst->caller->clone_reference (ref, ref->stmt);
> -
> - struct ipa_cst_ref_desc *dst_rdesc = ipa_refdesc_pool.allocate ();
> - dst_rdesc->cs = dst;
> - dst_rdesc->refcount = src_rdesc->refcount;
> - dst_rdesc->next_duplicate = NULL;
> - dst_jf->value.constant.rdesc = dst_rdesc;
> + /* Creation of a speculative edge. If the source edge is the one
> + grabbing a reference, we must create a new (duplicate)
> + reference description. Otherwise they refer to the same
> + description corresponding to a reference taken in a function
> + src->caller is inlined to. In that case we just must
> + increment the refcount. */
> + if (src_rdesc->cs == src)
> + {
> + symtab_node *n = symtab_node_for_jfunc (src_jf);
> + gcc_checking_assert (n);
> + ipa_ref *ref
> + = src->caller->find_reference (n, src->call_stmt,
> + src->lto_stmt_uid);
> + gcc_checking_assert (ref);
> + dst->caller->clone_reference (ref, ref->stmt);
> +
> + ipa_cst_ref_desc *dst_rdesc = ipa_refdesc_pool.allocate ();
> + dst_rdesc->cs = dst;
> + dst_rdesc->refcount = src_rdesc->refcount;
> + dst_rdesc->next_duplicate = NULL;
> + dst_jf->value.constant.rdesc = dst_rdesc;
> + }
> + else
> + {
> + src_rdesc->refcount++;
> + dst_jf->value.constant.rdesc = src_rdesc;
> + }
> }
> else if (src_rdesc->cs == src)
> {
>
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2021-09-27 12:36 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-23 17:26 [PATCH] ipa: Fix ICE when speculating calls from inlined functions (PR 102388) Martin Jambor
2021-09-27 12:36 ` Martin Liška
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).