[Bug c++/14639] New: Incorrect emission of unused compiler-generated destructor at -O2

[Bug c++/14639] New: Incorrect emission of unused compiler-generated destructor at -O2

[austern at apple dot com]

Consider the following file:
class AAA {
public:
  virtual ~AAA();
};

class BBB {
public:
  virtual ~BBB();
};

class xyz : public AAA, public BBB {
public:
  xyz() : AAA(), BBB() { }
};




Class xyz has no key method, so its vtable should be emitted only in translation units where it's used.  
With "gcc -O0 -c foo.cc" I get what I expect: the compiler doesn't emit _ZTV3xyz, _ZN3xyzD0Ev, or 
anything else.  With "gcc -O2 -c foo.cc" I get incorrect behavior: the compiler emits xyz's destructor 
and vtable, thunks, and all of the things in AAA and BBB that are required to support them.

This is a regression.  In 3.2.2, O0 and O2 both behave correctly.

-- 
           Summary: Incorrect emission of unused compiler-generated
                    destructor at -O2
           Product: gcc
           Version: 3.5.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P2
         Component: c++
        AssignedTo: unassigned at gcc dot gnu dot org
        ReportedBy: austern at apple dot com
                CC: gcc-bugs at gcc dot gnu dot org
 GCC build triplet: i686-pc-linux-gnu
  GCC host triplet: i686-pc-linux-gnu
GCC target triplet: i686-pc-linux-gnu


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

[Bug c++/14639] [3.4/3.5 Regression] [unit-at-a-time] Incorrect emission of unused compiler-generated destructor at -O2

[pinskia at gcc dot gnu dot org]

------- Additional Comments From pinskia at gcc dot gnu dot org  2004-03-19 01:33 -------
Mark/Jan, what is the expected behavior here, this is a regression from 3.3.  It is caused by unit-at-a-
time always trying to emit the vtable.

-- 
           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |mmitchel at gcc dot gnu dot
                   |                            |org, hubicka at gcc dot gnu
                   |                            |dot org
             Status|UNCONFIRMED                 |NEW
     Ever Confirmed|                            |1
      Known to fail|                            |3.4.0 3.5.0
      Known to work|                            |3.3
   Last reconfirmed|0000-00-00 00:00:00         |2004-03-19 01:33:08
               date|                            |
            Summary|Incorrect emission of unused|[3.4/3.5 Regression] [unit-
                   |compiler-generated          |at-a-time] Incorrect
                   |destructor at -O2           |emission of unused compiler-
                   |                            |generated destructor at -O2
   Target Milestone|---                         |3.4.1
            Version|3.5.0                       |3.4.0


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

[Bug c++/14639] [3.4/3.5 Regression] [unit-at-a-time] Incorrect emission of unused compiler-generated destructor at -O2

[mmitchel at gcc dot gnu dot org]

------- Additional Comments From mmitchel at gcc dot gnu dot org  2004-03-19 02:27 -------
This is a bug in unit-at-a-time.

I believe this actually makes the compiler non-conformant to the C++ standard
because it will result in generating functions for which no generation is
possible.  That might be incorrect, but this is at the very least a serious QoI
problem.

Jan?

-- 


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

[Bug c++/14639] [3.4/3.5 Regression] [unit-at-a-time] Incorrect emission of unused compiler-generated destructor at -O2

[austern at apple dot com]

------- Additional Comments From austern at apple dot com  2004-03-19 04:02 -------
I haven't yet looked at whether this means the compiler fails to conform to the C++ standard, but I'm 
sure it means we now fail to conform to the Itanium C++ ABI.

-- 


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

[Bug c++/14639] [3.4/3.5 Regression] [unit-at-a-time] Incorrect emission of unused compiler-generated destructor at -O2

[mark at codesourcery dot com]

------- Additional Comments From mark at codesourcery dot com  2004-03-19 04:44 -------
Subject: Re:  [3.4/3.5 Regression] [unit-at-a-time] Incorrect
 emission of unused compiler-generated destructor at -O2

austern at apple dot com wrote:

>------- Additional Comments From austern at apple dot com  2004-03-19 04:02 -------
>I haven't yet looked at whether this means the compiler fails to conform to the C++ standard, but I'm 
>sure it means we now fail to conform to the Itanium C++ ABI.
>  
>
Yes, indeed.



-- 


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

[Bug c++/14639] [3.4/3.5 Regression] [unit-at-a-time] Incorrect emission of unused compiler-generated destructor at -O2

[hubicka at ucw dot cz]

------- Additional Comments From hubicka at ucw dot cz  2004-03-21 18:26 -------
Subject: Re:  New: Incorrect emission of unused compiler-generated destructor at -O2

> Consider the following file:
> class AAA {
> public:
>   virtual ~AAA();
> };
> 
> class BBB {
> public:
>   virtual ~BBB();
> };
> 
> class xyz : public AAA, public BBB {
> public:
>   xyz() : AAA(), BBB() { }
> };
> 
> 
> 
> 
> Class xyz has no key method, so its vtable should be emitted only in translation units where it's used.  
> With "gcc -O0 -c foo.cc" I get what I expect: the compiler doesn't emit _ZTV3xyz, _ZN3xyzD0Ev, or 
> anything else.  With "gcc -O2 -c foo.cc" I get incorrect behavior: the compiler emits xyz's destructor 
> and vtable, thunks, and all of the things in AAA and BBB that are required to support them.
> 
> This is a regression.  In 3.2.2, O0 and O2 both behave correctly.

This can be avoided by disabling following mark_referenced call:

Index: cp/method.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/cp/method.c,v
retrieving revision 1.279
diff -c -3 -p -r1.279 method.c
*** cp/method.c	12 Mar 2004 17:09:01 -0000	1.279
--- cp/method.c	20 Mar 2004 15:27:33 -0000
*************** use_thunk (tree thunk_fndecl, bool emit_
*** 359,365 ****
       this translation unit.  */
    TREE_ADDRESSABLE (function) = 1;
    mark_used (function);
!   mark_referenced (DECL_ASSEMBLER_NAME (function));
    if (!emit_p)
      return;
  
--- 359,365 ----
       this translation unit.  */
    TREE_ADDRESSABLE (function) = 1;
    mark_used (function);
!   /*mark_referenced (DECL_ASSEMBLER_NAME (function));*/
    if (!emit_p)
      return;
  
The problem is that C++ frotnend manages thunks "on the side" and does
not show to backend what DECLs will be used by thunks and what thunks
will be emit.  This is dealt with handling the depenedencies
conservativly.   This line comes from replacing DECL_REFERENCED.* = 1 by
mark_referenced call so I am not quite sure it is still necesary
(testsuite passes without this line).

However we likely need some way to explain backend the hidden
dependencies.  Would it be possible to explain me why exactly the thunk
needs to be output and what DECLs will it reference?
(I know that the thunks are associated to functions and are output at
the same time the function is output and they become needed by
use_thunk, but this all can be overly conservative now)

Honza
> 
> -- 
>            Summary: Incorrect emission of unused compiler-generated
>                     destructor at -O2
>            Product: gcc
>            Version: 3.5.0
>             Status: UNCONFIRMED
>           Severity: normal
>           Priority: P2
>          Component: c++
>         AssignedTo: unassigned at gcc dot gnu dot org
>         ReportedBy: austern at apple dot com
>                 CC: gcc-bugs at gcc dot gnu dot org
>  GCC build triplet: i686-pc-linux-gnu
>   GCC host triplet: i686-pc-linux-gnu
> GCC target triplet: i686-pc-linux-gnu
> 
> 
> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=14639


-- 


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

Re: [Bug c++/14639] New: Incorrect emission of unused compiler-generated destructor at -O2

[Jan Hubicka]

> Consider the following file:
> class AAA {
> public:
>   virtual ~AAA();
> };
> 
> class BBB {
> public:
>   virtual ~BBB();
> };
> 
> class xyz : public AAA, public BBB {
> public:
>   xyz() : AAA(), BBB() { }
> };
> 
> 
> 
> 
> Class xyz has no key method, so its vtable should be emitted only in translation units where it's used.  
> With "gcc -O0 -c foo.cc" I get what I expect: the compiler doesn't emit _ZTV3xyz, _ZN3xyzD0Ev, or 
> anything else.  With "gcc -O2 -c foo.cc" I get incorrect behavior: the compiler emits xyz's destructor 
> and vtable, thunks, and all of the things in AAA and BBB that are required to support them.
> 
> This is a regression.  In 3.2.2, O0 and O2 both behave correctly.

This can be avoided by disabling following mark_referenced call:

Index: cp/method.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/cp/method.c,v
retrieving revision 1.279
diff -c -3 -p -r1.279 method.c
*** cp/method.c	12 Mar 2004 17:09:01 -0000	1.279
--- cp/method.c	20 Mar 2004 15:27:33 -0000
*************** use_thunk (tree thunk_fndecl, bool emit_
*** 359,365 ****
       this translation unit.  */
    TREE_ADDRESSABLE (function) = 1;
    mark_used (function);
!   mark_referenced (DECL_ASSEMBLER_NAME (function));
    if (!emit_p)
      return;
  
--- 359,365 ----
       this translation unit.  */
    TREE_ADDRESSABLE (function) = 1;
    mark_used (function);
!   /*mark_referenced (DECL_ASSEMBLER_NAME (function));*/
    if (!emit_p)
      return;
  
The problem is that C++ frotnend manages thunks "on the side" and does
not show to backend what DECLs will be used by thunks and what thunks
will be emit.  This is dealt with handling the depenedencies
conservativly.   This line comes from replacing DECL_REFERENCED.* = 1 by
mark_referenced call so I am not quite sure it is still necesary
(testsuite passes without this line).

However we likely need some way to explain backend the hidden
dependencies.  Would it be possible to explain me why exactly the thunk
needs to be output and what DECLs will it reference?
(I know that the thunks are associated to functions and are output at
the same time the function is output and they become needed by
use_thunk, but this all can be overly conservative now)

Honza
> 
> -- 
>            Summary: Incorrect emission of unused compiler-generated
>                     destructor at -O2
>            Product: gcc
>            Version: 3.5.0
>             Status: UNCONFIRMED
>           Severity: normal
>           Priority: P2
>          Component: c++
>         AssignedTo: unassigned at gcc dot gnu dot org
>         ReportedBy: austern at apple dot com
>                 CC: gcc-bugs at gcc dot gnu dot org
>  GCC build triplet: i686-pc-linux-gnu
>   GCC host triplet: i686-pc-linux-gnu
> GCC target triplet: i686-pc-linux-gnu
> 
> 
> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=14639

Re: [Bug c++/14639] New: Incorrect emission of unused compiler-generated destructor at -O2

[Mark Mitchell]

Jan Hubicka wrote:

>>Consider the following file:
>>class AAA {
>>public:
>>  virtual ~AAA();
>>};
>>
>>class BBB {
>>public:
>>  virtual ~BBB();
>>};
>>
>>class xyz : public AAA, public BBB {
>>public:
>>  xyz() : AAA(), BBB() { }
>>};
>>
>>
>>
>>
>>Class xyz has no key method, so its vtable should be emitted only in translation units where it's used.  
>>With "gcc -O0 -c foo.cc" I get what I expect: the compiler doesn't emit _ZTV3xyz, _ZN3xyzD0Ev, or 
>>anything else.  With "gcc -O2 -c foo.cc" I get incorrect behavior: the compiler emits xyz's destructor 
>>and vtable, thunks, and all of the things in AAA and BBB that are required to support them.
>>
>>This is a regression.  In 3.2.2, O0 and O2 both behave correctly.
>>    
>>
>
>This can be avoided by disabling following mark_referenced call:
>
>Index: cp/method.c
>===================================================================
>RCS file: /cvs/gcc/gcc/gcc/cp/method.c,v
>retrieving revision 1.279
>diff -c -3 -p -r1.279 method.c
>*** cp/method.c	12 Mar 2004 17:09:01 -0000	1.279
>--- cp/method.c	20 Mar 2004 15:27:33 -0000
>*************** use_thunk (tree thunk_fndecl, bool emit_
>*** 359,365 ****
>       this translation unit.  */
>    TREE_ADDRESSABLE (function) = 1;
>    mark_used (function);
>!   mark_referenced (DECL_ASSEMBLER_NAME (function));
>    if (!emit_p)
>      return;
>  
>--- 359,365 ----
>       this translation unit.  */
>    TREE_ADDRESSABLE (function) = 1;
>    mark_used (function);
>!   /*mark_referenced (DECL_ASSEMBLER_NAME (function));*/
>    if (!emit_p)
>      return;
>  
>The problem is that C++ frotnend manages thunks "on the side" and does
>not show to backend what DECLs will be used by thunks and what thunks
>will be emit.  This is dealt with handling the depenedencies
>conservativly.   This line comes from replacing DECL_REFERENCED.* = 1 by
>mark_referenced call so I am not quite sure it is still necesary
>(testsuite passes without this line).
>
>However we likely need some way to explain backend the hidden
>dependencies.  Would it be possible to explain me why exactly the thunk
>needs to be output and what DECLs will it reference?
>(I know that the thunks are associated to functions and are output at
>the same time the function is output and they become needed by
>use_thunk, but this all can be overly conservative now)
>  
>
Every thunk references exactly one function.  The thunk is just a little 
stub that adjusts one incoming parameter, and then transfers control to 
the referenced function.

 From a FUNCTION_DECL, you can look at DECL_THUNKS to find all of its 
thunks.  From a thunk, you can look at THUNK_TARGET to find the function 
to which it transfers control.  It does make sense to me that we call 
mark_referenced in the case that emit_p is zero.  If emit_p is 1, that 
does make sense.  In the emit_p == 0 case, we should wait to emit the 
thunk until we know that we need to emit the vtable referencing it, 
which is what we used to do.

-- 
Mark Mitchell
CodeSourcery, LLC
(916) 791-8304
mark@codesourcery.com

Re: [Bug c++/14639] New: Incorrect emission of unused compiler-generated destructor at -O2

[Jan Hubicka]

> Every thunk references exactly one function.  The thunk is just a little 
> stub that adjusts one incoming parameter, and then transfers control to 
> the referenced function.

OK, that was my original understanding but somehow I got to impression
that it is more complex.
So when the function itself gets marked as necesary we automatically
emit the thunk when the mark_used_thunk has been called, right?
I guess we need two thinks - we need to manage that once we discover
thunk to be necesary we need to mark the function it is refering to and
we need to emit thunks iff they are really reachable (not just used,
right)?

It would be ideal if the thunk was behaving like ordinary function, but
for some reason it is not.  Is there way to recognize thunk in C++
independent way?

I guess we will need kind of hooks - tell cgraph code that given decl is
thunk and thunk is like function but not quite as it has no body and
reffers to other function and it is not necesay to assemble it as it
gets assembled magically with the function body.  This is quite nasty (I
really don't want to introduce new kind of function - we already have
way too many of them), but does it at least sound right to you?
> 
> From a FUNCTION_DECL, you can look at DECL_THUNKS to find all of its 
> thunks.  From a thunk, you can look at THUNK_TARGET to find the function 
> to which it transfers control.  It does make sense to me that we call 
> mark_referenced in the case that emit_p is zero.  If emit_p is 1, that 
> does make sense.  In the emit_p == 0 case, we should wait to emit the 
> thunk until we know that we need to emit the vtable referencing it, 
> which is what we used to do.
Hmm, perhaps I am getting overzelaous here (hope so).  I am not quite
sure how emit_p works. When the emit_p is set?  If it is at function
expansion time, it is too late - we need to be finished this in advance.

Honza
> 
> -- 
> Mark Mitchell
> CodeSourcery, LLC
> (916) 791-8304
> mark@codesourcery.com

Re: [Bug c++/14639] New: Incorrect emission of unused compiler-generated destructor at -O2

[Mark Mitchell]

> So when the function itself gets marked as necesary we automatically
> emit the thunk when the mark_used_thunk has been called, right?

Yes.  Whenever a function is emittted, so are all of its associated thunks.

> I guess we need two thinks - we need to manage that once we discover
> thunk to be necesary we need to mark the function it is refering to and
> we need to emit thunks iff they are really reachable (not just used,
> right)?

Yes.

> It would be ideal if the thunk was behaving like ordinary function, but
> for some reason it is not.  Is there way to recognize thunk in C++
> independent way?

No.  But, you could always change things so that was possible, I 
suppose.  Thunks cannot be represented as ordinary functions until we 
have multiple entry points, and we don't yet have that.

> I guess we will need kind of hooks - tell cgraph code that given decl is
> thunk and thunk is like function but not quite as it has no body and
> reffers to other function and it is not necesay to assemble it as it
> gets assembled magically with the function body.

Yes.

> Hmm, perhaps I am getting overzelaous here (hope so).  I am not quite
> sure how emit_p works. When the emit_p is set?  If it is at function
> expansion time, it is too late - we need to be finished this in advance.

emit_p is set from emit_associated_thunks, which is called for a 
function's thunks right before the function itself is sent to 
tree_rest_of_compilation.

-- 
Mark Mitchell
CodeSourcery, LLC
mark@codesourcery.com

Re: [Bug c++/14639] New: Incorrect emission of unused compiler-generated destructor at -O2

[Jan Hubicka]

> >So when the function itself gets marked as necesary we automatically
> >emit the thunk when the mark_used_thunk has been called, right?
> 
> Yes.  Whenever a function is emittted, so are all of its associated thunks.
> 
> >I guess we need two thinks - we need to manage that once we discover
> >thunk to be necesary we need to mark the function it is refering to and
> >we need to emit thunks iff they are really reachable (not just used,
> >right)?
> 
> Yes.
> 
> >It would be ideal if the thunk was behaving like ordinary function, but
> >for some reason it is not.  Is there way to recognize thunk in C++
> >independent way?
> 
> No.  But, you could always change things so that was possible, I 
> suppose.  Thunks cannot be represented as ordinary functions until we 
> have multiple entry points, and we don't yet have that.
> 
> >I guess we will need kind of hooks - tell cgraph code that given decl is
> >thunk and thunk is like function but not quite as it has no body and
> >reffers to other function and it is not necesay to assemble it as it
> >gets assembled magically with the function body.
> 
> Yes.
> 
> >Hmm, perhaps I am getting overzelaous here (hope so).  I am not quite
> >sure how emit_p works. When the emit_p is set?  If it is at function
> >expansion time, it is too late - we need to be finished this in advance.
> 
> emit_p is set from emit_associated_thunks, which is called for a 
> function's thunks right before the function itself is sent to 
> tree_rest_of_compilation.

Mark,
I am trying to look into it, but I am not able to figure out testcase
where the patch above fails.  It should have function that is referenced
by thunk.  Additionally the thunk needs to be output but the function
itself does not (if there wasn't the reference from the thunk).  In that
case I would expect us to fail to see the dependency in between thunk
and function and not output it at all with the patch applied.

Do you have idea how such testcase shall look like?  I am really not
very faimiliar with these details of C++ implementation making it
somewhat dificult to track this down resonably.

Thanks,
Honza

[Bug c++/14639] [3.4/3.5 Regression] [unit-at-a-time] Incorrect emission of unused compiler-generated destructor at -O2

[mark at codesourcery dot com]

------- Additional Comments From mark at codesourcery dot com  2004-03-21 19:25 -------
Subject: Re:  New: Incorrect emission of unused compiler-generated
 destructor at -O2

Jan Hubicka wrote:

>>Consider the following file:
>>class AAA {
>>public:
>>  virtual ~AAA();
>>};
>>
>>class BBB {
>>public:
>>  virtual ~BBB();
>>};
>>
>>class xyz : public AAA, public BBB {
>>public:
>>  xyz() : AAA(), BBB() { }
>>};
>>
>>
>>
>>
>>Class xyz has no key method, so its vtable should be emitted only in translation units where it's used.  
>>With "gcc -O0 -c foo.cc" I get what I expect: the compiler doesn't emit _ZTV3xyz, _ZN3xyzD0Ev, or 
>>anything else.  With "gcc -O2 -c foo.cc" I get incorrect behavior: the compiler emits xyz's destructor 
>>and vtable, thunks, and all of the things in AAA and BBB that are required to support them.
>>
>>This is a regression.  In 3.2.2, O0 and O2 both behave correctly.
>>    
>>
>
>This can be avoided by disabling following mark_referenced call:
>
>Index: cp/method.c
>===================================================================
>RCS file: /cvs/gcc/gcc/gcc/cp/method.c,v
>retrieving revision 1.279
>diff -c -3 -p -r1.279 method.c
>*** cp/method.c	12 Mar 2004 17:09:01 -0000	1.279
>--- cp/method.c	20 Mar 2004 15:27:33 -0000
>*************** use_thunk (tree thunk_fndecl, bool emit_
>*** 359,365 ****
>       this translation unit.  */
>    TREE_ADDRESSABLE (function) = 1;
>    mark_used (function);
>!   mark_referenced (DECL_ASSEMBLER_NAME (function));
>    if (!emit_p)
>      return;
>  
>--- 359,365 ----
>       this translation unit.  */
>    TREE_ADDRESSABLE (function) = 1;
>    mark_used (function);
>!   /*mark_referenced (DECL_ASSEMBLER_NAME (function));*/
>    if (!emit_p)
>      return;
>  
>The problem is that C++ frotnend manages thunks "on the side" and does
>not show to backend what DECLs will be used by thunks and what thunks
>will be emit.  This is dealt with handling the depenedencies
>conservativly.   This line comes from replacing DECL_REFERENCED.* = 1 by
>mark_referenced call so I am not quite sure it is still necesary
>(testsuite passes without this line).
>
>However we likely need some way to explain backend the hidden
>dependencies.  Would it be possible to explain me why exactly the thunk
>needs to be output and what DECLs will it reference?
>(I know that the thunks are associated to functions and are output at
>the same time the function is output and they become needed by
>use_thunk, but this all can be overly conservative now)
>  
>
Every thunk references exactly one function.  The thunk is just a little 
stub that adjusts one incoming parameter, and then transfers control to 
the referenced function.

 From a FUNCTION_DECL, you can look at DECL_THUNKS to find all of its 
thunks.  From a thunk, you can look at THUNK_TARGET to find the function 
to which it transfers control.  It does make sense to me that we call 
mark_referenced in the case that emit_p is zero.  If emit_p is 1, that 
does make sense.  In the emit_p == 0 case, we should wait to emit the 
thunk until we know that we need to emit the vtable referencing it, 
which is what we used to do.



-- 


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

[Bug c++/14639] [3.4/3.5 Regression] [unit-at-a-time] Incorrect emission of unused compiler-generated destructor at -O2

[steven at gcc dot gnu dot org]

------- Additional Comments From steven at gcc dot gnu dot org  2004-03-21 23:32 -------
As mentioned in the audit trail, with this bug "we now fail to conform to the 
Itanium C++ ABI."  Mark, wouldn't that make this bug a definite show stopper 
for 3.4.0, and can this bug therefore be retargeted for 3.4.0? 
 

-- 


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

[Bug c++/14639] [3.4/3.5 Regression] [unit-at-a-time] Incorrect emission of unused compiler-generated destructor at -O2

[jh at suse dot cz]

------- Additional Comments From jh at suse dot cz  2004-03-22 09:44 -------
Subject: Re:  New: Incorrect emission of unused compiler-generated destructor at -O2

> Every thunk references exactly one function.  The thunk is just a little 
> stub that adjusts one incoming parameter, and then transfers control to 
> the referenced function.

OK, that was my original understanding but somehow I got to impression
that it is more complex.
So when the function itself gets marked as necesary we automatically
emit the thunk when the mark_used_thunk has been called, right?
I guess we need two thinks - we need to manage that once we discover
thunk to be necesary we need to mark the function it is refering to and
we need to emit thunks iff they are really reachable (not just used,
right)?

It would be ideal if the thunk was behaving like ordinary function, but
for some reason it is not.  Is there way to recognize thunk in C++
independent way?

I guess we will need kind of hooks - tell cgraph code that given decl is
thunk and thunk is like function but not quite as it has no body and
reffers to other function and it is not necesay to assemble it as it
gets assembled magically with the function body.  This is quite nasty (I
really don't want to introduce new kind of function - we already have
way too many of them), but does it at least sound right to you?
> 
> From a FUNCTION_DECL, you can look at DECL_THUNKS to find all of its 
> thunks.  From a thunk, you can look at THUNK_TARGET to find the function 
> to which it transfers control.  It does make sense to me that we call 
> mark_referenced in the case that emit_p is zero.  If emit_p is 1, that 
> does make sense.  In the emit_p == 0 case, we should wait to emit the 
> thunk until we know that we need to emit the vtable referencing it, 
> which is what we used to do.
Hmm, perhaps I am getting overzelaous here (hope so).  I am not quite
sure how emit_p works. When the emit_p is set?  If it is at function
expansion time, it is too late - we need to be finished this in advance.

Honza
> 
> -- 
> Mark Mitchell
> CodeSourcery, LLC
> (916) 791-8304
> mark@codesourcery.com


-- 


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

[Bug c++/14639] [3.4/3.5 Regression] [unit-at-a-time] Incorrect emission of unused compiler-generated destructor at -O2

[mark at codesourcery dot com]

------- Additional Comments From mark at codesourcery dot com  2004-03-22 15:31 -------
Subject: Re:  New: Incorrect emission of unused compiler-generated
 destructor at -O2

> So when the function itself gets marked as necesary we automatically
> emit the thunk when the mark_used_thunk has been called, right?

Yes.  Whenever a function is emittted, so are all of its associated thunks.

> I guess we need two thinks - we need to manage that once we discover
> thunk to be necesary we need to mark the function it is refering to and
> we need to emit thunks iff they are really reachable (not just used,
> right)?

Yes.

> It would be ideal if the thunk was behaving like ordinary function, but
> for some reason it is not.  Is there way to recognize thunk in C++
> independent way?

No.  But, you could always change things so that was possible, I 
suppose.  Thunks cannot be represented as ordinary functions until we 
have multiple entry points, and we don't yet have that.

> I guess we will need kind of hooks - tell cgraph code that given decl is
> thunk and thunk is like function but not quite as it has no body and
> reffers to other function and it is not necesay to assemble it as it
> gets assembled magically with the function body.

Yes.

> Hmm, perhaps I am getting overzelaous here (hope so).  I am not quite
> sure how emit_p works. When the emit_p is set?  If it is at function
> expansion time, it is too late - we need to be finished this in advance.

emit_p is set from emit_associated_thunks, which is called for a 
function's thunks right before the function itself is sent to 
tree_rest_of_compilation.



-- 


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

[Bug c++/14639] [3.4/3.5 Regression] [unit-at-a-time] Incorrect emission of unused compiler-generated destructor at -O2

[jh at suse dot cz]

------- Additional Comments From jh at suse dot cz  2004-03-23 09:52 -------
Subject: Re:  New: Incorrect emission of unused compiler-generated destructor at -O2

> >So when the function itself gets marked as necesary we automatically
> >emit the thunk when the mark_used_thunk has been called, right?
> 
> Yes.  Whenever a function is emittted, so are all of its associated thunks.
> 
> >I guess we need two thinks - we need to manage that once we discover
> >thunk to be necesary we need to mark the function it is refering to and
> >we need to emit thunks iff they are really reachable (not just used,
> >right)?
> 
> Yes.
> 
> >It would be ideal if the thunk was behaving like ordinary function, but
> >for some reason it is not.  Is there way to recognize thunk in C++
> >independent way?
> 
> No.  But, you could always change things so that was possible, I 
> suppose.  Thunks cannot be represented as ordinary functions until we 
> have multiple entry points, and we don't yet have that.
> 
> >I guess we will need kind of hooks - tell cgraph code that given decl is
> >thunk and thunk is like function but not quite as it has no body and
> >reffers to other function and it is not necesay to assemble it as it
> >gets assembled magically with the function body.
> 
> Yes.
> 
> >Hmm, perhaps I am getting overzelaous here (hope so).  I am not quite
> >sure how emit_p works. When the emit_p is set?  If it is at function
> >expansion time, it is too late - we need to be finished this in advance.
> 
> emit_p is set from emit_associated_thunks, which is called for a 
> function's thunks right before the function itself is sent to 
> tree_rest_of_compilation.

Mark,
I am trying to look into it, but I am not able to figure out testcase
where the patch above fails.  It should have function that is referenced
by thunk.  Additionally the thunk needs to be output but the function
itself does not (if there wasn't the reference from the thunk).  In that
case I would expect us to fail to see the dependency in between thunk
and function and not output it at all with the patch applied.

Do you have idea how such testcase shall look like?  I am really not
very faimiliar with these details of C++ implementation making it
somewhat dificult to track this down resonably.

Thanks,
Honza


-- 


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

[Bug c++/14639] [3.4/3.5 Regression] [unit-at-a-time] Incorrect emission of unused compiler-generated destructor at -O2

[mark at codesourcery dot com]

------- Additional Comments From mark at codesourcery dot com  2004-03-23 21:25 -------
Subject: Re:  [3.4/3.5 Regression] [unit-at-a-time] Incorrect
 emission of unused compiler-generated destructor at -O2


>I am trying to look into it, but I am not able to figure out testcase
>where the patch above fails.  It should have function that is referenced
>by thunk.  Additionally the thunk needs to be output but the function
>itself does not (if there wasn't the reference from the thunk).  In that
>case I would expect us to fail to see the dependency in between thunk
>and function and not output it at all with the patch applied.
>
>Do you have idea how such testcase shall look like?  I am really not
>very faimiliar with these details of C++ implementation making it
>somewhat dificult to track this down resonably.
>  
>
There are three kinds of entities in play:

(1) Vtables

(2) Virtual functions

(3) Thunks

A vtable is an array, containing pointers to either virtual functions or 
thunks.  Thunks are little stubs that reference a virtual function.  A 
virtual function and its associated thunks are always emitted together.

Are you sure this is a problem with thunks and not with vtables?

In the test case, the object file should be empty.  There are no 
definitions of any functions except for the inline xyz::xyz method, and 
since it is inline it will not be emitted.

I expect that the bug is that cgraph is incorrectly deciding to emit a 
vtable.  Having decided that, you are emitting everything else because 
it is pointed to from the vtable.

The way to solve this problem is to figure out what thing you are 
deciding to output first.  Since the object file should be entirely 
empty, outputting anything at all is a bug.

I've retargeted this at 3.4.0, as not solving this will cause incredible 
amounts of code bloat for C++ applications.



-- 


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

[Bug c++/14639] [3.4/3.5 Regression] [unit-at-a-time] Incorrect emission of unused compiler-generated destructor at -O2

[mmitchel at gcc dot gnu dot org]

------- Additional Comments From mmitchel at gcc dot gnu dot org  2004-03-23 21:26 -------
Retargeted at 3.4.0.

-- 
           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|3.4.1                       |3.4.0


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

[Bug c++/14639] [3.4/3.5 Regression] [unit-at-a-time] Incorrect emission of unused compiler-generated destructor at -O2

[mmitchel at gcc dot gnu dot org]

------- Additional Comments From mmitchel at gcc dot gnu dot org  2004-03-23 21:25 -------
Retargeted at 3.4.0.

-- 


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

[Bug c++/14639] [3.4/3.5 Regression] [unit-at-a-time] Incorrect emission of unused compiler-generated destructor at -O2

[hubicka at ucw dot cz]

------- Additional Comments From hubicka at ucw dot cz  2004-03-24 18:22 -------
Subject: Re:  [3.4/3.5 Regression] [unit-at-a-time] Incorrect emission of unused compiler-generated destructor at -O2

> 
> ------- Additional Comments From mark at codesourcery dot com  2004-03-23 21:25 -------
> Subject: Re:  [3.4/3.5 Regression] [unit-at-a-time] Incorrect
>  emission of unused compiler-generated destructor at -O2
> 
> 
> >I am trying to look into it, but I am not able to figure out testcase
> >where the patch above fails.  It should have function that is referenced
> >by thunk.  Additionally the thunk needs to be output but the function
> >itself does not (if there wasn't the reference from the thunk).  In that
> >case I would expect us to fail to see the dependency in between thunk
> >and function and not output it at all with the patch applied.
> >
> >Do you have idea how such testcase shall look like?  I am really not
> >very faimiliar with these details of C++ implementation making it
> >somewhat dificult to track this down resonably.
> >  
> >
> There are three kinds of entities in play:
> 
> (1) Vtables
> 
> (2) Virtual functions
> 
> (3) Thunks
> 
> A vtable is an array, containing pointers to either virtual functions or 
> thunks.  Thunks are little stubs that reference a virtual function.  A 
> virtual function and its associated thunks are always emitted together.
> 
> Are you sure this is a problem with thunks and not with vtables?

Yes.  The way cgraph code works is giving
cgraph_finalize_function/cgraph_finalize_variable calls that adds
function/variable into cgraph datastructure.  It is not directly output
to file as we used to do, instead cgraph decide when it is good time to
do so (at the end of compilation for unit-at-a-time or immediately in
most cases for non-unit-at-a-time) and if do it at all.

Both vtables and functions are not output until they are needed (either
externally visiable or referenced by some datastructure or function that
is already needed).  Problem with thunks is that they go on side and
they are neither functions nor variables.  The old code contained
several TREE_SYMBOL_REFERENCED(...)=1 calls that are now replaced by
mark_referenced (...) calls that makes cgraph code to believe that the
functions/vtables are referenced by some already output
assembly and thus they get scheduled to be output as well
unconditionally.
> 
> In the test case, the object file should be empty.  There are no 
> definitions of any functions except for the inline xyz::xyz method, and 
> since it is inline it will not be emitted.

This happens with the patch I sent earlier.  It just removes one of
mark_referenced calls in the C++ backend.  There are 4 such calls and I
am unsure if they are actaully needed or not.
I am seeking for testcase exhibiting that mark_referenced at that place
is needed.  (I've originally replaced them from the
TREE_SYMBOL_REFERENCED and didn't really checked it without the change).
> 
> I expect that the bug is that cgraph is incorrectly deciding to emit a 
> vtable.  Having decided that, you are emitting everything else because 
> it is pointed to from the vtable.

It goes by C++ frontend deciding to emit vtable in C++ frontend notion
(so it is delayed until needed by cgraph), this makes C++ frontend to
mark_used the thunk in the vtable that in turn makes mark_referenced the
function and cgraph code gets fooled.

We simply need to figure way to do the trick without using
mark_referenced bit and instead making the dependency obvious to cgraph.

The thunks are referenced only by vtables, right?  Perhaps the thunks
should not be mark_used at all during the C++ frontend is looking for
used thinks and we can go directly to the thunked functions.

Later we can we can catch it inside mark_referenced via C++ hook that
will mark_used the thunk.  That way we will again output only thunks
that are referenced by vtables that are output to assembly file.

At the moment cgraph code works by actually outputting all global
variables until all needed functions are known, so such patch would work
for 3.4.  For 3.5 I would preffer to reorganize things in a way so
variables are output very last so we can take into account the scenarios
where variable used to be reachable but optimizers elliminated all
references to it.  But for 3.5 we may want to get the multiple entry
points working too.
> 
> The way to solve this problem is to figure out what thing you are 
> deciding to output first.  Since the object file should be entirely 
> empty, outputting anything at all is a bug.
> 
> I've retargeted this at 3.4.0, as not solving this will cause incredible 
> amounts of code bloat for C++ applications.

How much incredible it is?  (ie having testcase that exhibits a lot of
unnecesary thunks/vtables would be usefull too). 

Honza
> 
> 
> 
> -- 
> 
> 
> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=14639
> 
> ------- You are receiving this mail because: -------
> You are on the CC list for the bug, or are watching someone who is.


-- 


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

[Bug c++/14639] [3.4/3.5 Regression] [unit-at-a-time] Incorrect emission of unused compiler-generated destructor at -O2

[mark at codesourcery dot com]

------- Additional Comments From mark at codesourcery dot com  2004-03-24 18:44 -------
Subject: Re:  [3.4/3.5 Regression] [unit-at-a-time] Incorrect
 emission of unused compiler-generated destructor at -O2

>>In the test case, the object file should be empty.  There are no 
>>definitions of any functions except for the inline xyz::xyz method, and 
>>since it is inline it will not be emitted.
> 
> 
> This happens with the patch I sent earlier.  It just removes one of
> mark_referenced calls in the C++ backend.  There are 4 such calls and I
> am unsure if they are actaully needed or not.
> I am seeking for testcase exhibiting that mark_referenced at that place
> is needed.  (I've originally replaced them from the
> TREE_SYMBOL_REFERENCED and didn't really checked it without the change).

Thanks for your explanation.

I agree that it does not make sense for use_thunk to call 
"mark_referenced" on the function to which the thunk transfers control 
in the "emit_p == 0" case.

That should only happen when the thunk is actually output, i.e, in the 
"emit_p == 1" case, because in that case the thunk has been directly 
generated into the assembly file and the function really is needed. 
(Eventually, it would be better to move that under the control of 
cgraph, but that is probably too invasive a change right now.)

So, if you make that change does it fix the problem?

> How much incredible it is?  (ie having testcase that exhibits a lot of
> unnecesary thunks/vtables would be usefull too). 

Just take the original test case, but add thousands of classes.  That's 
a very typical situation for some class frameworks.



-- 


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

[Bug c++/14639] [3.4/3.5 Regression] [unit-at-a-time] Incorrect emission of unused compiler-generated destructor at -O2

[hubicka at ucw dot cz]

------- Additional Comments From hubicka at ucw dot cz  2004-03-24 23:51 -------
Subject: Re:  [3.4/3.5 Regression] [unit-at-a-time] Incorrect emission of unused compiler-generated destructor at -O2

> 
> ------- Additional Comments From mark at codesourcery dot com  2004-03-24 18:44 -------
> Subject: Re:  [3.4/3.5 Regression] [unit-at-a-time] Incorrect
>  emission of unused compiler-generated destructor at -O2
> 
> >>In the test case, the object file should be empty.  There are no 
> >>definitions of any functions except for the inline xyz::xyz method, and 
> >>since it is inline it will not be emitted.
> > 
> > 
> > This happens with the patch I sent earlier.  It just removes one of
> > mark_referenced calls in the C++ backend.  There are 4 such calls and I
> > am unsure if they are actaully needed or not.
> > I am seeking for testcase exhibiting that mark_referenced at that place
> > is needed.  (I've originally replaced them from the
> > TREE_SYMBOL_REFERENCED and didn't really checked it without the change).
> 
> Thanks for your explanation.
> 
> I agree that it does not make sense for use_thunk to call 
> "mark_referenced" on the function to which the thunk transfers control 
> in the "emit_p == 0" case.
> 
> That should only happen when the thunk is actually output, i.e, in the 
> "emit_p == 1" case, because in that case the thunk has been directly 
> generated into the assembly file and the function really is needed. 
> (Eventually, it would be better to move that under the control of 
> cgraph, but that is probably too invasive a change right now.)
> 
> So, if you make that change does it fix the problem?

It will fix the problem because the thunk is only output at the time
function is assembled.  But it will make mark_referenced useless anyway
as when the function is assembled it is already mark_reference-d.
Removing the call as done by my patch should be fine then, but we must
ensure that referencing the vtable that references the thunk makes
cgraph code notice that the function referenced by the thunk is
referenced too, so the dependencies goes right.  I am not quite sure how
to construct testcase, but I will try to dig into it.  Obviously the
testsuite works so it seems to be fine in majority of case but my
experience is that there are common dead ends not covered by testsuite
in these areas, so I still feel unsafe about that patch, but all my
attempts to construct testcase seems to be failing.
> 
> > How much incredible it is?  (ie having testcase that exhibits a lot of
> > unnecesary thunks/vtables would be usefull too). 
> 
> Just take the original test case, but add thousands of classes.  That's 
> a very typical situation for some class frameworks.

We obvisouly emit unnecesary thunks, lets see what we can do about this.
I tried to compile mozilla with the patch fixing the testcase and
measured no difference in code size tought...

Honza
> 
> 
> 
> -- 
> 
> 
> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=14639
> 
> ------- You are receiving this mail because: -------
> You are on the CC list for the bug, or are watching someone who is.


-- 


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

[Bug c++/14639] [3.4/3.5 Regression] [unit-at-a-time] Incorrect emission of unused compiler-generated destructor at -O2

[mark at codesourcery dot com]

------- Additional Comments From mark at codesourcery dot com  2004-03-26 08:23 -------
Subject: Re:  [3.4/3.5 Regression] [unit-at-a-time] Incorrect
 emission of unused compiler-generated destructor at -O2

hubicka at ucw dot cz wrote:

> It will fix the problem because the thunk is only output at the time
> function is assembled.  But it will make mark_referenced useless anyway
> as when the function is assembled it is already mark_reference-d.
> Removing the call as done by my patch should be fine then, but we must
> ensure that referencing the vtable that references the thunk makes
> cgraph code notice that the function referenced by the thunk is
> referenced too, so the dependencies goes right.  I am not quite sure how
> to construct testcase, but I will try to dig into it.  Obviously the
> testsuite works so it seems to be fine in majority of case but my
> experience is that there are common dead ends not covered by testsuite
> in these areas, so I still feel unsafe about that patch, but all my
> attempts to construct testcase seems to be failing.

I am not sure that the case you are worried about can happen.

For that to happen, there would have to be a vtable that contains a 
reference to a thunk, but no references to the function to which the 
thunk refers.  The only way a thunk to C::f can occur is in a vtable for 
  C, or a base subobject of C.  But, in that case, C::f will also be 
referenced directly from the vtable.

>>>How much incredible it is?  (ie having testcase that exhibits a lot of
>>>unnecesary thunks/vtables would be usefull too). 
>>
>>Just take the original test case, but add thousands of classes.  That's 
>>a very typical situation for some class frameworks.
> 
> 
> We obvisouly emit unnecesary thunks, lets see what we can do about this.
> I tried to compile mozilla with the patch fixing the testcase and
> measured no difference in code size tought...

Many of the "unncessary" thunks are required by the ABI.  The ABI 
guarantees that all thunks are put out with the function to which they 
transfer control.

Based on our discussion, I think your patch is OK.  Please check it in, 
but remove the call to mark_referenced copmletely, rather than 
commenting it out.

Thanks,



-- 


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

[Bug c++/14639] [3.4/3.5 Regression] [unit-at-a-time] Incorrect emission of unused compiler-generated destructor at -O2

[hubicka at ucw dot cz]

------- Additional Comments From hubicka at ucw dot cz  2004-03-26 10:01 -------
Subject: Re:  [3.4/3.5 Regression] [unit-at-a-time] Incorrect emission of unused compiler-generated destructor at -O2

> 
> ------- Additional Comments From mark at codesourcery dot com  2004-03-26 08:23 -------
> Subject: Re:  [3.4/3.5 Regression] [unit-at-a-time] Incorrect
>  emission of unused compiler-generated destructor at -O2
> 
> hubicka at ucw dot cz wrote:
> 
> > It will fix the problem because the thunk is only output at the time
> > function is assembled.  But it will make mark_referenced useless anyway
> > as when the function is assembled it is already mark_reference-d.
> > Removing the call as done by my patch should be fine then, but we must
> > ensure that referencing the vtable that references the thunk makes
> > cgraph code notice that the function referenced by the thunk is
> > referenced too, so the dependencies goes right.  I am not quite sure how
> > to construct testcase, but I will try to dig into it.  Obviously the
> > testsuite works so it seems to be fine in majority of case but my
> > experience is that there are common dead ends not covered by testsuite
> > in these areas, so I still feel unsafe about that patch, but all my
> > attempts to construct testcase seems to be failing.
> 
> I am not sure that the case you are worried about can happen.
> 
> For that to happen, there would have to be a vtable that contains a 
> reference to a thunk, but no references to the function to which the 
> thunk refers.  The only way a thunk to C::f can occur is in a vtable for 
>   C, or a base subobject of C.  But, in that case, C::f will also be 
> referenced directly from the vtable.
> 
> >>>How much incredible it is?  (ie having testcase that exhibits a lot of
> >>>unnecesary thunks/vtables would be usefull too). 
> >>
> >>Just take the original test case, but add thousands of classes.  That's 
> >>a very typical situation for some class frameworks.
> > 
> > 
> > We obvisouly emit unnecesary thunks, lets see what we can do about this.
> > I tried to compile mozilla with the patch fixing the testcase and
> > measured no difference in code size tought...
> 
> Many of the "unncessary" thunks are required by the ABI.  The ABI 
> guarantees that all thunks are put out with the function to which they 
> transfer control.
> 
> Based on our discussion, I think your patch is OK.  Please check it in, 
> but remove the call to mark_referenced copmletely, rather than 
> commenting it out.

OK, thanks!  I will try to investigate the remaining 3 calls if they can
cause similar showstopper too.

Honza
> 
> Thanks,
> 
> 
> 
> -- 
> 
> 
> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=14639
> 
> ------- You are receiving this mail because: -------
> You are on the CC list for the bug, or are watching someone who is.


-- 


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

[Bug c++/14639] [3.4/3.5 Regression] [unit-at-a-time] Incorrect emission of unused compiler-generated destructor at -O2

[cvs-commit at gcc dot gnu dot org]

------- Additional Comments From cvs-commit at gcc dot gnu dot org  2004-03-28 15:42 -------
Subject: Bug 14639

CVSROOT:	/cvs/gcc
Module name:	gcc
Branch: 	gcc-3_4-branch
Changes by:	hubicka@gcc.gnu.org	2004-03-28 15:41:57

Modified files:
	gcc/cp         : ChangeLog method.c 

Log message:
	PR C++/14639
	* method.c (use_think): Do not mark thunk as referenced.

Patches:
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/cp/ChangeLog.diff?cvsroot=gcc&only_with_tag=gcc-3_4-branch&r1=1.3892.2.88&r2=1.3892.2.89
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/cp/method.c.diff?cvsroot=gcc&only_with_tag=gcc-3_4-branch&r1=1.275.4.1&r2=1.275.4.2



-- 


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

[Bug c++/14639] [3.5 Regression] [unit-at-a-time] Incorrect emission of unused compiler-generated destructor at -O2

[pinskia at gcc dot gnu dot org]

-- 
           What    |Removed                     |Added
----------------------------------------------------------------------------
      Known to fail|3.4.0 3.5.0                 |3.5.0
      Known to work|3.3                         |3.3 3.4.0
            Summary|[3.4/3.5 Regression] [unit- |[3.5 Regression] [unit-at-a-
                   |at-a-time] Incorrect        |time] Incorrect emission of
                   |emission of unused compiler-|unused compiler-generated
                   |generated destructor at -O2 |destructor at -O2
   Target Milestone|3.4.0                       |3.5.0


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

[Bug c++/14639] [3.5 Regression] [unit-at-a-time] Incorrect emission of unused compiler-generated destructor at -O2

[bowdidge at apple dot com]

------- Additional Comments From bowdidge at apple dot com  2004-06-03 23:27 -------
This bug still keeps parts of Qt from building on Darwin.  

The problem got worse after the tree-ssa 
merge.  Before the merge, the unused symbols would get generated if the code fragment were 
compiled at -O2; the symbols wouldn't be generated if -fno-unit-at-a-time was added to the compile 
line.  With the current top-of-tree, the symbols are always produced -- -O0, -O2, and with or without 
-fno-unit-at-a-time.

-- 


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

[Bug c++/14639] [3.5 Regression] [unit-at-a-time] Incorrect emission of unused compiler-generated destructor at -O2

[pinskia at gcc dot gnu dot org]

------- Additional Comments From pinskia at gcc dot gnu dot org  2004-06-03 23:32 -------
Actually it was the merge which changed how unit-at-a-time is done but rather Zack's patch which 
removed a large number of setting TREE_SYMBOL_REFERENCED.

-- 
           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |pinskia at gcc dot gnu dot
                   |                            |org


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

[Bug c++/14639] [3.5 Regression] [unit-at-a-time] Incorrect emission of unused compiler-generated destructor at -O2

[pinskia at gcc dot gnu dot org]

------- Additional Comments From pinskia at gcc dot gnu dot org  2004-06-03 23:35 -------
Here is the patch for 3.5.0 after Zack's patch:
Index: method.c
===============================================================
====
RCS file: /cvs/gcc/gcc/gcc/cp/method.c,v
retrieving revision 1.283
diff -u -p -r1.283 method.c
--- method.c    24 May 2004 21:07:42 -0000      1.283
+++ method.c    3 Jun 2004 23:34:11 -0000
@@ -353,7 +353,6 @@ use_thunk (tree thunk_fndecl, bool emit_
      this translation unit.  */
   TREE_ADDRESSABLE (function) = 1;
   mark_used (function);
-  mark_decl_referenced (function);
   if (!emit_p)
     return;

Should I apply this Jan/Mark?

-- 
           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |ABI


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

[Bug c++/14639] [3.5 Regression] [unit-at-a-time] Incorrect emission of unused compiler-generated destructor at -O2

[bowdidge at apple dot com]

------- Additional Comments From bowdidge at apple dot com  2004-06-03 23:38 -------
That makes sense.  I hadn't seen the problem until recently.

-- 


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

[Bug c++/14639] [3.5 Regression] [unit-at-a-time] Incorrect emission of unused compiler-generated destructor at -O2

[mark at codesourcery dot com]

------- Additional Comments From mark at codesourcery dot com  2004-06-03 23:38 -------
Subject: Re:  [3.5 Regression] [unit-at-a-time] Incorrect emission
 of unused compiler-generated destructor at -O2

pinskia at gcc dot gnu dot org wrote:

> ------- Additional Comments From pinskia at gcc dot gnu dot org  2004-06-03 23:35 -------
> Here is the patch for 3.5.0 after Zack's patch:
> Index: method.c
> ===============================================================
> ====
> RCS file: /cvs/gcc/gcc/gcc/cp/method.c,v
> retrieving revision 1.283
> diff -u -p -r1.283 method.c
> --- method.c    24 May 2004 21:07:42 -0000      1.283
> +++ method.c    3 Jun 2004 23:34:11 -0000
> @@ -353,7 +353,6 @@ use_thunk (tree thunk_fndecl, bool emit_
>       this translation unit.  */
>    TREE_ADDRESSABLE (function) = 1;
>    mark_used (function);
> -  mark_decl_referenced (function);
>    if (!emit_p)
>      return;
> 
> Should I apply this Jan/Mark?

If it passes tests, yes, please.



-- 


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

[Bug c++/14639] [3.5 Regression] [unit-at-a-time] Incorrect emission of unused compiler-generated destructor at -O2

[cvs-commit at gcc dot gnu dot org]

------- Additional Comments From cvs-commit at gcc dot gnu dot org  2004-06-03 23:45 -------
Subject: Bug 14639

CVSROOT:	/cvs/gcc
Module name:	gcc
Changes by:	pinskia@gcc.gnu.org	2004-06-03 23:45:46

Modified files:
	gcc/cp         : ChangeLog method.c 

Log message:
	2004-06-03  Andrew Pinski  <pinskia@physics.uc.edu>
	Jan Hubicka  <jh@suse.cz>
	
	PR c++/14639
	* method.c (use_think): Do not mark thunk as referenced.

Patches:
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/cp/ChangeLog.diff?cvsroot=gcc&r1=1.4080&r2=1.4081
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/cp/method.c.diff?cvsroot=gcc&r1=1.283&r2=1.284



-- 


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

[Bug c++/14639] [3.5 Regression] [unit-at-a-time] Incorrect emission of unused compiler-generated destructor at -O2

[pinskia at gcc dot gnu dot org]

------- Additional Comments From pinskia at gcc dot gnu dot org  2004-06-03 23:46 -------
Fixed.

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


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

[Bug c++/14639] [3.5 Regression] [unit-at-a-time] Incorrect emission of unused compiler-generated destructor at -O2

[bowdidge at apple dot com]

------- Additional Comments From bowdidge at apple dot com  2004-06-04 00:37 -------
After applying Andrew's fix, compiling Matt's fragments at the top of the radar with gcc -c  and 
checking symbols in the resulting .o file showed:

With -O2, has unnecessary symbols.
With -O0, the unnecessary symbols ARE generated.
With -O2 -fno-unit-at-a-time, the unnecessar

-- 


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

[Bug c++/14639] [3.5 Regression] [unit-at-a-time] Incorrect emission of unused compiler-generated destructor at -O2

[bowdidge at apple dot com]

------- Additional Comments From bowdidge at apple dot com  2004-06-04 00:38 -------
After applying Andrew's fix, compiling Matt's fragments at the top of the radar with gcc -c  and 
checking symbols in the resulting .o file showed:

With -O2: does not generate unnecessary symbols.
With -O0, generates unnecessary symbols.
With -O2, generates unnecessary symbols
With -O0, generates unnecessary symbols

In addition, Qt still fails to build.  

-- 


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

[Bug c++/14639] [3.5 Regression] [unit-at-a-time] Incorrect emission of unused compiler-generated destructor at -O2

[pinskia at gcc dot gnu dot org]

------- Additional Comments From pinskia at gcc dot gnu dot org  2004-06-04 00:40 -------
Reopening for now.

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


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

[Bug c++/14639] [3.5 Regression] [non-unit-at-a-time] Incorrect emission of unused compiler-generated destructor at -O2

[pinskia at gcc dot gnu dot org]

------- Additional Comments From pinskia at gcc dot gnu dot org  2004-06-04 00:50 -------
Ok this now only happens with -fno-unit-at-a-time, investigating.

-- 
           What    |Removed                     |Added
----------------------------------------------------------------------------
         AssignedTo|unassigned at gcc dot gnu   |pinskia at gcc dot gnu dot
                   |dot org                     |org
             Status|REOPENED                    |ASSIGNED
            Summary|[3.5 Regression] [unit-at-a-|[3.5 Regression] [non-unit-
                   |time] Incorrect emission of |at-a-time] Incorrect
                   |unused compiler-generated   |emission of unused compiler-
                   |destructor at -O2           |generated destructor at -O2


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

[Bug c++/14639] [3.5 Regression] [non-unit-at-a-time] Incorrect emission of unused compiler-generated destructor

[bowdidge at apple dot com]

------- Additional Comments From bowdidge at apple dot com  2004-06-04 00:58 -------
For completeness, my report should have said:
With -O2: does not generate unnecessary symbols.
With -O0, generates unnecessary symbols.
With -O2 -fno-unit-at-a-time, generates unnecessary symbols
With -O0 -fno-unit-at-a-time, generates unnecessary symbols

So with Andrew's first patch -O2 with unit-at-a-time worked, but all other conditions failed.

-- 


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

[Bug c++/14639] [3.5 Regression] [non-unit-at-a-time] Incorrect emission of unused compiler-generated destructor

[pinskia at gcc dot gnu dot org]

------- Additional Comments From pinskia at gcc dot gnu dot org  2004-06-04 01:09 -------
Now I really wish non unit-at-a-time goes away.

-- 


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

[Bug c++/14639] [3.5 Regression] [non-unit-at-a-time] Incorrect emission of unused compiler-generated destructor

[pinskia at gcc dot gnu dot org]

------- Additional Comments From pinskia at gcc dot gnu dot org  2004-06-04 01:50 -------
This should fix it by making non-unit-at-a-time more like unit-at-a-time also removes one ??? to fix 
the rest of the way because now we mark the variables when outputing the asm so there is no need to 
do it while analyzing the function.  I have not tested or bootstrapped this yet, Robert could you test this 
as I have a test to study for/do?

Index: passes.c
===============================================================
====
RCS file: /cvs/gcc/gcc/gcc/passes.c,v
retrieving revision 2.17
diff -u -p -r2.17 passes.c
--- passes.c	30 May 2004 18:32:27 -0000	2.17
+++ passes.c	4 Jun 2004 01:45:17 -0000
@@ -370,7 +370,7 @@ rest_of_decl_compilation (tree decl,
 	   || (flag_unit_at_a_time && DECL_INITIAL (decl)))
 	  && !DECL_EXTERNAL (decl))
 	{
-	  if (flag_unit_at_a_time && !cgraph_global_info_ready
+	  if (!cgraph_global_info_ready
 	      && TREE_CODE (decl) != FUNCTION_DECL && top_level)
 	    cgraph_varpool_finalize_decl (decl);
 	  else
Index: cgraphunit.c
===============================================================
====
RCS file: /cvs/gcc/gcc/gcc/cgraphunit.c,v
retrieving revision 1.64
diff -u -p -r1.64 cgraphunit.c
--- cgraphunit.c	30 May 2004 18:32:24 -0000	1.64
+++ cgraphunit.c	4 Jun 2004 01:45:17 -0000
@@ -395,11 +395,6 @@ record_call_1 (tree *tp, int *walk_subtr
   switch (TREE_CODE (t))
     {
     case VAR_DECL:
-      /* ??? Really, we should mark this decl as *potentially* referenced
-	 by this function and re-examine whether the decl is actually used
-	 after rtl has been generated.  */
-      if (TREE_STATIC (t))
-        cgraph_varpool_mark_needed_node (cgraph_varpool_node (t));
       break;
 
     case ADDR_EXPR:


-- 


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

[Bug c++/14639] [3.5 Regression] [non-unit-at-a-time] Incorrect emission of unused compiler-generated destructor

[bowdidge at apple dot com]

------- Additional Comments From bowdidge at apple dot com  2004-06-04 02:12 -------
The test case passes fine, but the first compilation in the Qt installation gets stuck in an infinite loop in 
finish_file.  The first part of the patch (in passes.c) is in rest_of_decl_compilation, so I'd guess that's the 
cause.   I'll try backing out that change, and see what happens.

Here's part of the sample:

    248 Thread_110b
      248 start
        248 _start
          248 toplev_main
            248 do_compile
              248 compile_file
                248 c_common_parse_file
                  248 finish_file
                    248 walk_namespaces_r
                      239 wrapup_global_declarations
                        150 wrapup_global_declarations
                        72 rest_of_decl_compilation
                          39 cgraph_varpool_finalize_decl
                            25 cgraph_varpool_node
                              13 htab_find_slot_with_hash
                                11 htab_find_slot_with_hash
                                2 eq_varpool_node
                                  2 eq_varpool_node
                              7 cgraph_varpool_node
                              3 htab_find_slot
                                3 htab_find_slot


-- 


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

[Bug c++/14639] [3.5 Regression] [non-unit-at-a-time] Incorrect emission of unused compiler-generated destructor

[pinskia at gcc dot gnu dot org]

------- Additional Comments From pinskia at gcc dot gnu dot org  2004-06-04 02:18 -------
Can you try with my patch to fix PR 15721: <http://gcc.gnu.org/ml/gcc-patches/2004-06/
msg00088.html> to see if that helps, most likely what is going on now is that the 
wrapup_global_declarations is not being smart as it should be.

-- 


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

[Bug c++/14639] [3.5 Regression] [non-unit-at-a-time] Incorrect emission of unused compiler-generated destructor

[pinskia at gcc dot gnu dot org]

------- Additional Comments From pinskia at gcc dot gnu dot org  2004-06-04 02:20 -------
I should note that the my patch to toplevel.c is needed because that is where the outputing is 
happening in first place.

-- 


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

[Bug c++/14639] [3.5 Regression] [non-unit-at-a-time] Incorrect emission of unused compiler-generated destructor

[pinskia at gcc dot gnu dot org]

-- 
           What    |Removed                     |Added
----------------------------------------------------------------------------
  BugsThisDependsOn|                            |15816


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

[Bug c++/14639] [3.5 Regression] [non-unit-at-a-time] Incorrect emission of unused compiler-generated destructor

[bowdidge at apple dot com]

------- Additional Comments From bowdidge at apple dot com  2004-06-04 03:45 -------
I applied the three patches suggested to a top-of-tree compiler -- the changes to method.c, the 
changes to passes.c and toplev.c, and the changes included in PR/15721.  Qt fails to link when trying to 
compile moc, the first executable compiled.  The failure shows many more symbols missing than before 
patching. 

On the bright side, the small test case attached to the bug still shows no symbols being output (as 
expected.) I did not attempt to run the testsuite on it or bootstrap.


-- 


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

[Bug c++/14639] [3.5 Regression] [non-unit-at-a-time] Incorrect emission of unused compiler-generated destructor

[bowdidge at apple dot com]

------- Additional Comments From bowdidge at apple dot com  2004-06-04 04:53 -------
The reappearance of the bug may have appeared after 5/27/04 because it doesn't appear on the lno-
branch.  The lno-branch merged with mainline around then.  

-- 


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

[Bug c++/14639] [3.5 Regression] [non-unit-at-a-time] Incorrect emission of unused compiler-generated destructor

[pinskia at gcc dot gnu dot org]

------- Additional Comments From pinskia at gcc dot gnu dot org  2004-06-04 04:58 -------
The only patch which changed the C++ front-end was mine which changed how to check if a decl is 
needed or not :(  I still think non-unit-at-a-time is should go away for 3.5.0.

-- 


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

[Bug c++/14639] [3.5 Regression] [non-unit-at-a-time] Incorrect emission of unused compiler-generated destructor

[bowdidge at apple dot com]

------- Additional Comments From bowdidge at apple dot com  2004-06-04 19:13 -------
I don't particularly care about -fno-unit-at-a-time; we were using it to workaround existing bugs.  As 
long as the default behavior produces appropriate symbols, I don't care what happens to -fno-unit-at-
a-time.

-- 


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

[Bug c++/14639] [3.5 Regression] [non-unit-at-a-time] Incorrect emission of unused compiler-generated destructor

[hubicka at ucw dot cz]

------- Additional Comments From hubicka at ucw dot cz  2004-06-04 21:04 -------
Subject: Re:  [3.5 Regression] [unit-at-a-time] Incorrect emission of unused compiler-generated destructor at -O2

> 
> ------- Additional Comments From pinskia at gcc dot gnu dot org  2004-06-03 23:35 -------
> Here is the patch for 3.5.0 after Zack's patch:
> Index: method.c
> ===============================================================
> ====
> RCS file: /cvs/gcc/gcc/gcc/cp/method.c,v
> retrieving revision 1.283
> diff -u -p -r1.283 method.c
> --- method.c    24 May 2004 21:07:42 -0000      1.283
> +++ method.c    3 Jun 2004 23:34:11 -0000
> @@ -353,7 +353,6 @@ use_thunk (tree thunk_fndecl, bool emit_
>       this translation unit.  */
>    TREE_ADDRESSABLE (function) = 1;
>    mark_used (function);
> -  mark_decl_referenced (function);
>    if (!emit_p)
>      return;
> 
> Should I apply this Jan/Mark?

This is fine with me, but Mark approved the patch originally only for
3.4 (that is why i didn't applied it to mainline) but perhaps he meant
both? :)

Honza
> 
> -- 
>            What    |Removed                     |Added
> ----------------------------------------------------------------------------
>            Keywords|                            |ABI
> 
> 
> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=14639
> 
> ------- You are receiving this mail because: -------
> You are on the CC list for the bug, or are watching someone who is.


-- 


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

[Bug c++/14639] [3.5 Regression] [non-unit-at-a-time] Incorrect emission of unused compiler-generated destructor

[hubicka at ucw dot cz]

------- Additional Comments From hubicka at ucw dot cz  2004-06-04 21:11 -------
Subject: Re:  [3.5 Regression] [non-unit-at-a-time] Incorrect emission of unused compiler-generated destructor

> 
> ------- Additional Comments From pinskia at gcc dot gnu dot org  2004-06-04 01:50 -------
> This should fix it by making non-unit-at-a-time more like unit-at-a-time also removes one ??? to fix 
> the rest of the way because now we mark the variables when outputing the asm so there is no need to 
> do it while analyzing the function.  I have not tested or bootstrapped this yet, Robert could you test this 
> as I have a test to study for/do?
> 
> Index: passes.c
> ===============================================================
> ====
> RCS file: /cvs/gcc/gcc/gcc/passes.c,v
> retrieving revision 2.17
> diff -u -p -r2.17 passes.c
> --- passes.c	30 May 2004 18:32:27 -0000	2.17
> +++ passes.c	4 Jun 2004 01:45:17 -0000
> @@ -370,7 +370,7 @@ rest_of_decl_compilation (tree decl,
>  	   || (flag_unit_at_a_time && DECL_INITIAL (decl)))
>  	  && !DECL_EXTERNAL (decl))
>  	{
> -	  if (flag_unit_at_a_time && !cgraph_global_info_ready
> +	  if (!cgraph_global_info_ready
>  	      && TREE_CODE (decl) != FUNCTION_DECL && top_level)
>  	    cgraph_varpool_finalize_decl (decl);
>  	  else
> Index: cgraphunit.c
> ===============================================================
> ====
> RCS file: /cvs/gcc/gcc/gcc/cgraphunit.c,v
> retrieving revision 1.64
> diff -u -p -r1.64 cgraphunit.c
> --- cgraphunit.c	30 May 2004 18:32:24 -0000	1.64
> +++ cgraphunit.c	4 Jun 2004 01:45:17 -0000
> @@ -395,11 +395,6 @@ record_call_1 (tree *tp, int *walk_subtr
>    switch (TREE_CODE (t))
>      {
>      case VAR_DECL:
> -      /* ??? Really, we should mark this decl as *potentially* referenced
> -	 by this function and re-examine whether the decl is actually used
> -	 after rtl has been generated.  */
> -      if (TREE_STATIC (t))
> -        cgraph_varpool_mark_needed_node (cgraph_varpool_node (t));

The mark call here must stay (it is the only place we do the
reachability analysis in the case we will drop it here compiler will
break at least if we get past the callback from the asm outputting code
(I am currently testing patch to do it but it has problems still...)

The "needed" in cgraph sense is "conservatively needed" so the Richard
comment should be probably placed elsewhere as this is defect of
callgraph code affecting both functions and variables.  The problem is
that at the moment cgraph code is trusting to "needed" flag for
variables and output them early instead of later pass really checking
whether they are really needed (ie still referenced by the optimized
body of function).  The reason why it is done this way is that I still
rely on backend calling me back when variable is needed as it is quite
non-trivial to do the reachability analysis on all kinds of
datastructures output by the front-end.  I am working on that as we
speak, but it will take time (I just hit problems with Java and
objective C bypassing the cgraph as I feared)

Honza
>        break;
>  
>      case ADDR_EXPR:
> 
> 
> -- 
> 
> 
> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=14639
> 
> ------- You are receiving this mail because: -------
> You are on the CC list for the bug, or are watching someone who is.


-- 


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

[Bug c++/14639] [3.5 Regression] [non-unit-at-a-time] Incorrect emission of unused compiler-generated destructor

[bowdidge at apple dot com]

------- Additional Comments From bowdidge at apple dot com  2004-06-04 21:15 -------
I just tried the code fragment on a mainline build for 2004-05-26 07:00, and I get the behavior I'd seen 
for the last month -- things break with -O2, but -fno-unit-at-a-time makes things work right.  This 
suggests something happened between 5/26 and today that triggered this specific problem.  I'll see if I 
can further reduce where the problem appeared.

-- 


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

[Bug c++/14639] [3.5 Regression] [non-unit-at-a-time] Incorrect emission of unused compiler-generated destructor

[hubicka at ucw dot cz]

------- Additional Comments From hubicka at ucw dot cz  2004-06-04 21:16 -------
Subject: Re:  [3.5 Regression] [non-unit-at-a-time] Incorrect emission of unused compiler-generated destructor

> 
> ------- Additional Comments From hubicka at ucw dot cz  2004-06-04 21:11 -------
> Subject: Re:  [3.5 Regression] [non-unit-at-a-time] Incorrect emission of unused compiler-generated destructor
> 
> > 
> > ------- Additional Comments From pinskia at gcc dot gnu dot org  2004-06-04 01:50 -------
> > This should fix it by making non-unit-at-a-time more like unit-at-a-time also removes one ??? to fix 
> > the rest of the way because now we mark the variables when outputing the asm so there is no need to 
> > do it while analyzing the function.  I have not tested or bootstrapped this yet, Robert could you test this 
> > as I have a test to study for/do?
> > 
> > Index: passes.c
> > ===============================================================
> > ====
> > RCS file: /cvs/gcc/gcc/gcc/passes.c,v
> > retrieving revision 2.17
> > diff -u -p -r2.17 passes.c
> > --- passes.c	30 May 2004 18:32:27 -0000	2.17
> > +++ passes.c	4 Jun 2004 01:45:17 -0000
> > @@ -370,7 +370,7 @@ rest_of_decl_compilation (tree decl,
> >  	   || (flag_unit_at_a_time && DECL_INITIAL (decl)))
> >  	  && !DECL_EXTERNAL (decl))
> >  	{
> > -	  if (flag_unit_at_a_time && !cgraph_global_info_ready
> > +	  if (!cgraph_global_info_ready
> >  	      && TREE_CODE (decl) != FUNCTION_DECL && top_level)
> >  	    cgraph_varpool_finalize_decl (decl);
> >  	  else
> > Index: cgraphunit.c
> > ===============================================================
> > ====
> > RCS file: /cvs/gcc/gcc/gcc/cgraphunit.c,v
> > retrieving revision 1.64
> > diff -u -p -r1.64 cgraphunit.c
> > --- cgraphunit.c	30 May 2004 18:32:24 -0000	1.64
> > +++ cgraphunit.c	4 Jun 2004 01:45:17 -0000
> > @@ -395,11 +395,6 @@ record_call_1 (tree *tp, int *walk_subtr
> >    switch (TREE_CODE (t))
> >      {
> >      case VAR_DECL:
> > -      /* ??? Really, we should mark this decl as *potentially* referenced
> > -	 by this function and re-examine whether the decl is actually used
> > -	 after rtl has been generated.  */
> > -      if (TREE_STATIC (t))
> > -        cgraph_varpool_mark_needed_node (cgraph_varpool_node (t));
> 
> The mark call here must stay (it is the only place we do the
> reachability analysis in the case we will drop it here compiler will
> break at least if we get past the callback from the asm outputting code
> (I am currently testing patch to do it but it has problems still...)
> 
> The "needed" in cgraph sense is "conservatively needed" so the Richard
> comment should be probably placed elsewhere as this is defect of
> callgraph code affecting both functions and variables.  The problem is
> that at the moment cgraph code is trusting to "needed" flag for
> variables and output them early instead of later pass really checking
> whether they are really needed (ie still referenced by the optimized
> body of function).  The reason why it is done this way is that I still
> rely on backend calling me back when variable is needed as it is quite
> non-trivial to do the reachability analysis on all kinds of
> datastructures output by the front-end.  I am working on that as we
> speak, but it will take time (I just hit problems with Java and
> objective C bypassing the cgraph as I feared)

OK, while writing the long philophical reply I forgot to ask.  Steven,
did you tracked down what is going wrong?
We might get around by not marking functions as needed in
non-unit-at-a-time but this is also going to break with changes I want
to do, so I would rather preffer to have some better sollution.

Honza
> 
> Honza
> >        break;
> >  
> >      case ADDR_EXPR:
> > 
> > 
> > -- 
> > 
> > 
> > http://gcc.gnu.org/bugzilla/show_bug.cgi?id=14639
> > 
> > ------- You are receiving this mail because: -------
> > You are on the CC list for the bug, or are watching someone who is.
> 
> 
> -- 
> 
> 
> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=14639
> 
> ------- You are receiving this mail because: -------
> You are on the CC list for the bug, or are watching someone who is.


-- 


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

[Bug c++/14639] [3.5 Regression] [non-unit-at-a-time] Incorrect emission of unused compiler-generated destructor

[bowdidge at apple dot com]

------- Additional Comments From bowdidge at apple dot com  2004-06-04 21:53 -------
Problem also doesn't appear on 2004-06-01 version of mainline.

-- 


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

[Bug c++/14639] [3.5 Regression] [non-unit-at-a-time] Incorrect emission of unused compiler-generated destructor

[bowdidge at apple dot com]

------- Additional Comments From bowdidge at apple dot com  2004-06-04 22:59 -------
Problem does appear on 2004-06-03 version of mainline.

-- 


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

[Bug c++/14639] [3.5 Regression] [non-unit-at-a-time] Incorrect emission of unused compiler-generated destructor

[bowdidge at apple dot com]

------- Additional Comments From bowdidge at apple dot com  2004-06-05 00:58 -------
The problem appears to be with one of these two patches to cp-tree.h.  Checking out mainline from 
0602 20:30 causes the problem to appear; reverting cp-tree back to version 1.970 cures 
the problem.

revision 1.972
date: 2004/06/02 20:24:30;  author: pinskia;  state: Exp;  lines: +1 -1
2004-06-02  Andrew Pinski  <pinskia@physics.uc.edu>

        * cp-tree.h: Fix typo.
----------------------------
revision 1.971
date: 2004/06/02 19:20:03;  author: pinskia;  state: Exp;  lines: +5 -2
2004-06-02  Andrew Pinski  <pinskia@physics.uc.edu>

        * cp-tree.h: Include cgraph.h
        (DECL_NEEDED_P): Use cgraph_*node on the decl instead of
        TREE_SYMBOL_REFERENCED on the DECL_ASSEMBLER_NAME of the decl.




-- 


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

[Bug c++/14639] [3.5 Regression] [non-unit-at-a-time] Incorrect emission of unused compiler-generated destructor

[bowdidge at apple dot com]

------- Additional Comments From bowdidge at apple dot com  2004-06-05 01:02 -------
I'd be happy either getting the bug fixed or the offending changes reverted.  The situation before 5/29 
at least had a workaround (-fno-unit-at-a-time), enabling us to test that the rest of Qt could build and 
do timing measurements of 3.5.

-- 


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

[Bug c++/14639] [3.5 Regression] [non-unit-at-a-time] Incorrect emission of unused compiler-generated destructor

[pinskia at gcc dot gnu dot org]

------- Additional Comments From pinskia at gcc dot gnu dot org  2004-06-05 01:19 -------
No that should not be reverted just yet, there is something else wrong, I think the decl is being marked 
too soon as needed as shown by my last patch.  Support two ways of figuring out if a decl is need is too 
much a maintainiablity nightmare which is the current situation is which Zack is trying to remove in his 
patches and I am just helping doing and also speed up the compiler at the same time by removing calls 
to decl_assembler_name (DECL_ASSEMBLER_NAME).

-- 


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

[Bug c++/14639] [3.5 Regression] [non-unit-at-a-time] Incorrect emission of unused compiler-generated destructor

[pinskia at gcc dot gnu dot org]

-- 
           What    |Removed                     |Added
----------------------------------------------------------------------------
           Severity|normal                      |critical


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

[Bug c++/14639] [3.5 Regression] [non-unit-at-a-time] Incorrect emission of unused compiler-generated destructor

[pinskia at gcc dot gnu dot org]

------- Additional Comments From pinskia at gcc dot gnu dot org  2004-06-15 03:43 -------
Patch posted here: <http://gcc.gnu.org/ml/gcc-patches/2004-06/msg01094.html>.

-- 
           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |patch, wrong-code


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

[Bug c++/14639] [3.5 Regression] [non-unit-at-a-time] Incorrect emission of unused compiler-generated destructor

[pinskia at gcc dot gnu dot org]

------- Additional Comments From pinskia at gcc dot gnu dot org  2004-06-15 20:52 -------
Fixed.

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


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

[Bug c++/14639] [3.5 Regression] [non-unit-at-a-time] Incorrect emission of unused compiler-generated destructor

[cvs-commit at gcc dot gnu dot org]

------- Additional Comments From cvs-commit at gcc dot gnu dot org  2004-06-15 20:52 -------
Subject: Bug 14639

CVSROOT:	/cvs/gcc
Module name:	gcc
Changes by:	pinskia@gcc.gnu.org	2004-06-15 20:52:01

Modified files:
	gcc/cp         : ChangeLog cp-tree.h 

Log message:
	2004-06-12  Andrew Pinski  <apinski@apple.com>
	
	PR c++/14639
	Revert:
	2004-06-02  Andrew Pinski  <pinskia@physics.uc.edu>
	
	* cp-tree.h: Fix typo.
	
	* cp-tree.h: Include cgraph.h
	(DECL_NEEDED_P): Use cgraph_*node on the decl instead of
	TREE_SYMBOL_REFERENCED on the DECL_ASSEMBLER_NAME of the decl.

Patches:
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/cp/ChangeLog.diff?cvsroot=gcc&r1=1.4099&r2=1.4100
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/cp/cp-tree.h.diff?cvsroot=gcc&r1=1.975&r2=1.976



-- 


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

[Bug c++/14639] [3.5 Regression] [non-unit-at-a-time] Incorrect emission of unused compiler-generated destructor

[pinskia at gcc dot gnu dot org]

-- 
Bug 14639 depends on bug 15816, which changed state.

Bug 15816 Summary: non-unit-at-a-time should be removed
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=15816

           What    |Old Value                   |New Value
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |RESOLVED
         Resolution|                            |WONTFIX

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