[Bug lto/57084] New: 483. xalancbmk run fails with -O2 -flto for i686

[Bug lto/57084] New: 483. xalancbmk run fails with -O2 -flto for i686

[izamyatin at gmail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=57084

             Bug #: 57084
           Summary: 483. xalancbmk run fails with -O2 -flto for i686
    Classification: Unclassified
           Product: gcc
           Version: 4.9.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: lto
        AssignedTo: unassigned@gcc.gnu.org
        ReportedBy: izamyatin@gmail.com


Happens after r198088

More precise flags:
-m32 -flto -O2 -msse2 -mfpmath=sse -ffast-math -march=corei7

[Bug lto/57084] 483. xalancbmk run fails with -O2 -flto for i686

[jamborm at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=57084

Martin Jambor <jamborm at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |ASSIGNED
   Last reconfirmed|                            |2013-04-29
                 CC|mjambor at suse dot cz      |jamborm at gcc dot gnu.org
         AssignedTo|unassigned at gcc dot       |jamborm at gcc dot gnu.org
                   |gnu.org                     |
     Ever Confirmed|0                           |1

--- Comment #1 from Martin Jambor <jamborm at gcc dot gnu.org> 2013-04-29 16:34:16 UTC ---
I have reproduced the problem and will investigate, LTOishness of this
problem is rather interesting.

[Bug lto/57084] 483. xalancbmk run fails with -O2 -flto for i686

[jamborm at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=57084

Martin Jambor <jamborm at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |hubicka at gcc dot gnu.org

--- Comment #2 from Martin Jambor <jamborm at gcc dot gnu.org> 2013-05-02 15:46:35 UTC ---
OK, after I verified and re-verified that the call target determined
by devirtualization is actually correct, I have noticed that the call
graph of the target is an inlined node.  I'm not sure what effects
exactly that has but the following (untested) patch helps while still
performing the devirtualization (and the assembly around the calls is
slightly different).

I think that the final patch will look a bit different because a) the
new code is basically copied from ipa_make_edge_direct_to_target an
thus we'd better commonize it somehow (and I'd like to discuss with
Honza at least the name of the new function) and b) I see no reason
why the constructor-based devirtualization method in PRE cannot have
the same problem and so we'll probably want to handle it there instead
(Honza said on IRC something about a patch he already had but I am not
sure what he meant).

Another interesting point is that at least in the 64 bit non-LTO
build, these polymorphic calls are devirtualized during inlining.  I'm
going to look at why we do not do it in 32 bit LTO one (so far I do
not know which one is the problem).


Index: src/gcc/ipa-prop.c
===================================================================
--- src.orig/gcc/ipa-prop.c
+++ src/gcc/ipa-prop.c
@@ -1978,6 +1978,7 @@ ipa_analyze_node (struct cgraph_node *no
 tree
 ipa_intraprocedural_devirtualization (gimple call)
 {
+  struct cgraph_node *callee;
   tree binfo, token, fndecl;
   struct ipa_jump_func jfunc;
   tree otr = gimple_call_fn (call);
@@ -1993,6 +1994,18 @@ ipa_intraprocedural_devirtualization (gi
   token = OBJ_TYPE_REF_TOKEN (otr);
   fndecl = gimple_get_virt_method_for_binfo (tree_low_cst (token, 1),
                          binfo);
+  if (!fndecl)
+    return NULL_TREE;
+
+  callee = cgraph_get_node (fndecl);
+  if (!callee || callee->global.inlined_to)
+    {
+      if (!canonicalize_constructor_val (fndecl, NULL)
+      || !TREE_PUBLIC (fndecl))
+    return NULL_TREE;
+      cgraph_get_create_real_symbol_node (fndecl);
+    }
+
   return fndecl;
 }

[Bug lto/57084] 483. xalancbmk run fails with -O2 -flto for i686

[jamborm at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=57084

--- Comment #4 from Martin Jambor <jamborm at gcc dot gnu.org> 2013-05-03 13:45:28 UTC ---
(In reply to comment #3)
> > @@ -1993,6 +1994,18 @@ ipa_intraprocedural_devirtualization (gi
> >    token = OBJ_TYPE_REF_TOKEN (otr);
> >    fndecl = gimple_get_virt_method_for_binfo (tree_low_cst (token, 1),
> >                           binfo);
> > +  if (!fndecl)
> > +    return NULL_TREE;
> > +
> > +  callee = cgraph_get_node (fndecl);
> > +  if (!callee || callee->global.inlined_to)
> > +    {
> > +      if (!canonicalize_constructor_val (fndecl, NULL)
> fndecl = canonicalize_constructor_val (fndecl, NULL) 
> Why do you need TREE_PUBLIC check?

Well, as I wrote in the comment above, I basically copied the code
what you added to ipa_make_edge_direct_to_target in February this
year.  The comment there says that "In the case of static functions we
are out of luck, since we already removed its body."  BTW, we also
only check canonicalize_constructor_val return value for non-NULL-ness
in that function.

Anyway, this was just a proof of concept patch.  I suppose that we'll
want to put this functionality to a separate function and call it from
both ipa_make_edge_direct_to_target and eliminate_bb (in
tree-ssa-pre.c).  I wonder what name to give it to it though,
especially given that cgraph_get_create_real_symbol_node already exits
(but unlike what is required IIUC cannot return NULL).
cgraph_make_fndecl_callable?

[Bug lto/57084] 483. xalancbmk run fails with -O2 -flto for i686

[jamborm at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=57084

Martin Jambor <jamborm at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                URL|                            |http://gcc.gnu.org/ml/gcc-p
                   |                            |atches/2013-05/msg00366.htm
                   |                            |l

--- Comment #5 from Martin Jambor <jamborm at gcc dot gnu.org> 2013-05-07 17:56:41 UTC ---
Proposed fix submitted to the mailing list:

http://gcc.gnu.org/ml/gcc-patches/2013-05/msg00366.html

[Bug lto/57084] 483. xalancbmk run fails with -O2 -flto for i686

[jamborm at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=57084

Martin Jambor <jamborm at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
         Resolution|---                         |FIXED

--- Comment #6 from Martin Jambor <jamborm at gcc dot gnu.org> ---
Fixed with:

Author: jamborm
Date: Thu May  9 16:32:17 2013
New Revision: 198743

URL: http://gcc.gnu.org/viewcvs?rev=198743&root=gcc&view=rev
Log:
2013-05-09  Martin Jambor  <mjambor@suse.cz>

    PR lto/57084
    * gimple-fold.c (canonicalize_constructor_val): Call
    cgraph_get_create_real_symbol_node instead of cgraph_get_create_node.


Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/gimple-fold.c

[Bug lto/57084] 483. xalancbmk run fails with -O2 -flto for i686

[jamborm at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=57084

--- Comment #7 from Martin Jambor <jamborm at gcc dot gnu.org> ---
Author: jamborm
Date: Tue Oct 29 14:32:13 2013
New Revision: 204163

URL: http://gcc.gnu.org/viewcvs?rev=204163&root=gcc&view=rev
Log:
2013-10-29  Martin Jambor  <mjambor@suse.cz>

    PR middle-end/58789
    Backport from mainline
        2013-05-09  Martin Jambor  <mjambor@suse.cz>

    PR lto/57084
    * gimple-fold.c (canonicalize_constructor_val): Call
    cgraph_get_create_real_symbol_node instead of cgraph_get_create_node.

    Backport from mainline
    2013-03-16  Jan Hubicka  <jh@suse.cz>

    * cgraph.h (cgraph_get_create_real_symbol_node): Declare.
    * cgraph.c (cgraph_get_create_real_symbol_node): New function.
    * cgrpahbuild.c: Use cgraph_get_create_real_symbol_node instead
    of cgraph_get_create_node.
    * ipa-prop.c (ipa_make_edge_direct_to_target): Likewise.


Modified:
    branches/gcc-4_8-branch/gcc/ChangeLog
    branches/gcc-4_8-branch/gcc/cgraph.c
    branches/gcc-4_8-branch/gcc/cgraph.h
    branches/gcc-4_8-branch/gcc/cgraphbuild.c
    branches/gcc-4_8-branch/gcc/gimple-fold.c
    branches/gcc-4_8-branch/gcc/ipa-prop.c