[Bug ipa/60973] Invalid propagation of a tail call in devirt pass
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=60973 Richard Biener rguenth at gcc dot gnu.org changed: What|Removed |Added Status|UNCONFIRMED |ASSIGNED Last reconfirmed||2014-05-13 Assignee|unassigned at gcc dot gnu.org |rguenth at gcc dot gnu.org Ever confirmed|0 |1 --- Comment #6 from Richard Biener rguenth at gcc dot gnu.org --- Ok.
[Bug ipa/60973] Invalid propagation of a tail call in devirt pass
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=60973 --- Comment #7 from Richard Biener rguenth at gcc dot gnu.org --- Author: rguenth Date: Tue May 13 11:04:44 2014 New Revision: 210364 URL: http://gcc.gnu.org/viewcvs?rev=210364root=gccview=rev Log: 2014-05-13 Richard Biener rguent...@suse.de PR ipa/60973 * tree-inline.c (remap_gimple_stmt): Clear tail call flag, it needs revisiting whether the call still may be tail-called. Modified: trunk/gcc/ChangeLog trunk/gcc/tree-inline.c
[Bug ipa/60973] Invalid propagation of a tail call in devirt pass
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=60973 --- Comment #9 from Richard Biener rguenth at gcc dot gnu.org --- Author: rguenth Date: Tue May 13 11:06:00 2014 New Revision: 210365 URL: http://gcc.gnu.org/viewcvs?rev=210365root=gccview=rev Log: 2014-05-13 Richard Biener rguent...@suse.de PR ipa/60973 * tree-inline.c (remap_gimple_stmt): Clear tail call flag, it needs revisiting whether the call still may be tail-called. Modified: branches/gcc-4_9-branch/gcc/ChangeLog branches/gcc-4_9-branch/gcc/tree-inline.c
[Bug ipa/60973] Invalid propagation of a tail call in devirt pass
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=60973 Richard Biener rguenth at gcc dot gnu.org changed: What|Removed |Added Status|ASSIGNED|RESOLVED Resolution|--- |FIXED Target Milestone|--- |4.9.1 --- Comment #8 from Richard Biener rguenth at gcc dot gnu.org --- Fixed for 4.9.1.
[Bug ipa/60973] Invalid propagation of a tail call in devirt pass
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=60973 Richard Biener rguenth at gcc dot gnu.org changed: What|Removed |Added CC||rguenth at gcc dot gnu.org --- Comment #4 from Richard Biener rguenth at gcc dot gnu.org --- Before tunks we never bothered to compute [tailcall] before inlining completed, but now explicitely setting the flag for thunks (and not letting it be computed - why wouldn't that work?) breaks this. So not setting the flag explicitely in expand_thunk looks like a better fix to me?
[Bug ipa/60973] Invalid propagation of a tail call in devirt pass
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=60973 --- Comment #5 from Jan Hubicka hubicka at ucw dot cz --- Before tunks we never bothered to compute [tailcall] before inlining completed, but now explicitely setting the flag for thunks (and not letting it be computed - why wouldn't that work?) breaks this. So not setting the flag explicitely in expand_thunk looks like a better fix to me? We always had this explicit set of tailcall in thunk expansion code - originally in C++ frontend and at early LTO times I just literaly moved it to cgraphunit. This patch http://gcc.gnu.org/ml/gcc-patches/2013-09/msg01035.html makes it possible that tunks are inlined since we lower them to gimple bodies early and it is why things breaks now as inliner does not expect it. My initial reaction (written in previously comment) was also that tailcall should discover the flags themself and we could avoid setting them in the thunk expansion. Sadly I think it is not quite the case; tailcall is very conservative and I believe it will give up in cases where thunks are possible. Also it is not run at -O0 and for thunks we want the tailcall to happen since it only improves debugging exprience and saves codegen time... So I would probably say we should fix that in tree-inline as your patch propose.
[Bug ipa/60973] Invalid propagation of a tail call in devirt pass
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=60973
--- Comment #3 from Uroš Bizjak ubizjak at gmail dot com ---
(In reply to Richard Biener from comment #1)
Index: gcc/tree-inline.c
I have checked this patch on my target, where it fixes the runtime problem. The
optimized tree dump results in:
int main(int, char**) (int argc, char * * argv)
{
int retval.0;
struct C c;
int _3;
bb 2:
C::C (c);
_3 = get_input ();
retval.0_8 = C::foo (c, _3);
if (retval.0_8 != 4)
goto bb 3;
else
goto bb 4;
bb 3:
abort ();
bb 4:
c ={v} {CLOBBER};
return 0;
}
which expands to the correct RTL sequence.
[Bug ipa/60973] Invalid propagation of a tail call in devirt pass
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=60973
--- Comment #2 from Jan Hubicka hubicka at gcc dot gnu.org ---
I wonder if the tailcall flag can't be reliably set by tree-tailcall, but i
suppose we want tailcall in thunks even at -O0.
The patch seems fine to me.
Note that this is not ipa-devirt issue, just semi-latent bug, so this probably
should be backported to all release branches.
The effect of CALL_FROM_THUNK is the following:
/* If we're compiling a thunk, pass through invisible references
instead of making a copy. */
if (call_from_thunk_p
|| (callee_copies
!TREE_ADDRESSABLE (type)
(base = get_base_address (args[i].tree_value))
TREE_CODE (base) != SSA_NAME
(!DECL_P (base) || MEM_P (DECL_RTL (base)
{
I am not really confident about this code, but I would say that if we inlined
the thunk itself, we still should have the copy around - at least I do not see
any code in tree-inline that would copy propagate this through.
So probably we do not need to make the another copy? This may be true in
general for all parameters passed by invisible references that was passed to
caller same way and caller does not modify them or use after the call. This
seems just special case of this optimization.
[Bug ipa/60973] Invalid propagation of a tail call in devirt pass
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=60973
Richard Biener rguenth at gcc dot gnu.org changed:
What|Removed |Added
Keywords||wrong-code
Target||h8300-elf
CC||hubicka at gcc dot gnu.org,
||jamborm at gcc dot gnu.org
Component|tree-optimization |ipa
Summary|Invalid propagation of a|Invalid propagation of a
|tail call in copyrename2|tail call in devirt pass
|pass|
--- Comment #1 from Richard Biener rguenth at gcc dot gnu.org ---
It's not copyrename (that's just the first dump you see it) but inlining.
Inlining probably needs to clear [tailcall] from all inlined stmts unless
it wants to prove the tailcall is still possible.
Thus,
Index: gcc/tree-inline.c
===
--- gcc/tree-inline.c (revision 209782)
+++ gcc/tree-inline.c (working copy)
@@ -1485,6 +1489,11 @@ remap_gimple_stmt (gimple stmt, copy_bod
/* Create a new deep copy of the statement. */
copy = gimple_copy (stmt);
+ /* Clear flags that need revisiting. */
+ if (is_gimple_call (copy)
+ gimple_call_tail_p (copy))
+ gimple_call_set_tail (copy, false);
+
/* Remap the region numbers for __builtin_eh_{pointer,filter},
RESX and EH_DISPATCH. */
if (id-eh_map)
not sure if GF_CALL_FROM_THUNK needs similar handling.
The bug is probably not h8300-elf specific (but usually tailcall expansion
fails as it re-checks the validity of the transform - and IIRC that is
required, so it may even be a h8300-elf backend bug).
