* [Bug ipa/60973] Invalid propagation of a tail call in devirt pass
2014-04-26 8:27 [Bug tree-optimization/60973] New: Invalid propagation of a tail call in copyrename2 pass ubizjak at gmail dot com
@ 2014-04-28 9:49 ` rguenth at gcc dot gnu.org
2014-05-06 11:28 ` hubicka at gcc dot gnu.org
` (7 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: rguenth at gcc dot gnu.org @ 2014-04-28 9:49 UTC (permalink / raw)
To: gcc-bugs
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).
^ permalink raw reply [flat|nested] 10+ messages in thread
* [Bug ipa/60973] Invalid propagation of a tail call in devirt pass
2014-04-26 8:27 [Bug tree-optimization/60973] New: Invalid propagation of a tail call in copyrename2 pass ubizjak at gmail dot com
2014-04-28 9:49 ` [Bug ipa/60973] Invalid propagation of a tail call in devirt pass rguenth at gcc dot gnu.org
@ 2014-05-06 11:28 ` hubicka at gcc dot gnu.org
2014-05-07 18:10 ` ubizjak at gmail dot com
` (6 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: hubicka at gcc dot gnu.org @ 2014-05-06 11:28 UTC (permalink / raw)
To: gcc-bugs
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.
^ permalink raw reply [flat|nested] 10+ messages in thread
* [Bug ipa/60973] Invalid propagation of a tail call in devirt pass
2014-04-26 8:27 [Bug tree-optimization/60973] New: Invalid propagation of a tail call in copyrename2 pass ubizjak at gmail dot com
2014-04-28 9:49 ` [Bug ipa/60973] Invalid propagation of a tail call in devirt pass rguenth at gcc dot gnu.org
2014-05-06 11:28 ` hubicka at gcc dot gnu.org
@ 2014-05-07 18:10 ` ubizjak at gmail dot com
2014-05-09 10:56 ` rguenth at gcc dot gnu.org
` (5 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: ubizjak at gmail dot com @ 2014-05-07 18:10 UTC (permalink / raw)
To: gcc-bugs
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.
>From gcc-bugs-return-450882-listarch-gcc-bugs=gcc.gnu.org@gcc.gnu.org Wed May 07 18:13:24 2014
Return-Path: <gcc-bugs-return-450882-listarch-gcc-bugs=gcc.gnu.org@gcc.gnu.org>
Delivered-To: listarch-gcc-bugs@gcc.gnu.org
Received: (qmail 12762 invoked by alias); 7 May 2014 18:13:24 -0000
Mailing-List: contact gcc-bugs-help@gcc.gnu.org; run by ezmlm
Precedence: bulk
List-Id: <gcc-bugs.gcc.gnu.org>
List-Archive: <http://gcc.gnu.org/ml/gcc-bugs/>
List-Post: <mailto:gcc-bugs@gcc.gnu.org>
List-Help: <mailto:gcc-bugs-help@gcc.gnu.org>
Sender: gcc-bugs-owner@gcc.gnu.org
Delivered-To: mailing list gcc-bugs@gcc.gnu.org
Received: (qmail 12702 invoked by uid 48); 7 May 2014 18:13:20 -0000
From: "hjl.tools at gmail dot com" <gcc-bugzilla@gcc.gnu.org>
To: gcc-bugs@gcc.gnu.org
Subject: [Bug c++/61082] [x86-64 Itanium ABI] g++ uses wrong return location for class with head padding
Date: Wed, 07 May 2014 18:13:00 -0000
X-Bugzilla-Reason: CC
X-Bugzilla-Type: changed
X-Bugzilla-Watch-Reason: None
X-Bugzilla-Product: gcc
X-Bugzilla-Component: c++
X-Bugzilla-Version: 4.9.0
X-Bugzilla-Keywords: ABI, wrong-code
X-Bugzilla-Severity: major
X-Bugzilla-Who: hjl.tools at gmail dot com
X-Bugzilla-Status: UNCONFIRMED
X-Bugzilla-Priority: P3
X-Bugzilla-Assigned-To: unassigned at gcc dot gnu.org
X-Bugzilla-Target-Milestone: ---
X-Bugzilla-Flags:
X-Bugzilla-Changed-Fields:
Message-ID: <bug-61082-4-HWoFZqVxqP@http.gcc.gnu.org/bugzilla/>
In-Reply-To: <bug-61082-4@http.gcc.gnu.org/bugzilla/>
References: <bug-61082-4@http.gcc.gnu.org/bugzilla/>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 7bit
X-Bugzilla-URL: http://gcc.gnu.org/bugzilla/
Auto-Submitted: auto-generated
MIME-Version: 1.0
X-SW-Source: 2014-05/txt/msg00574.txt.bz2
Content-length: 896
http://gcc.gnu.org/bugzilla/show_bug.cgi?ida082
--- Comment #14 from H.J. Lu <hjl.tools at gmail dot com> ---
(In reply to David Greene from comment #13)
> I see that 3.2.3 4 (b) is talking about considering adjacent fields in an
> eightbyte. Is the intent to classify each eightbyte in an aggregate and
> then consider each eightbyte separately for assigning argument and return
> registers?
That is correct.
> The post-merger cleanup described in 5 appears to handle passing in memory
> in that it considers all of the fields in the aggregate together.
>
> Given the above, if a field crosses an eightbyte either it is larger than an
> eightbyte either it is unaligned which forces the whole argument to memory
> or its eightbytes are classified separately in a recursive manner.
>
> Does this sound correct? If so, I think gcc is correct in what it's doing.
I think GCC is correct.
^ permalink raw reply [flat|nested] 10+ messages in thread
* [Bug ipa/60973] Invalid propagation of a tail call in devirt pass
2014-04-26 8:27 [Bug tree-optimization/60973] New: Invalid propagation of a tail call in copyrename2 pass ubizjak at gmail dot com
` (2 preceding siblings ...)
2014-05-07 18:10 ` ubizjak at gmail dot com
@ 2014-05-09 10:56 ` rguenth at gcc dot gnu.org
2014-05-09 14:27 ` hubicka at ucw dot cz
` (4 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: rguenth at gcc dot gnu.org @ 2014-05-09 10:56 UTC (permalink / raw)
To: gcc-bugs
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?
^ permalink raw reply [flat|nested] 10+ messages in thread
* [Bug ipa/60973] Invalid propagation of a tail call in devirt pass
2014-04-26 8:27 [Bug tree-optimization/60973] New: Invalid propagation of a tail call in copyrename2 pass ubizjak at gmail dot com
` (3 preceding siblings ...)
2014-05-09 10:56 ` rguenth at gcc dot gnu.org
@ 2014-05-09 14:27 ` hubicka at ucw dot cz
2014-05-13 7:57 ` rguenth at gcc dot gnu.org
` (3 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: hubicka at ucw dot cz @ 2014-05-09 14:27 UTC (permalink / raw)
To: gcc-bugs
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.
^ permalink raw reply [flat|nested] 10+ messages in thread
* [Bug ipa/60973] Invalid propagation of a tail call in devirt pass
2014-04-26 8:27 [Bug tree-optimization/60973] New: Invalid propagation of a tail call in copyrename2 pass ubizjak at gmail dot com
` (4 preceding siblings ...)
2014-05-09 14:27 ` hubicka at ucw dot cz
@ 2014-05-13 7:57 ` rguenth at gcc dot gnu.org
2014-05-13 11:05 ` rguenth at gcc dot gnu.org
` (2 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: rguenth at gcc dot gnu.org @ 2014-05-13 7:57 UTC (permalink / raw)
To: gcc-bugs
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.
^ permalink raw reply [flat|nested] 10+ messages in thread
* [Bug ipa/60973] Invalid propagation of a tail call in devirt pass
2014-04-26 8:27 [Bug tree-optimization/60973] New: Invalid propagation of a tail call in copyrename2 pass ubizjak at gmail dot com
` (5 preceding siblings ...)
2014-05-13 7:57 ` rguenth at gcc dot gnu.org
@ 2014-05-13 11:05 ` rguenth at gcc dot gnu.org
2014-05-13 11:06 ` rguenth at gcc dot gnu.org
2014-05-13 11:06 ` rguenth at gcc dot gnu.org
8 siblings, 0 replies; 10+ messages in thread
From: rguenth at gcc dot gnu.org @ 2014-05-13 11:05 UTC (permalink / raw)
To: gcc-bugs
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=210364&root=gcc&view=rev
Log:
2014-05-13 Richard Biener <rguenther@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
^ permalink raw reply [flat|nested] 10+ messages in thread
* [Bug ipa/60973] Invalid propagation of a tail call in devirt pass
2014-04-26 8:27 [Bug tree-optimization/60973] New: Invalid propagation of a tail call in copyrename2 pass ubizjak at gmail dot com
` (6 preceding siblings ...)
2014-05-13 11:05 ` rguenth at gcc dot gnu.org
@ 2014-05-13 11:06 ` rguenth at gcc dot gnu.org
2014-05-13 11:06 ` rguenth at gcc dot gnu.org
8 siblings, 0 replies; 10+ messages in thread
From: rguenth at gcc dot gnu.org @ 2014-05-13 11:06 UTC (permalink / raw)
To: gcc-bugs
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=210365&root=gcc&view=rev
Log:
2014-05-13 Richard Biener <rguenther@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
^ permalink raw reply [flat|nested] 10+ messages in thread
* [Bug ipa/60973] Invalid propagation of a tail call in devirt pass
2014-04-26 8:27 [Bug tree-optimization/60973] New: Invalid propagation of a tail call in copyrename2 pass ubizjak at gmail dot com
` (7 preceding siblings ...)
2014-05-13 11:06 ` rguenth at gcc dot gnu.org
@ 2014-05-13 11:06 ` rguenth at gcc dot gnu.org
8 siblings, 0 replies; 10+ messages in thread
From: rguenth at gcc dot gnu.org @ 2014-05-13 11:06 UTC (permalink / raw)
To: gcc-bugs
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.
^ permalink raw reply [flat|nested] 10+ messages in thread