* [Bug ipa/93621] [10 Regression] ICE in redirect_call_stmt_to_callee, at cgraph.c:1443 since r10-5567
[not found] <bug-93621-4@http.gcc.gnu.org/bugzilla/>
@ 2020-03-20 18:52 ` hubicka at gcc dot gnu.org
2020-03-22 12:04 ` asolokha at gmx dot com
` (16 subsequent siblings)
17 siblings, 0 replies; 18+ messages in thread
From: hubicka at gcc dot gnu.org @ 2020-03-20 18:52 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93621
Jan Hubicka <hubicka at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
CC| |mjambor at suse dot cz
--- Comment #3 from Jan Hubicka <hubicka at gcc dot gnu.org> ---
The testcase builds for me now, but this is Martin's code (apparently checking
that we did not forget to apply param adjustments)
Martin, was this fixed?
Honza
^ permalink raw reply [flat|nested] 18+ messages in thread* [Bug ipa/93621] [10 Regression] ICE in redirect_call_stmt_to_callee, at cgraph.c:1443 since r10-5567
[not found] <bug-93621-4@http.gcc.gnu.org/bugzilla/>
2020-03-20 18:52 ` [Bug ipa/93621] [10 Regression] ICE in redirect_call_stmt_to_callee, at cgraph.c:1443 since r10-5567 hubicka at gcc dot gnu.org
@ 2020-03-22 12:04 ` asolokha at gmx dot com
2020-03-22 17:57 ` marxin at gcc dot gnu.org
` (15 subsequent siblings)
17 siblings, 0 replies; 18+ messages in thread
From: asolokha at gmx dot com @ 2020-03-22 12:04 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93621
--- Comment #4 from Arseny Solokha <asolokha at gmx dot com> ---
(In reply to Jan Hubicka from comment #3)
> The testcase builds for me now
It still ICEs for me.
^ permalink raw reply [flat|nested] 18+ messages in thread* [Bug ipa/93621] [10 Regression] ICE in redirect_call_stmt_to_callee, at cgraph.c:1443 since r10-5567
[not found] <bug-93621-4@http.gcc.gnu.org/bugzilla/>
2020-03-20 18:52 ` [Bug ipa/93621] [10 Regression] ICE in redirect_call_stmt_to_callee, at cgraph.c:1443 since r10-5567 hubicka at gcc dot gnu.org
2020-03-22 12:04 ` asolokha at gmx dot com
@ 2020-03-22 17:57 ` marxin at gcc dot gnu.org
2020-03-26 14:55 ` marxin at gcc dot gnu.org
` (14 subsequent siblings)
17 siblings, 0 replies; 18+ messages in thread
From: marxin at gcc dot gnu.org @ 2020-03-22 17:57 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93621
--- Comment #5 from Martin Liška <marxin at gcc dot gnu.org> ---
Yes, I can confirm it still ICEs.
^ permalink raw reply [flat|nested] 18+ messages in thread* [Bug ipa/93621] [10 Regression] ICE in redirect_call_stmt_to_callee, at cgraph.c:1443 since r10-5567
[not found] <bug-93621-4@http.gcc.gnu.org/bugzilla/>
` (2 preceding siblings ...)
2020-03-22 17:57 ` marxin at gcc dot gnu.org
@ 2020-03-26 14:55 ` marxin at gcc dot gnu.org
2020-03-26 15:35 ` marxin at gcc dot gnu.org
` (13 subsequent siblings)
17 siblings, 0 replies; 18+ messages in thread
From: marxin at gcc dot gnu.org @ 2020-03-26 14:55 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93621
Martin Liška <marxin at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Known to fail| |10.0, 9.3.0
Known to work| |9.2.0
--- Comment #6 from Martin Liška <marxin at gcc dot gnu.org> ---
Apparently, GCC 9 branch is also affected since 9.3.0 release.
^ permalink raw reply [flat|nested] 18+ messages in thread* [Bug ipa/93621] [10 Regression] ICE in redirect_call_stmt_to_callee, at cgraph.c:1443 since r10-5567
[not found] <bug-93621-4@http.gcc.gnu.org/bugzilla/>
` (3 preceding siblings ...)
2020-03-26 14:55 ` marxin at gcc dot gnu.org
@ 2020-03-26 15:35 ` marxin at gcc dot gnu.org
2020-04-02 16:49 ` jamborm at gcc dot gnu.org
` (12 subsequent siblings)
17 siblings, 0 replies; 18+ messages in thread
From: marxin at gcc dot gnu.org @ 2020-03-26 15:35 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93621
Martin Liška <marxin at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Known to work|9.2.0 |9.3.0
Known to fail|9.3.0 |
--- Comment #7 from Martin Liška <marxin at gcc dot gnu.org> ---
No, I used bogus prebuilt binary.
^ permalink raw reply [flat|nested] 18+ messages in thread* [Bug ipa/93621] [10 Regression] ICE in redirect_call_stmt_to_callee, at cgraph.c:1443 since r10-5567
[not found] <bug-93621-4@http.gcc.gnu.org/bugzilla/>
` (4 preceding siblings ...)
2020-03-26 15:35 ` marxin at gcc dot gnu.org
@ 2020-04-02 16:49 ` jamborm at gcc dot gnu.org
2020-04-02 20:58 ` jamborm at gcc dot gnu.org
` (11 subsequent siblings)
17 siblings, 0 replies; 18+ messages in thread
From: jamborm at gcc dot gnu.org @ 2020-04-02 16:49 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93621
Martin Jambor <jamborm at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Assignee|unassigned at gcc dot gnu.org |jamborm at gcc dot gnu.org
Status|NEW |ASSIGNED
--- Comment #8 from Martin Jambor <jamborm at gcc dot gnu.org> ---
Let me have a look
^ permalink raw reply [flat|nested] 18+ messages in thread* [Bug ipa/93621] [10 Regression] ICE in redirect_call_stmt_to_callee, at cgraph.c:1443 since r10-5567
[not found] <bug-93621-4@http.gcc.gnu.org/bugzilla/>
` (5 preceding siblings ...)
2020-04-02 16:49 ` jamborm at gcc dot gnu.org
@ 2020-04-02 20:58 ` jamborm at gcc dot gnu.org
2020-04-03 7:19 ` rguenth at gcc dot gnu.org
` (10 subsequent siblings)
17 siblings, 0 replies; 18+ messages in thread
From: jamborm at gcc dot gnu.org @ 2020-04-02 20:58 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93621
--- Comment #9 from Martin Jambor <jamborm at gcc dot gnu.org> ---
(In reply to Jan Hubicka from comment #3)
> The testcase builds for me now, but this is Martin's code
that's questionable :-) Git blame points correctly to me but before
new IPA-SRA the assert used to be:
gcc_assert (!node || !node->clone.combined_args_to_skip);
and was added by Honza in 2012 (in 66a20fc2a7de).
> (apparently
> checking that we did not forget to apply param adjustments)
AFAIU no, quite the opposite, it checks that we are not going to apply
param adjustment twice to a call, which is in a way what we are about
to do.
We find ourselves looking at a call statement with parameters already
adjusted and the decl in the statement being the IPA-CP created one.
In the cgraph edge, however, the callee's decl is one created during
save_inline_function_body. Because redirect_call_stmt_to_callee
decides whether it has to do anything by comparing decls, it thinks it
has to redirect and remove params and... BOOM.
When I wrote that the call had already been adjusted that actually was
not entirely true. The call was already created that way in
expand_thunk, because it is in an expanded artificial thunk of the
IPA-CP clone.
The assumption was that because the decl would be the correct one from
the start, no additional redirection would be taking place. That
perhaps wasn't the best idea as save_inline_function_body can clearly
violate that (and in future some IPA pass might want to redirect the
edge too).
Having said that, I am not sure where to best fix this so late in the
GCC 10 development cycle.
^ permalink raw reply [flat|nested] 18+ messages in thread* [Bug ipa/93621] [10 Regression] ICE in redirect_call_stmt_to_callee, at cgraph.c:1443 since r10-5567
[not found] <bug-93621-4@http.gcc.gnu.org/bugzilla/>
` (6 preceding siblings ...)
2020-04-02 20:58 ` jamborm at gcc dot gnu.org
@ 2020-04-03 7:19 ` rguenth at gcc dot gnu.org
2020-04-03 7:31 ` rguenth at gcc dot gnu.org
` (9 subsequent siblings)
17 siblings, 0 replies; 18+ messages in thread
From: rguenth at gcc dot gnu.org @ 2020-04-03 7:19 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93621
--- Comment #10 from Richard Biener <rguenth at gcc dot gnu.org> ---
The offending commit eventually changed when/how many times
disable_insertion_hook () is called. So that maybe points to the real cause.
^ permalink raw reply [flat|nested] 18+ messages in thread* [Bug ipa/93621] [10 Regression] ICE in redirect_call_stmt_to_callee, at cgraph.c:1443 since r10-5567
[not found] <bug-93621-4@http.gcc.gnu.org/bugzilla/>
` (7 preceding siblings ...)
2020-04-03 7:19 ` rguenth at gcc dot gnu.org
@ 2020-04-03 7:31 ` rguenth at gcc dot gnu.org
2020-04-03 9:15 ` hubicka at ucw dot cz
` (8 subsequent siblings)
17 siblings, 0 replies; 18+ messages in thread
From: rguenth at gcc dot gnu.org @ 2020-04-03 7:31 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93621
--- Comment #11 from Richard Biener <rguenth at gcc dot gnu.org> ---
(In reply to Richard Biener from comment #10)
> The offending commit eventually changed when/how many times
> disable_insertion_hook () is called. So that maybe points to the real cause.
Eventually that just triggered a different transform though.
^ permalink raw reply [flat|nested] 18+ messages in thread* [Bug ipa/93621] [10 Regression] ICE in redirect_call_stmt_to_callee, at cgraph.c:1443 since r10-5567
[not found] <bug-93621-4@http.gcc.gnu.org/bugzilla/>
` (8 preceding siblings ...)
2020-04-03 7:31 ` rguenth at gcc dot gnu.org
@ 2020-04-03 9:15 ` hubicka at ucw dot cz
2020-04-03 10:18 ` jamborm at gcc dot gnu.org
` (7 subsequent siblings)
17 siblings, 0 replies; 18+ messages in thread
From: hubicka at ucw dot cz @ 2020-04-03 9:15 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93621
--- Comment #12 from Jan Hubicka <hubicka at ucw dot cz> ---
> Having said that, I am not sure where to best fix this so late in the
> GCC 10 development cycle.
So the problem is that thunk is expanded on the adjusted decl but we
still keep the adjustments and later fail to apply them?
I guess we have two options:
1) force thunk expansion to happen on original decls (before cloning)
so the body ends up being same as for ordinary function
2) remove the adjustments after expansion - this should IMO work
under the assumption that optimization passes don't insert
non-trivial code into the thunk before they expand the thunk (i.e.
if you want to adjust it in ipa-sra you will want to first produce
the thunk and then do adjustement)
It seems to me that 2 should be not that hard to implement
Does that make sense?
Honza
^ permalink raw reply [flat|nested] 18+ messages in thread* [Bug ipa/93621] [10 Regression] ICE in redirect_call_stmt_to_callee, at cgraph.c:1443 since r10-5567
[not found] <bug-93621-4@http.gcc.gnu.org/bugzilla/>
` (9 preceding siblings ...)
2020-04-03 9:15 ` hubicka at ucw dot cz
@ 2020-04-03 10:18 ` jamborm at gcc dot gnu.org
2020-04-03 11:05 ` jamborm at gcc dot gnu.org
` (6 subsequent siblings)
17 siblings, 0 replies; 18+ messages in thread
From: jamborm at gcc dot gnu.org @ 2020-04-03 10:18 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93621
--- Comment #13 from Martin Jambor <jamborm at gcc dot gnu.org> ---
(In reply to Jan Hubicka from comment #12)
> > Having said that, I am not sure where to best fix this so late in the
> > GCC 10 development cycle.
>
> So the problem is that thunk is expanded on the adjusted decl but we
> still keep the adjustments and later fail to apply them?
>
> I guess we have two options:
> 1) force thunk expansion to happen on original decls (before cloning)
> so the body ends up being same as for ordinary function
I was thinking about this too. I will try to look into expand_thunk
whether I can leave the call statement mostly alone (apart from the
thunk transform itself, of course).
> 2) remove the adjustments after expansion - this should IMO work
> under the assumption that optimization passes don't insert
> non-trivial code into the thunk before they expand the thunk (i.e.
> if you want to adjust it in ipa-sra you will want to first produce
> the thunk and then do adjustement)
> It seems to me that 2 should be not that hard to implement
> Does that make sense?
Unfortunately I don't think so. The adjustment is attached to the
callee (just like in the past the skip_args bitmap was - and we're
only skipping arguments in the testcase), so you cannot just remove it
in one caller. Or am I missing something?
^ permalink raw reply [flat|nested] 18+ messages in thread* [Bug ipa/93621] [10 Regression] ICE in redirect_call_stmt_to_callee, at cgraph.c:1443 since r10-5567
[not found] <bug-93621-4@http.gcc.gnu.org/bugzilla/>
` (10 preceding siblings ...)
2020-04-03 10:18 ` jamborm at gcc dot gnu.org
@ 2020-04-03 11:05 ` jamborm at gcc dot gnu.org
2020-04-03 13:22 ` jamborm at gcc dot gnu.org
` (5 subsequent siblings)
17 siblings, 0 replies; 18+ messages in thread
From: jamborm at gcc dot gnu.org @ 2020-04-03 11:05 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93621
--- Comment #14 from Martin Jambor <jamborm at gcc dot gnu.org> ---
Actually, we should be able to simply skip applying adjustments, if
e->caller->former_thunk_p(). I'm playing with a patch.
^ permalink raw reply [flat|nested] 18+ messages in thread* [Bug ipa/93621] [10 Regression] ICE in redirect_call_stmt_to_callee, at cgraph.c:1443 since r10-5567
[not found] <bug-93621-4@http.gcc.gnu.org/bugzilla/>
` (11 preceding siblings ...)
2020-04-03 11:05 ` jamborm at gcc dot gnu.org
@ 2020-04-03 13:22 ` jamborm at gcc dot gnu.org
2020-04-03 17:00 ` jamborm at gcc dot gnu.org
` (4 subsequent siblings)
17 siblings, 0 replies; 18+ messages in thread
From: jamborm at gcc dot gnu.org @ 2020-04-03 13:22 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93621
--- Comment #15 from Martin Jambor <jamborm at gcc dot gnu.org> ---
It turns out that no, recursive inlining will happily put an adjusted and not
yet adjusted call into the same function which was formerly a thunk.
^ permalink raw reply [flat|nested] 18+ messages in thread* [Bug ipa/93621] [10 Regression] ICE in redirect_call_stmt_to_callee, at cgraph.c:1443 since r10-5567
[not found] <bug-93621-4@http.gcc.gnu.org/bugzilla/>
` (12 preceding siblings ...)
2020-04-03 13:22 ` jamborm at gcc dot gnu.org
@ 2020-04-03 17:00 ` jamborm at gcc dot gnu.org
2020-04-06 8:47 ` jamborm at gcc dot gnu.org
` (3 subsequent siblings)
17 siblings, 0 replies; 18+ messages in thread
From: jamborm at gcc dot gnu.org @ 2020-04-03 17:00 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93621
--- Comment #16 from Martin Jambor <jamborm at gcc dot gnu.org> ---
The following workaround works for the testcase but would need to be
generalized for a chain of former_decl_of's to be universal, I'm afraid:
diff --git a/gcc/cgraph.c b/gcc/cgraph.c
index 6b780f80eb3..241b996151a 100644
--- a/gcc/cgraph.c
+++ b/gcc/cgraph.c
@@ -1467,7 +1467,8 @@ cgraph_edge::redirect_call_stmt_to_callee (cgraph_edge
*e)
if (e->indirect_unknown_callee
- || decl == e->callee->decl)
+ || decl == e->callee->decl
+ || decl == e->callee->former_clone_of)
return e->call_stmt;
if (flag_checking && decl)
diff --git a/gcc/ipa-inline-transform.c b/gcc/ipa-inline-transform.c
index eed992d314d..a6675768552 100644
--- a/gcc/ipa-inline-transform.c
+++ b/gcc/ipa-inline-transform.c
@@ -588,6 +588,7 @@ save_inline_function_body (struct cgraph_node *node)
first_clone->next_sibling_clone = NULL;
gcc_assert (!first_clone->prev_sibling_clone);
}
+ first_clone->former_clone_of = node->decl;
first_clone->clone_of = NULL;
/* Now node in question has no clones. */
^ permalink raw reply [flat|nested] 18+ messages in thread* [Bug ipa/93621] [10 Regression] ICE in redirect_call_stmt_to_callee, at cgraph.c:1443 since r10-5567
[not found] <bug-93621-4@http.gcc.gnu.org/bugzilla/>
` (13 preceding siblings ...)
2020-04-03 17:00 ` jamborm at gcc dot gnu.org
@ 2020-04-06 8:47 ` jamborm at gcc dot gnu.org
2020-04-14 17:31 ` jamborm at gcc dot gnu.org
` (2 subsequent siblings)
17 siblings, 0 replies; 18+ messages in thread
From: jamborm at gcc dot gnu.org @ 2020-04-06 8:47 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93621
--- Comment #17 from Martin Jambor <jamborm at gcc dot gnu.org> ---
Created attachment 48208
--> https://gcc.gnu.org/bugzilla/attachment.cgi?id=48208&action=edit
WIP patch
This is the current version of my patch to fix this. I think that at
least for the purposes of JIT I need to find a place to deallocate the
new summary - but that can only happen after all inlining is done.
Then I'll add that, re-base and submit it to the mailing list.
^ permalink raw reply [flat|nested] 18+ messages in thread* [Bug ipa/93621] [10 Regression] ICE in redirect_call_stmt_to_callee, at cgraph.c:1443 since r10-5567
[not found] <bug-93621-4@http.gcc.gnu.org/bugzilla/>
` (14 preceding siblings ...)
2020-04-06 8:47 ` jamborm at gcc dot gnu.org
@ 2020-04-14 17:31 ` jamborm at gcc dot gnu.org
2020-04-16 17:21 ` cvs-commit at gcc dot gnu.org
2020-04-16 17:27 ` jamborm at gcc dot gnu.org
17 siblings, 0 replies; 18+ messages in thread
From: jamborm at gcc dot gnu.org @ 2020-04-14 17:31 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93621
--- Comment #18 from Martin Jambor <jamborm at gcc dot gnu.org> ---
I posted a patch to fix this for review to the mailing list:
https://gcc.gnu.org/pipermail/gcc-patches/2020-April/543659.html
^ permalink raw reply [flat|nested] 18+ messages in thread* [Bug ipa/93621] [10 Regression] ICE in redirect_call_stmt_to_callee, at cgraph.c:1443 since r10-5567
[not found] <bug-93621-4@http.gcc.gnu.org/bugzilla/>
` (15 preceding siblings ...)
2020-04-14 17:31 ` jamborm at gcc dot gnu.org
@ 2020-04-16 17:21 ` cvs-commit at gcc dot gnu.org
2020-04-16 17:27 ` jamborm at gcc dot gnu.org
17 siblings, 0 replies; 18+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2020-04-16 17:21 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93621
--- Comment #19 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Martin Jambor <jamborm@gcc.gnu.org>:
https://gcc.gnu.org/g:7123347c8b44571811c4b58506b06fb09969bccb
commit r10-7760-g7123347c8b44571811c4b58506b06fb09969bccb
Author: Martin Jambor <mjambor@suse.cz>
Date: Thu Apr 16 19:21:02 2020 +0200
ipa: Make call redirection detect already adjusted calls (PR 93621)
PR 93621 testcase makes redirect_call_stmt_to_callee wrongly assume
that a call statement needs redirecting but then rightly fails an
assert ensuring the call statement parameters have not already been
adjusted because they were already created adjusted as part of thunk
expansion.
The test fails because the decl in the call call statement is
different than the decl of the callee, because the latter was created
in save_inline_function_body. This patch adds a way to link these two
and detect the situation in redirect_call_stmt_to_callee.
2020-04-16 Martin Jambor <mjambor@suse.cz>
PR ipa/93621
* ipa-inline.h (ipa_saved_clone_sources): Declare.
* ipa-inline-transform.c (ipa_saved_clone_sources): New variable.
(save_inline_function_body): Link the new body holder with the
previous one.
* cgraph.c: Include ipa-inline.h.
(cgraph_edge::redirect_call_stmt_to_callee): Try to find the decl
from
the statement in ipa_saved_clone_sources.
* cgraphunit.c: Include ipa-inline.h.
(expand_all_functions): Free ipa_saved_clone_sources.
testsuite/
* g++.dg/ipa/pr93621.C: New test.
^ permalink raw reply [flat|nested] 18+ messages in thread* [Bug ipa/93621] [10 Regression] ICE in redirect_call_stmt_to_callee, at cgraph.c:1443 since r10-5567
[not found] <bug-93621-4@http.gcc.gnu.org/bugzilla/>
` (16 preceding siblings ...)
2020-04-16 17:21 ` cvs-commit at gcc dot gnu.org
@ 2020-04-16 17:27 ` jamborm at gcc dot gnu.org
17 siblings, 0 replies; 18+ messages in thread
From: jamborm at gcc dot gnu.org @ 2020-04-16 17:27 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93621
Martin Jambor <jamborm at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Status|ASSIGNED |RESOLVED
Resolution|--- |FIXED
--- Comment #20 from Martin Jambor <jamborm at gcc dot gnu.org> ---
Fixed for GCC 10, see the review email thread for caveats/future plans about
this.
^ permalink raw reply [flat|nested] 18+ messages in thread