[Bug middle-end/48674] New: [4.7 Regression] FAIL: g++.dg/torture/pr48661.C

[Bug middle-end/48674] New: [4.7 Regression] FAIL: g++.dg/torture/pr48661.C

[hjl.tools at gmail dot com]

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

           Summary: [4.7 Regression] FAIL: g++.dg/torture/pr48661.C
           Product: gcc
           Version: 4.7.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: middle-end
        AssignedTo: unassigned@gcc.gnu.org
        ReportedBy: hjl.tools@gmail.com


On Linux/ia32, revision 172677 gave

FAIL: g++.dg/torture/pr48661.C  -O1  (internal compiler error)
FAIL: g++.dg/torture/pr48661.C  -O1  (test for excess errors)
FAIL: g++.dg/torture/pr48661.C  -O2  (internal compiler error)
FAIL: g++.dg/torture/pr48661.C  -O2  (test for excess errors)
FAIL: g++.dg/torture/pr48661.C  -O2 -flto  (internal compiler error)
FAIL: g++.dg/torture/pr48661.C  -O2 -flto  (test for excess errors)
FAIL: g++.dg/torture/pr48661.C  -O2 -flto -flto-partition=none  (internal
compiler error)
FAIL: g++.dg/torture/pr48661.C  -O2 -flto -flto-partition=none  (test for
excess errors)
FAIL: g++.dg/torture/pr48661.C  -O3 -fomit-frame-pointer  (internal compiler
error)
FAIL: g++.dg/torture/pr48661.C  -O3 -fomit-frame-pointer  (test for excess
errors)
FAIL: g++.dg/torture/pr48661.C  -O3 -g  (internal compiler error)
FAIL: g++.dg/torture/pr48661.C  -O3 -g  (test for excess errors)
FAIL: g++.dg/torture/pr48661.C  -Os  (internal compiler error)
FAIL: g++.dg/torture/pr48661.C  -Os  (test for excess errors)


Revision 172670 is OK.

[Bug middle-end/48674] [4.7 Regression] FAIL: g++.dg/torture/pr48661.C

[hjl.tools at gmail dot com]

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

H.J. Lu <hjl.tools at gmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jakub at redhat dot com

--- Comment #1 from H.J. Lu <hjl.tools at gmail dot com> 2011-04-18 23:34:24 UTC ---
I got

661.C:77:1: error: a call to thunk improperly represented in the call graph:^M
# VUSE <.MEM_11>^M
D.2279_3 = D::_ZTv0_n12_NK1D1mEv (&q.D.2119);^M
^M
void test()/26(14) @0xf7716854 (asm: _Z4testv) availability:available analyzed
needed reachable body externally_visible finalized^M
  called by: int main()/28 (1.00 per call) (can throw external) ^M
  calls: void bar(int)/1 (1.00 per call) void bar(int)/1 (1.00 per call)
virtual int D::m() const/15 (1.00 per call) D::D(const A&)/14 (1.00 per call)
^M
  References: ^M
  Refering this function: ^M
  has 3 outgoing edges for indirect calls.^M
/export/gnu/import/svn/gcc-test-ia32/src-trunk/gcc/testsuite/g++.dg/torture/pr48661.C:77:1:
internal compiler error: verify_cgraph_node failed^M
Please submit a full bug report,^M
with preprocessed source if appropriate.^M
See <http://gcc.gnu.org/bugs.html> for instructions.^M
compiler exited with status 1

[Bug middle-end/48674] [4.7 Regression] FAIL: g++.dg/torture/pr48661.C

[jakub at gcc dot gnu.org]

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

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2011.04.19 06:42:54
                 CC|                            |jakub at gcc dot gnu.org
     Ever Confirmed|0                           |1

--- Comment #2 from Jakub Jelinek <jakub at gcc dot gnu.org> 2011-04-19 06:42:54 UTC ---
The ICE is caused by http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=172644
(as I said in my mail, when writing the patch it worked fine on the trunk,
while when bootstrapping/regtesting it later on it already failed that way).

[Bug middle-end/48674] [4.7 Regression] FAIL: g++.dg/torture/pr48661.C

[rguenth at gcc dot gnu.org]

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

Richard Guenther <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |4.7.0

[Bug middle-end/48674] [4.7 Regression] FAIL: g++.dg/torture/pr48661.C

[ro at gcc dot gnu.org]

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

Rainer Orth <ro at gcc dot gnu.org> changed:

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

--- Comment #3 from Rainer Orth <ro at gcc dot gnu.org> 2011-05-02 12:51:58 UTC ---
Richard, could you please have a look?  This failure is causing considerable
amounts of testsuite noise.

Thanks.
  Rainer

[Bug middle-end/48674] [4.7 Regression] FAIL: g++.dg/torture/pr48661.C

[rguenth at gcc dot gnu.org]

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

--- Comment #4 from Richard Guenther <rguenth at gcc dot gnu.org> 2011-05-02 13:23:41 UTC ---
Well, I think it is a mistake in the cgraph infrastructure and I hoped that
Honza and Martin would work on this.  At least I see no way to tell
whether a function-decl is a thunk or not (and I don't see a reason to
not expose direct calls to thunks, as iterated previously).

Yes, the noise is annoying but I can't do anything but paper over the issue
by either reverting the patch, reverting the testcase or trying to be
clever with not exposing direct calls to thunks (which will trivially
break with LTO and then, as now, continue to create wrong-code or ICE).

So, Honza?  Can you make this a priority?

[Bug middle-end/48674] [4.7 Regression] FAIL: g++.dg/torture/pr48661.C

[ro at CeBiTec dot Uni-Bielefeld.DE]

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

--- Comment #6 from ro at CeBiTec dot Uni-Bielefeld.DE <ro at CeBiTec dot Uni-Bielefeld.DE> 2011-05-02 13:47:47 UTC ---
> --- Comment #4 from Richard Guenther <rguenth at gcc dot gnu.org> 2011-05-02 13:23:41 UTC ---

> Yes, the noise is annoying but I can't do anything but paper over the issue
> by either reverting the patch, reverting the testcase or trying to be
> clever with not exposing direct calls to thunks (which will trivially
> break with LTO and then, as now, continue to create wrong-code or ICE).

Fine with me: at this point, I'd like to avoid xfailing/skipping
testcases to make sure they aren't forgotten.  I'd just like to make
sure that the amount of noise doesn't get too big.

    Rainer

[Bug middle-end/48674] [4.7 Regression] FAIL: g++.dg/torture/pr48661.C

[rguenth at gcc dot gnu.org]

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

--- Comment #5 from Richard Guenther <rguenth at gcc dot gnu.org> 2011-05-02 13:30:33 UTC ---
2011-01-03  Martin Jambor  <mjambor@suse.cz>

       * cgraphunit.c (verify_cgraph_node): Verify there is no direct call to
       a thunk.

also "exposed" this.

[Bug middle-end/48674] [4.7 Regression] FAIL: g++.dg/torture/pr48661.C

[rguenth at gcc dot gnu.org]

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

--- Comment #7 from Richard Guenther <rguenth at gcc dot gnu.org> 2011-05-02 14:27:34 UTC ---
Patch:

Index: gcc/cgraph.c
===================================================================
--- gcc/cgraph.c        (revision 173249)
+++ gcc/cgraph.c        (working copy)
@@ -613,13 +613,15 @@ cgraph_add_thunk (struct cgraph_node *de
   node = cgraph_same_body_alias_1 (decl_node, alias, decl);
   gcc_assert (node);
   gcc_checking_assert (!virtual_offset
-                      || tree_int_cst_equal (virtual_offset,
-                                             size_int (virtual_value)));
+                      || double_int_equal_p
+                           (tree_to_double_int (virtual_offset),
+                            shwi_to_double_int (virtual_value)));
   node->thunk.fixed_offset = fixed_offset;
   node->thunk.this_adjusting = this_adjusting;
   node->thunk.virtual_value = virtual_value;
   node->thunk.virtual_offset_p = virtual_offset != NULL;
   node->thunk.alias = real_alias;
+  node->same_body_alias = false;
   node->thunk.thunk_p = true;
   return node;
 }
Index: gcc/cgraphunit.c
===================================================================
--- gcc/cgraphunit.c    (revision 173249)
+++ gcc/cgraphunit.c    (working copy)
@@ -678,16 +678,6 @@ verify_cgraph_node (struct cgraph_node *
                                debug_tree (decl);
                                error_found = true;
                              }
-                           else if (decl
-                                    && (n = cgraph_get_node_or_alias (decl))
-                                    && (n->same_body_alias
-                                        && n->thunk.thunk_p))
-                             {
-                               error ("a call to thunk improperly represented
"
-                                      "in the call graph:");
-                               cgraph_debug_gimple_stmt (this_cfun, stmt);
-                               error_found = true;
-                             }
                          }
                        else if (decl)
                          {

[Bug middle-end/48674] [4.7 Regression] FAIL: g++.dg/torture/pr48661.C

[hubicka at gcc dot gnu.org]

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

--- Comment #9 from Jan Hubicka <hubicka at gcc dot gnu.org> 2011-05-02 14:55:38 UTC ---
Just to add, the other alternatives discussed were

 I) Avoid direct calls to thunks at early optimization time.
    This has problem with fact that C++ FE doesn't really tell us about thunks
    in other units (though I think it has the knowledge) and thus this breaks
    with LTO or if someone does direct call by hand (using aliasing ASM name
that
    does not work at the moment in non-LTO compilation)
II) Invent Gimple representation of thunks
    To do so, one would need to invent way represeting variadic thunks
    Also either we would need to trash existing ASM thunk output machinery
    and re-implement in a way so RTL optimizers produce same code expanding
    the new construct.
    Still we would need to handle same special cases for 1) and 2) from 
    my initial list of problems
III)Thunks are really meant to be alternative entry points to the functions.
    We should eventually bite the bullet and implement them.  
IV) Attach the info to edges.  This makes things more transparent to WHOPR,
    and most of ipa passes: i.e. if you don't care about it, you can pretend
    thunks does not exist. It is also more consistent with III since
alternative
    entry points should probably be represented as different kind of edges into
    single callgraph node (=function).

    It however seems to make less sense to people and makes it easier to 
    introduce wrong code bugs by ignoring the info on edges wehere you should
not.

[Bug middle-end/48674] [4.7 Regression] FAIL: g++.dg/torture/pr48661.C

[hubicka at gcc dot gnu.org]

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

Jan Hubicka <hubicka at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
         AssignedTo|unassigned at gcc dot       |hubicka at gcc dot gnu.org
                   |gnu.org                     |

--- Comment #8 from Jan Hubicka <hubicka at gcc dot gnu.org> 2011-05-02 14:47:49 UTC ---
As discussed on IRC, the above patch is not really good idea.  The consequence
of it is that the cgraph edges and IPA references will point to thunk nodes.
Those are not really nodes of callgraph and a idea of graph where edge targets
are not neccesarily its nodes is slipperly.

I have patches to turn thunks to appear as function in cgraph that are somewhat
ugly, but apparently less ugly from all alternatives we was able to come with.
My original plan was to first push cleanups to callgraph, but because the whole
issue got escalated by the real world testcase, I will try to separate them
from rest of symtab code I have pending and push them to mainline soon.

The weakest point of this approach is that thunks differ from usual functions
in several ways:

1) they don't have body representable in gimple (this is the case of variadic
thunks). This require changes at places where IPA passes want to analyze
function bodies:

   a) inliner must to know how to inline thunk (or to not inline it as I do as
      a temporary hack)
   b) inline-analysis needs to special case thunk function and guess their size
   c) ipa-prop needs to compute proper jump function for THIS
   d) rest of IPA passes (pure-const/reference) are more or less trivial, 
      just need to avoid idea of looking into the function body since there 
      is nothing relevant for them anyway.

2) they don't appear as function at ABI level and must be output in chunk with
the function they are attached.
3) our RTL output machinery is somewhat convoluted.

the second two needs following changes I know of

   a) unreachable function removal must be weakened a bit on thunk functions
   b) WHOPR partitioning must know that it always have to associate thunk 
      into every partition the thunked function comes to
   c) cgraph_assemble_function 

My code bootstraps, but does not really work for i.e. Mozilla, so it is
entirely possible more side corners needs to be handled, too ;(

Honza

[Bug middle-end/48674] [4.7 Regression] FAIL: g++.dg/torture/pr48661.C

[rguenth at gcc dot gnu.org]

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

--- Comment #10 from Richard Guenther <rguenth at gcc dot gnu.org> 2011-05-02 14:56:47 UTC ---
(In reply to comment #8)
> As discussed on IRC, the above patch is not really good idea.  The consequence
> of it is that the cgraph edges and IPA references will point to thunk nodes.
> Those are not really nodes of callgraph and a idea of graph where edge targets
> are not neccesarily its nodes is slipperly.
> 
> I have patches to turn thunks to appear as function in cgraph that are somewhat
> ugly, but apparently less ugly from all alternatives we was able to come with.
> My original plan was to first push cleanups to callgraph, but because the whole
> issue got escalated by the real world testcase, I will try to separate them
> from rest of symtab code I have pending and push them to mainline soon.
> 
> The weakest point of this approach is that thunks differ from usual functions
> in several ways:
> 
> 1) they don't have body representable in gimple (this is the case of variadic
> thunks). This require changes at places where IPA passes want to analyze
> function bodies:
> 
>    a) inliner must to know how to inline thunk (or to not inline it as I do as
>       a temporary hack)
>    b) inline-analysis needs to special case thunk function and guess their size
>    c) ipa-prop needs to compute proper jump function for THIS
>    d) rest of IPA passes (pure-const/reference) are more or less trivial, 
>       just need to avoid idea of looking into the function body since there 
>       is nothing relevant for them anyway.

Most of them is moot if we just accept that direct calls to thunks are
a barrier for IPA optimizations (in the case they do not have a gimple
body).  We could represent variadic thunks in gimple using
__builtin_va_arg_pack
and friends if we accept the fact that we are never going to expand the
gimple body from its stmts but instead from the usual mechanism of expanding
such thunks.  Whatever transformations we applied to the tunk body doesn't
matter (they were just useless).

> 2) they don't appear as function at ABI level and must be output in chunk with
> the function they are attached.

Similar to aliases (which, of course, also has ugly current handling).

> 3) our RTL output machinery is somewhat convoluted.

For thunks?  Yeah.

> the second two needs following changes I know of
> 
>    a) unreachable function removal must be weakened a bit on thunk functions
>    b) WHOPR partitioning must know that it always have to associate thunk 
>       into every partition the thunked function comes to

Similar to aliases.

>    c) cgraph_assemble_function 

Or rather expand.

> My code bootstraps, but does not really work for i.e. Mozilla, so it is
> entirely possible more side corners needs to be handled, too ;(

So, indeed it sort-of makes sense to do symtab related changes first to
get aliases implemented in a sane way.  As thunks really need similar
handling in a lot of places.

Richard.

> Honza

[Bug middle-end/48674] [4.7 Regression] FAIL: g++.dg/torture/pr48661.C

[hubicka at gcc dot gnu.org]

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

--- Comment #11 from Jan Hubicka <hubicka at gcc dot gnu.org> 2011-05-02 15:26:23 UTC ---
> Most of them is moot if we just accept that direct calls to thunks are
> a barrier for IPA optimizations (in the case they do not have a gimple
> body).  We could represent variadic thunks in gimple using
> __builtin_va_arg_pack

Well, we could also go for the "half way" variant. Represent those thunks we
can and handle those thunks we can't as special cases. Problem of this is
  1) real thunks will become even more weird and rare special case (I think
     we have two testcases for variadic thunks in testsuite)
  2) we will need to forbid certain class of transformations of boides or the
     plan to throw them away won't work.

     We don't want to change function signatures, for example, we don't want
     to remove THIS parameter because we prove it is dead. 
     Things, like cloning, will most likely have tendency to upset ASM
     machinery in particular on exotic targets.
It seems to me that we would replace one ugly case of thunks, but two classes
of more evil thunks and less evil thunks.

> Similar to aliases (which, of course, also has ugly current handling).

Yes, aliases and thunks are symmetric to large extend. Thus my plan to handle

> So, indeed it sort-of makes sense to do symtab related changes first to
> get aliases implemented in a sane way.  As thunks really need similar
> handling in a lot of places.

Yep, or go the other way - first get thunks working and then make aliases on
top of them.  Aliases are more difficult than thunks by
  1) they can have different visibilities than the symbol they alias
  2) there is problem with fact that we don't know symbol names
  3) they have target specific behavior. I.e. ELF has no aliases in end, but
     some targets do. 
So one way or another, the conversion is not fun ;(

Honza

[Bug middle-end/48674] [4.7 Regression] FAIL: g++.dg/torture/pr48661.C

[dominiq at lps dot ens.fr]

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

--- Comment #12 from Dominique d'Humieres <dominiq at lps dot ens.fr> 2011-05-14 12:31:43 UTC ---
These failures disappeared on between revisions 173492 and 173525 on
powerpc-apple-darwin9.8 (see
http://gcc.gnu.org/ml/gcc-testresults/2011-05/msg00719.html and
http://gcc.gnu.org/ml/gcc-testresults/2011-05/msg00759.html ) and revisions
173516 and 173518 on i686-pc-linux-gnu (see
http://gcc.gnu.org/ml/gcc-testresults/2011-05/msg00708.html and
http://gcc.gnu.org/ml/gcc-testresults/2011-05/msg00718.html ). So this PR is
fixed/hidden by revision 173517.

[Bug middle-end/48674] [4.7 Regression] FAIL: g++.dg/torture/pr48661.C

[rguenth at gcc dot gnu.org]

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

Richard Guenther <rguenth at gcc dot gnu.org> changed:

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

--- Comment #13 from Richard Guenther <rguenth at gcc dot gnu.org> 2011-08-02 14:03:48 UTC ---
Because it was fixed.