* [PATCH] PR c++/27574
@ 2008-09-04 22:02 Dodji Seketeli
2008-09-05 8:56 ` Richard Guenther
` (2 more replies)
0 siblings, 3 replies; 32+ messages in thread
From: Dodji Seketeli @ 2008-09-04 22:02 UTC (permalink / raw)
To: Gcc Patch List; +Cc: ian, Jason Merrill
[-- Attachment #1: Type: text/plain, Size: 558 bytes --]
Hello,
This patch was initially done by Ian Lance Taylor to fix PR c++/27574 in
gcc 4.1, but never got applied.
I am interested in seeing this fixed in gcc 4.1, 4.2, 4.3 and trunk so I
tested the patch on those branches and it works and fixes the problem.
I did regtested it on those branches for x86_64.
Assuming the patch looks good, would it be okay to commit it to 4.1, 4.2
and 4.3 ?
This morning's trunk looked to be a bit broken (some tests were not
passing) so I'd maybe wait a bit for trunk to stabilize before pushing
it to trunk.
Thanks
[-- Attachment #2: PR27574-patch.txt --]
[-- Type: text/plain, Size: 1768 bytes --]
gcc/ChangeLog:
2008-09-04 Dodji Seketeli <dodji@redhat.com>
PR c++/27574
* gcc/cgraph.c (cgraph_remove_node): Do not remove the body of
abstract functions. It might be useful to emit debugging
information. This is a patch from Ian Lance Taylor.
gcc/testsuite/ChangeLog:
2008-09-04 Dodji Seketeli <dodji@gnome.org>
PR c++/27574
* g++.dg/debug/dwarf2/local-var-in-contructor.C: New testcase.
diff --git a/gcc/cgraph.c b/gcc/cgraph.c
index 199c639..3d21190 100644
--- a/gcc/cgraph.c
+++ b/gcc/cgraph.c
@@ -479,6 +479,14 @@ cgraph_remove_node (struct cgraph_node *node)
kill_body = true;
}
+ /* We don't release the body of abstract functions, because they may
+ be needed when emitting debugging information. In particular
+ this will happen for C++ constructors/destructors. FIXME:
+ Ideally we would check to see whether there are any reachable
+ functions whose DECL_ABSTRACT_ORIGIN points to this decl. */
+ if (DECL_ABSTRACT (node->decl))
+ kill_body = false;
+
if (kill_body && !dump_enabled_p (TDI_tree_all) && flag_unit_at_a_time)
{
DECL_SAVED_TREE (node->decl) = NULL;
diff --git a/gcc/testsuite/g++.dg/debug/dwarf2/local-var-in-contructor.C b/gcc/testsuite/g++.dg/debug/dwarf2/local-var-in-contructor.C
new file mode 100644
index 0000000..d61d27f
--- /dev/null
+++ b/gcc/testsuite/g++.dg/debug/dwarf2/local-var-in-contructor.C
@@ -0,0 +1,30 @@
+// Contributed by Dodji Seketeli <dodji@redhat.com>
+// Origin PR27574
+// { dg-do compile }
+// { dg-options "-O0 -g" }
+// { dg-final { scan-assembler "problem" } }
+
+void f (int *)
+{
+}
+
+class A
+{
+public:
+ A(int i);
+};
+
+A::A(int i)
+{
+ int *problem = new int(i);
+ f (problem);
+}
+
+int
+main (void)
+{
+ A a (0);
+
+ return 0;
+}
+
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH] PR c++/27574
2008-09-04 22:02 [PATCH] PR c++/27574 Dodji Seketeli
@ 2008-09-05 8:56 ` Richard Guenther
2008-09-05 17:41 ` Jason Merrill
2008-09-15 11:40 ` [PING] PATCH " Dodji Seketeli
2008-09-24 20:46 ` [PATCH] " Jason Merrill
2 siblings, 1 reply; 32+ messages in thread
From: Richard Guenther @ 2008-09-05 8:56 UTC (permalink / raw)
To: Dodji Seketeli; +Cc: Gcc Patch List, ian, Jason Merrill
On Thu, Sep 4, 2008 at 11:45 PM, Dodji Seketeli <dodji@redhat.com> wrote:
> Hello,
>
> This patch was initially done by Ian Lance Taylor to fix PR c++/27574 in gcc
> 4.1, but never got applied.
>
> I am interested in seeing this fixed in gcc 4.1, 4.2, 4.3 and trunk so I
> tested the patch on those branches and it works and fixes the problem.
>
> I did regtested it on those branches for x86_64.
>
> Assuming the patch looks good, would it be okay to commit it to 4.1, 4.2 and
> 4.3 ?
Note that the 4.1 branch is closed.
Richard.
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH] PR c++/27574
2008-09-05 8:56 ` Richard Guenther
@ 2008-09-05 17:41 ` Jason Merrill
2008-09-23 18:57 ` Mark Mitchell
0 siblings, 1 reply; 32+ messages in thread
From: Jason Merrill @ 2008-09-05 17:41 UTC (permalink / raw)
To: Richard Guenther; +Cc: Dodji Seketeli, Gcc Patch List, ian
Richard Guenther wrote:
> Note that the 4.1 branch is closed.
In the past, I believe the policy has been that it's OK to check in
patches on closed branches if they are appropriate for a release branch;
closed just means there won't be any more releases.
Jason
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PING] PATCH PR c++/27574
2008-09-04 22:02 [PATCH] PR c++/27574 Dodji Seketeli
2008-09-05 8:56 ` Richard Guenther
@ 2008-09-15 11:40 ` Dodji Seketeli
2008-09-24 20:46 ` [PATCH] " Jason Merrill
2 siblings, 0 replies; 32+ messages in thread
From: Dodji Seketeli @ 2008-09-15 11:40 UTC (permalink / raw)
To: Gcc Patch List; +Cc: ian, Jason Merrill
Hello,
This is a PING for patch submission
http://gcc.gnu.org/ml/gcc-patches/2008-09/msg00418.html .
Thanks in advance,
Dodji.
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH] PR c++/27574
2008-09-05 17:41 ` Jason Merrill
@ 2008-09-23 18:57 ` Mark Mitchell
2008-09-24 23:15 ` Richard Guenther
0 siblings, 1 reply; 32+ messages in thread
From: Mark Mitchell @ 2008-09-23 18:57 UTC (permalink / raw)
To: Jason Merrill; +Cc: Richard Guenther, Dodji Seketeli, Gcc Patch List, ian
Jason Merrill wrote:
> Richard Guenther wrote:
>> Note that the 4.1 branch is closed.
>
> In the past, I believe the policy has been that it's OK to check in
> patches on closed branches if they are appropriate for a release branch;
> closed just means there won't be any more releases.
Yes, that's my understanding of the policy as well. I see no harm if
people want to continue maintaining old branches under the usual
release-branch rules.
--
Mark Mitchell
CodeSourcery
mark@codesourcery.com
(650) 331-3385 x713
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH] PR c++/27574
2008-09-04 22:02 [PATCH] PR c++/27574 Dodji Seketeli
2008-09-05 8:56 ` Richard Guenther
2008-09-15 11:40 ` [PING] PATCH " Dodji Seketeli
@ 2008-09-24 20:46 ` Jason Merrill
2008-09-26 14:19 ` Dodji Seketeli
2 siblings, 1 reply; 32+ messages in thread
From: Jason Merrill @ 2008-09-24 20:46 UTC (permalink / raw)
To: Dodji Seketeli; +Cc: Gcc Patch List, ian
Seems like the right fix would be to include clones in the inlined_to
list of the abstract function.
Jason
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH] PR c++/27574
2008-09-23 18:57 ` Mark Mitchell
@ 2008-09-24 23:15 ` Richard Guenther
2008-09-25 14:01 ` Mark Mitchell
0 siblings, 1 reply; 32+ messages in thread
From: Richard Guenther @ 2008-09-24 23:15 UTC (permalink / raw)
To: Mark Mitchell; +Cc: Jason Merrill, Dodji Seketeli, Gcc Patch List, ian
On Tue, Sep 23, 2008 at 8:17 PM, Mark Mitchell <mark@codesourcery.com> wrote:
> Jason Merrill wrote:
>> Richard Guenther wrote:
>>> Note that the 4.1 branch is closed.
>>
>> In the past, I believe the policy has been that it's OK to check in
>> patches on closed branches if they are appropriate for a release branch;
>> closed just means there won't be any more releases.
>
> Yes, that's my understanding of the policy as well. I see no harm if
> people want to continue maintaining old branches under the usual
> release-branch rules.
I thought for the 4.1 branch there was the additional issue that we
do not want to make it GPLv3. So while individual contributions under
GLPv2 are ok, backporting other peoples work may create a problem.
So I thought better really close the branch to avoid even thinking about
this issue.
Richard.
> --
> Mark Mitchell
> CodeSourcery
> mark@codesourcery.com
> (650) 331-3385 x713
>
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH] PR c++/27574
2008-09-24 23:15 ` Richard Guenther
@ 2008-09-25 14:01 ` Mark Mitchell
0 siblings, 0 replies; 32+ messages in thread
From: Mark Mitchell @ 2008-09-25 14:01 UTC (permalink / raw)
To: Richard Guenther; +Cc: Jason Merrill, Dodji Seketeli, Gcc Patch List, ian
Richard Guenther wrote:
> I thought for the 4.1 branch there was the additional issue that we
> do not want to make it GPLv3. So while individual contributions under
> GLPv2 are ok, backporting other peoples work may create a problem.
>
> So I thought better really close the branch to avoid even thinking about
> this issue.
That's a good point.
I suppose we could apply GPLv3 to the 4.1 branch, but I agree that we
shouldn't do that. Of course, the worst thing would be to apply a GPLv3
patch without fully converting the license notices, which would be very
confusing.
But, as long as the patch is GPLv2, I don't see a particular problem.
--
Mark Mitchell
CodeSourcery
mark@codesourcery.com
(650) 331-3385 x713
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH] PR c++/27574
2008-09-24 20:46 ` [PATCH] " Jason Merrill
@ 2008-09-26 14:19 ` Dodji Seketeli
2008-09-26 23:51 ` Jason Merrill
0 siblings, 1 reply; 32+ messages in thread
From: Dodji Seketeli @ 2008-09-26 14:19 UTC (permalink / raw)
To: Jason Merrill; +Cc: Gcc Patch List, jh
Hello,
Jason Merrill a écrit :
> Seems like the right fix would be to include clones in the inlined_to
> list of the abstract function.
>
After talking with Honza on IRC, it appeared that it wouldn't work
because nodes that are added to inline_to must be actually inlined
otherwise, later sanity check that would break.
I guess Honza can give more details about this :-)
Cheers,
Dodji.
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH] PR c++/27574
2008-09-26 14:19 ` Dodji Seketeli
@ 2008-09-26 23:51 ` Jason Merrill
2008-09-27 0:26 ` Jan Hubicka
0 siblings, 1 reply; 32+ messages in thread
From: Jason Merrill @ 2008-09-26 23:51 UTC (permalink / raw)
To: Dodji Seketeli; +Cc: Gcc Patch List, jh
Dodji Seketeli wrote:
> Jason Merrill a écrit :
>> Seems like the right fix would be to include clones in the inlined_to
>> list of the abstract function.
>
> After talking with Honza on IRC, it appeared that it wouldn't work
> because nodes that are added to inline_to must be actually inlined
> otherwise, later sanity check that would break.
Can't the later sanity check check for inlining or cloning?
Jason
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH] PR c++/27574
2008-09-26 23:51 ` Jason Merrill
@ 2008-09-27 0:26 ` Jan Hubicka
2008-09-30 18:46 ` Jason Merrill
0 siblings, 1 reply; 32+ messages in thread
From: Jan Hubicka @ 2008-09-27 0:26 UTC (permalink / raw)
To: Jason Merrill; +Cc: Dodji Seketeli, Gcc Patch List, jh
> Dodji Seketeli wrote:
> >Jason Merrill a ĂŠcrit :
> >>Seems like the right fix would be to include clones in the inlined_to
> >>list of the abstract function.
> >
> >After talking with Honza on IRC, it appeared that it wouldn't work
> >because nodes that are added to inline_to must be actually inlined
> >otherwise, later sanity check that would break.
>
> Can't the later sanity check check for inlining or cloning?
We definitly can remove the sanity check, but the inline clones are
designed for representing inline plan (i.e. producing virtual clones
that do not have actual tree representation), so using them for linking
functions that was originaly produced as clones by frontend but appear
as real functions in backend does not make that much sense.
If we want to have functions related together, we definitly can add new
lists for that, same way as we do for nested functions and such.
Honza
>
> Jason
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH] PR c++/27574
2008-09-27 0:26 ` Jan Hubicka
@ 2008-09-30 18:46 ` Jason Merrill
2008-10-06 19:46 ` Dodji Seketeli
0 siblings, 1 reply; 32+ messages in thread
From: Jason Merrill @ 2008-09-30 18:46 UTC (permalink / raw)
To: Jan Hubicka; +Cc: Dodji Seketeli, Gcc Patch List, jh
Jan Hubicka wrote:
> If we want to have functions related together, we definitly can add new
> lists for that, same way as we do for nested functions and such.
My inclination would be to do this for trunk, and apply Ian's patch to
the release branches.
Jason
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH] PR c++/27574
2008-09-30 18:46 ` Jason Merrill
@ 2008-10-06 19:46 ` Dodji Seketeli
2008-10-06 22:39 ` Jason Merrill
0 siblings, 1 reply; 32+ messages in thread
From: Dodji Seketeli @ 2008-10-06 19:46 UTC (permalink / raw)
To: Jason Merrill; +Cc: Jan Hubicka, Gcc Patch List, jh
Hello,
Jason Merrill a ĂŠcrit :
> Jan Hubicka wrote:
>> If we want to have functions related together, we definitly can add new
>> lists for that, same way as we do for nested functions and such.
>
> My inclination would be to do this for trunk, and apply Ian's patch to
> the release branches.
Jan, I did post a superset of Ian's patch to make the sanity check code
not cry when it encounters abstract functions. Could you please tell if
it looks okay to you so that I can push it to the released branches ?
The post is at http://gcc.gnu.org/ml/gcc-patches/2008-09/msg01815.html.
Now for the approach to use in trunk, there is something I am not sure
about.
My understanding is that for each node which node->decl is abstract,
we'd like to build a node->global->cloned_to list of nodes that points
to the clones of the abstract node.
That'd allow the code of cgraph_remove_node() check for the presence of
node->global->cloned_to list and not kill the body of the node if that
list is present.
Now my problem is where (in which function) to build that list ?
As this is mainly targeted at abstract constructors, I could -- in
build_clone(), gcc/cp/class.c -- add and update a list of clones to each
abstract function that is cloned. That list of clones would then be used
to update the node->global->cloned_to list, in
cgraph_finalize_compilation_unit(), in cgraphunit.c. I am not sure if
this would be the right approach though.
Cheers,
Dodji.
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH] PR c++/27574
2008-10-06 19:46 ` Dodji Seketeli
@ 2008-10-06 22:39 ` Jason Merrill
2008-10-09 7:54 ` Dodji Seketeli
0 siblings, 1 reply; 32+ messages in thread
From: Jason Merrill @ 2008-10-06 22:39 UTC (permalink / raw)
To: Dodji Seketeli; +Cc: Jan Hubicka, Gcc Patch List, jh
Dodji Seketeli wrote:
> Jan, I did post a superset of Ian's patch to make the sanity check code
> not cry when it encounters abstract functions. Could you please tell if
> it looks okay to you so that I can push it to the released branches ?
> The post is at http://gcc.gnu.org/ml/gcc-patches/2008-09/msg01815.html.
s/comply/complain/
> Now for the approach to use in trunk, there is something I am not sure
> about.
>
> My understanding is that for each node which node->decl is abstract,
> we'd like to build a node->global->cloned_to list of nodes that points
> to the clones of the abstract node.
> That'd allow the code of cgraph_remove_node() check for the presence of
> node->global->cloned_to list and not kill the body of the node if that
> list is present.
I think it would be better to do it the other way around; if a function
we still need has DECL_ABSTRACT_ORIGIN, we need to keep the decl it
points to. But again, I don't know the cgraph code.
Jason
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH] PR c++/27574
2008-10-06 22:39 ` Jason Merrill
@ 2008-10-09 7:54 ` Dodji Seketeli
2008-10-09 18:49 ` Jason Merrill
2008-11-03 11:28 ` [PATCH] PR c++/27574 (PING) Dodji Seketeli
0 siblings, 2 replies; 32+ messages in thread
From: Dodji Seketeli @ 2008-10-09 7:54 UTC (permalink / raw)
To: jh; +Cc: Gcc Patch List, Jason Merrill, Jan Hubicka
[-- Attachment #1: Type: text/plain, Size: 878 bytes --]
Jason Merrill a ĂŠcrit :
[...]
>> Jan, I did post a superset of Ian's patch to make the sanity check
>> code not cry when it encounters abstract functions. Could you please
>> tell if it looks okay to you so that I can push it to the released
>> branches ? The post is at
>> http://gcc.gnu.org/ml/gcc-patches/2008-09/msg01815.html.
>
> s/comply/complain/
>
Thanks. The first attached patch named
PR27574-patch-for-released-branches.txt fixes that.
[...]
> I think it would be better to do it the other way around; if a function
> we still need has DECL_ABSTRACT_ORIGIN, we need to keep the decl it
> points to.
[...]
The second attached patch named patches/PR27574-patch-for-trunk.txt
tries to implement that approach.
So would the first patch be OK to apply to the released branches and the
second for trunk ? Both pass distcheck on x86_64.
Cheers,
Dodji.
[-- Attachment #2: PR27574-patch-for-released-branches.txt --]
[-- Type: text/plain, Size: 2467 bytes --]
gcc/Changelog:
PR c++/27574
* gcc/cgraph.c (cgraph_remove_node): Do not remove the body of
abstract functions. It might be useful to emit debugging
information. This is a patch from Ian Lance Taylor.
* cgraphunit.c (cgraph_optimize): Do not cry when bodies of abstract
functions are still around. They are useful to output debug info.
gcc/testsuite/ChangeLog:
PR c++/27574
* g++.dg/debug/dwarf2/local-var-in-contructor.C: New testcase.
diff --git a/gcc/cgraph.c b/gcc/cgraph.c
index 199c639..3d21190 100644
--- a/gcc/cgraph.c
+++ b/gcc/cgraph.c
@@ -479,6 +479,14 @@ cgraph_remove_node (struct cgraph_node *node)
kill_body = true;
}
+ /* We don't release the body of abstract functions, because they may
+ be needed when emitting debugging information. In particular
+ this will happen for C++ constructors/destructors. FIXME:
+ Ideally we would check to see whether there are any reachable
+ functions whose DECL_ABSTRACT_ORIGIN points to this decl. */
+ if (DECL_ABSTRACT (node->decl))
+ kill_body = false;
+
if (kill_body && !dump_enabled_p (TDI_tree_all) && flag_unit_at_a_time)
{
DECL_SAVED_TREE (node->decl) = NULL;
diff --git a/gcc/cgraphunit.c b/gcc/cgraphunit.c
index 65a8651..83b070d 100644
--- a/gcc/cgraphunit.c
+++ b/gcc/cgraphunit.c
@@ -1391,7 +1391,10 @@ cgraph_optimize (void)
for (node = cgraph_nodes; node; node = node->next)
if (node->analyzed
&& (node->global.inlined_to
- || DECL_SAVED_TREE (node->decl)))
+ || DECL_SAVED_TREE (node->decl))
+ /* Abstract functions are needed to output debug info,
+ so don't complain about them if they are still around. */
+ && !DECL_ABSTRACT (node->decl))
{
error_found = true;
dump_cgraph_node (stderr, node);
diff --git a/gcc/testsuite/g++.dg/debug/dwarf2/local-var-in-contructor.C b/gcc/testsuite/g++.dg/debug/dwarf2/local-var-in-contructor.C
new file mode 100644
index 0000000..d61d27f
--- /dev/null
+++ b/gcc/testsuite/g++.dg/debug/dwarf2/local-var-in-contructor.C
@@ -0,0 +1,30 @@
+// Contributed by Dodji Seketeli <dodji@redhat.com>
+// Origin PR27574
+// { dg-do compile }
+// { dg-options "-O0 -g" }
+// { dg-final { scan-assembler "problem" } }
+
+void f (int *)
+{
+}
+
+class A
+{
+public:
+ A(int i);
+};
+
+A::A(int i)
+{
+ int *problem = new int(i);
+ f (problem);
+}
+
+int
+main (void)
+{
+ A a (0);
+
+ return 0;
+}
+
[-- Attachment #3: PR27574-patch-for-trunk.txt --]
[-- Type: text/plain, Size: 5208 bytes --]
gcc/ChangeLog:
2008-10-08 Dodji Seketeli <dodji@redhat.com>
PR debug/27574
* cgraph.h: New abstract_and_needed member to struct cgraph_node.
* cgraphunit.c (cgraph_analyze_functions): Flag abstract functions
- which clones are reachable - as "abstract and needed".
Then make sure the body of those abstract and needed functions
are not dropped because they are used to emit debug info.
(cgraph_mark_functions_to_output): Do not complain if some abstract and
needed functions are still around.
(cgraph_optimize): Likewise.
* ipa.c (cgraph_remove_unreachable_nodes): Do not remove abstract and
needed functions nodes.
gcc/testsuite/ChangeLog:
2008-10-08 Dodji Seketeli <dodji@redhat.com>
PR debug/27574
* g++.dg/debug/dwarf2/local-var-in-contructor.C: New test.
diff --git a/gcc/cgraph.h b/gcc/cgraph.h
index a6018dc..7825804 100644
--- a/gcc/cgraph.h
+++ b/gcc/cgraph.h
@@ -166,6 +166,9 @@ struct cgraph_node GTY((chain_next ("%h.next"), chain_prev ("%h.previous")))
/* Set when function must be output - it is externally visible
or its address is taken. */
unsigned needed : 1;
+ /* Set when decl is an abstract function pointed to by the
+ ABSTRACT_DECL_ORIGIN of a reachable function. */
+ unsigned abstract_and_needed : 1;
/* Set when function is reachable by call from other function
that is either reachable or needed. */
unsigned reachable : 1;
diff --git a/gcc/cgraphunit.c b/gcc/cgraphunit.c
index 7c84573..754d4e7 100644
--- a/gcc/cgraphunit.c
+++ b/gcc/cgraphunit.c
@@ -896,6 +896,15 @@ cgraph_analyze_functions (void)
if (!edge->callee->reachable)
cgraph_mark_reachable_node (edge->callee);
+ /* If decl is a clone of an abstract function, mark that abstract
+ function so that we don't release its body. That abstract function body will
+ be needed to output debug info. */
+ if (DECL_ABSTRACT_ORIGIN (decl))
+ {
+ struct cgraph_node *origin_node = cgraph_node (DECL_ABSTRACT_ORIGIN (decl));
+ origin_node->abstract_and_needed = true;
+ }
+
/* We finalize local static variables during constructing callgraph
edges. Process their attributes too. */
process_function_and_variable_attributes (first_processed,
@@ -928,7 +937,7 @@ cgraph_analyze_functions (void)
if (node->local.finalized && !gimple_has_body_p (decl))
cgraph_reset_node (node);
- if (!node->reachable && gimple_has_body_p (decl))
+ if (!node->reachable && gimple_has_body_p (decl) && !node->abstract_and_needed)
{
if (cgraph_dump_file)
fprintf (cgraph_dump_file, " %s", cgraph_node_name (node));
@@ -1003,7 +1012,8 @@ cgraph_mark_functions_to_output (void)
#ifdef ENABLE_CHECKING
if (!node->global.inlined_to
&& gimple_has_body_p (decl)
- && !DECL_EXTERNAL (decl))
+ && !DECL_EXTERNAL (decl)
+ && !node->abstract_and_needed)
{
dump_cgraph_node (stderr, node);
internal_error ("failed to reclaim unneeded function");
@@ -1011,7 +1021,8 @@ cgraph_mark_functions_to_output (void)
#endif
gcc_assert (node->global.inlined_to
|| !gimple_has_body_p (decl)
- || DECL_EXTERNAL (decl));
+ || DECL_EXTERNAL (decl)
+ || node->abstract_and_needed);
}
@@ -1325,7 +1336,11 @@ cgraph_optimize (void)
for (node = cgraph_nodes; node; node = node->next)
if (node->analyzed
&& (node->global.inlined_to
- || gimple_has_body_p (node->decl)))
+ || gimple_has_body_p (node->decl))
+ /* Abstract functions are needed to output debug info,
+ so don't complain about them if they are
+ still around. */
+ && !node->abstract_and_needed)
{
error_found = true;
dump_cgraph_node (stderr, node);
diff --git a/gcc/ipa.c b/gcc/ipa.c
index 0e2cb2d..e5c3115 100644
--- a/gcc/ipa.c
+++ b/gcc/ipa.c
@@ -160,8 +160,8 @@ cgraph_remove_unreachable_nodes (bool before_inlining_p, FILE *file)
node->global.inlined_to = NULL;
if (file)
fprintf (file, " %s", cgraph_node_name (node));
- if (!node->analyzed || !DECL_EXTERNAL (node->decl)
- || before_inlining_p)
+ if ((!node->analyzed || !DECL_EXTERNAL (node->decl)
+ || before_inlining_p) && !node->abstract_and_needed)
cgraph_remove_node (node);
else
{
@@ -187,7 +187,7 @@ cgraph_remove_unreachable_nodes (bool before_inlining_p, FILE *file)
node->analyzed = false;
node->local.inlinable = false;
}
- else
+ else if (!node->abstract_and_needed)
cgraph_remove_node (node);
}
changed = true;
diff --git a/gcc/testsuite/g++.dg/debug/dwarf2/local-var-in-contructor.C b/gcc/testsuite/g++.dg/debug/dwarf2/local-var-in-contructor.C
new file mode 100644
index 0000000..d61d27f
--- /dev/null
+++ b/gcc/testsuite/g++.dg/debug/dwarf2/local-var-in-contructor.C
@@ -0,0 +1,30 @@
+// Contributed by Dodji Seketeli <dodji@redhat.com>
+// Origin PR27574
+// { dg-do compile }
+// { dg-options "-O0 -g" }
+// { dg-final { scan-assembler "problem" } }
+
+void f (int *)
+{
+}
+
+class A
+{
+public:
+ A(int i);
+};
+
+A::A(int i)
+{
+ int *problem = new int(i);
+ f (problem);
+}
+
+int
+main (void)
+{
+ A a (0);
+
+ return 0;
+}
+
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH] PR c++/27574
2008-10-09 7:54 ` Dodji Seketeli
@ 2008-10-09 18:49 ` Jason Merrill
2008-11-03 11:28 ` [PATCH] PR c++/27574 (PING) Dodji Seketeli
1 sibling, 0 replies; 32+ messages in thread
From: Jason Merrill @ 2008-10-09 18:49 UTC (permalink / raw)
To: Dodji Seketeli; +Cc: jh, Gcc Patch List, Jan Hubicka
Dodji Seketeli wrote:
> So would the first patch be OK to apply to the released branches and the
> second for trunk ? Both pass distcheck on x86_64.
They look fine to me, but I'd like someone who knows cgraph sign off on
them.
Jason
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH] PR c++/27574 (PING)
2008-10-09 7:54 ` Dodji Seketeli
2008-10-09 18:49 ` Jason Merrill
@ 2008-11-03 11:28 ` Dodji Seketeli
2008-11-03 12:37 ` Richard Guenther
1 sibling, 1 reply; 32+ messages in thread
From: Dodji Seketeli @ 2008-11-03 11:28 UTC (permalink / raw)
To: jh; +Cc: Gcc Patch List, Jan Hubicka
Hello Jan (and maybe to any other callgraph maintainer out there),
This is a ping for a review of the patch I posted at
http://gcc.gnu.org/ml/gcc-patches/2008-10/msg00367.html .
Cheers,
Dodji.
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH] PR c++/27574 (PING)
2008-11-03 11:28 ` [PATCH] PR c++/27574 (PING) Dodji Seketeli
@ 2008-11-03 12:37 ` Richard Guenther
2008-11-03 13:56 ` Dodji Seketeli
0 siblings, 1 reply; 32+ messages in thread
From: Richard Guenther @ 2008-11-03 12:37 UTC (permalink / raw)
To: Dodji Seketeli; +Cc: jh, Gcc Patch List, Jan Hubicka
On Mon, Nov 3, 2008 at 12:25 PM, Dodji Seketeli <dodji@redhat.com> wrote:
> Hello Jan (and maybe to any other callgraph maintainer out there),
>
> This is a ping for a review of the patch I posted at
> http://gcc.gnu.org/ml/gcc-patches/2008-10/msg00367.html .
Hm. What is the effect on text size for C++ applications? We do not
need the function text itself but only its debug information - the debug
information for the decl, not the text, correct?
Richard.
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH] PR c++/27574 (PING)
2008-11-03 12:37 ` Richard Guenther
@ 2008-11-03 13:56 ` Dodji Seketeli
2008-11-03 14:43 ` Richard Guenther
0 siblings, 1 reply; 32+ messages in thread
From: Dodji Seketeli @ 2008-11-03 13:56 UTC (permalink / raw)
To: Richard Guenther; +Cc: jh, Gcc Patch List, Jan Hubicka
Richard Guenther a écrit :
> Hm. What is the effect on text size for C++ applications?
Well, I don't have any hard figures for that, but I suspect that as the
only the bodies of abstract functions which clones are reachable (and
thus which bodies are emitted) are now being added, the text size might
grow only very slightly.
In any case, I am rebuilding/installing a gcc tree here so that I can
compile some c++ programs of mine to come up with some data.
> We do not need the function text itself but only its debug information - the debug
> information for the decl, not the text, correct?
I think for a function decl, we "only" need DECL_ARGUMENTS (decl) and
DECL_INITIAL (decl) to be present so that we can generate debug info.
Dodji.
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH] PR c++/27574 (PING)
2008-11-03 13:56 ` Dodji Seketeli
@ 2008-11-03 14:43 ` Richard Guenther
2008-11-03 17:14 ` Jan Hubicka
2008-11-03 17:48 ` Dodji Seketeli
0 siblings, 2 replies; 32+ messages in thread
From: Richard Guenther @ 2008-11-03 14:43 UTC (permalink / raw)
To: Dodji Seketeli; +Cc: jh, Gcc Patch List, Jan Hubicka
On Mon, Nov 3, 2008 at 2:53 PM, Dodji Seketeli <dodji@redhat.com> wrote:
> Richard Guenther a écrit :
>>
>> Hm. What is the effect on text size for C++ applications?
>
> Well, I don't have any hard figures for that, but I suspect that as the only
> the bodies of abstract functions which clones are reachable (and thus which
> bodies are emitted) are now being added, the text size might grow only very
> slightly.
>
> In any case, I am rebuilding/installing a gcc tree here so that I can
> compile some c++ programs of mine to come up with some data.
>
>> We do not need the function text itself but only its debug information -
>> the debug
>> information for the decl, not the text, correct?
>
> I think for a function decl, we "only" need DECL_ARGUMENTS (decl) and
> DECL_INITIAL (decl) to be present so that we can generate debug info.
So the only problem is the following line
DECL_INITIAL (node->decl) = error_mark_node;
in cgraph_release_function_body? Does it fix the PR
if you make that conditional on DECL_ABSTRACT?
Thanks,
Richard.
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH] PR c++/27574 (PING)
2008-11-03 14:43 ` Richard Guenther
@ 2008-11-03 17:14 ` Jan Hubicka
2008-11-03 18:02 ` Dodji Seketeli
2008-11-03 17:48 ` Dodji Seketeli
1 sibling, 1 reply; 32+ messages in thread
From: Jan Hubicka @ 2008-11-03 17:14 UTC (permalink / raw)
To: Richard Guenther; +Cc: Dodji Seketeli, jh, Gcc Patch List, Jan Hubicka
> On Mon, Nov 3, 2008 at 2:53 PM, Dodji Seketeli <dodji@redhat.com> wrote:
> > Richard Guenther a ĂŠcrit :
> >>
> >> Hm. What is the effect on text size for C++ applications?
> >
> > Well, I don't have any hard figures for that, but I suspect that as the only
> > the bodies of abstract functions which clones are reachable (and thus which
> > bodies are emitted) are now being added, the text size might grow only very
> > slightly.
> >
> > In any case, I am rebuilding/installing a gcc tree here so that I can
> > compile some c++ programs of mine to come up with some data.
> >
> >> We do not need the function text itself but only its debug information -
> >> the debug
> >> information for the decl, not the text, correct?
> >
> > I think for a function decl, we "only" need DECL_ARGUMENTS (decl) and
> > DECL_INITIAL (decl) to be present so that we can generate debug info.
>
> So the only problem is the following line
>
> DECL_INITIAL (node->decl) = error_mark_node;
>
> in cgraph_release_function_body? Does it fix the PR
> if you make that conditional on DECL_ABSTRACT?
The reason for setting DECL_INITIAL there was that BLOCK pointers used
to point to generic function body that remained in memory.
I think it is not needed at all and might not be the case anymore, so it
might be fine. Still blocks occupy quite a lot of ram, can dwarf2out be
in charge of releasing them?
Honza
>
> Thanks,
> Richard.
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH] PR c++/27574 (PING)
2008-11-03 14:43 ` Richard Guenther
2008-11-03 17:14 ` Jan Hubicka
@ 2008-11-03 17:48 ` Dodji Seketeli
1 sibling, 0 replies; 32+ messages in thread
From: Dodji Seketeli @ 2008-11-03 17:48 UTC (permalink / raw)
To: Richard Guenther; +Cc: jh, Gcc Patch List, Jan Hubicka
Richard Guenther a écrit :
> So the only problem is the following line
>
> DECL_INITIAL (node->decl) = error_mark_node;
>
> in cgraph_release_function_body? Does it fix the PR
> if you make that conditional on DECL_ABSTRACT?
Yes, it fixes the problem. And in a much more compact way than what I
was trying to do. Thanks.
I am still running regtests (for C and C++). I will post a new patch
when that's done.
Thanks,
Dodji.
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH] PR c++/27574 (PING)
2008-11-03 17:14 ` Jan Hubicka
@ 2008-11-03 18:02 ` Dodji Seketeli
2008-11-03 18:39 ` Jason Merrill
0 siblings, 1 reply; 32+ messages in thread
From: Dodji Seketeli @ 2008-11-03 18:02 UTC (permalink / raw)
To: Jan Hubicka; +Cc: Richard Guenther, Gcc Patch List, Jason Merrill
Jan Hubicka a écrit :
[...]
> The reason for setting DECL_INITIAL there was that BLOCK pointers used
> to point to generic function body that remained in memory.
Okay.
> I think it is not needed at all and might not be the case anymore, so it
> might be fine.
Yeah, it looks like it's not needed.
> Still blocks occupy quite a lot of ram, can dwarf2out be
> in charge of releasing them?
You mean, you'd want dwarf2out to set the DECL_INITIAL (decl) to
error_mark_node after emitting the debug info ? I certainly could do
that, assuming that does not go against the spirit of dwarf2out.
Cheers,
Dodji.
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH] PR c++/27574 (PING)
2008-11-03 18:02 ` Dodji Seketeli
@ 2008-11-03 18:39 ` Jason Merrill
2008-11-03 20:56 ` Dodji Seketeli
0 siblings, 1 reply; 32+ messages in thread
From: Jason Merrill @ 2008-11-03 18:39 UTC (permalink / raw)
To: Dodji Seketeli; +Cc: Jan Hubicka, Richard Guenther, Gcc Patch List
Dodji Seketeli wrote:
> Jan Hubicka a écrit :
>> Still blocks occupy quite a lot of ram, can dwarf2out be
>> in charge of releasing them?
>
> You mean, you'd want dwarf2out to set the DECL_INITIAL (decl) to
> error_mark_node after emitting the debug info ? I certainly could do
> that, assuming that does not go against the spirit of dwarf2out.
This doesn't seem specific to dwarf2out; dbxout also uses the block
structure in DECL_INITIAL. I'd think that clearing DECL_INITIAL should
happen in code independent of which debugging backend is used.
Jason
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH] PR c++/27574 (PING)
2008-11-03 18:39 ` Jason Merrill
@ 2008-11-03 20:56 ` Dodji Seketeli
2008-11-03 22:17 ` Jason Merrill
0 siblings, 1 reply; 32+ messages in thread
From: Dodji Seketeli @ 2008-11-03 20:56 UTC (permalink / raw)
To: Jason Merrill; +Cc: Jan Hubicka, Richard Guenther, Gcc Patch List
[-- Attachment #1: Type: text/plain, Size: 184 bytes --]
Okay, so here is another patch proposal.
It passes regtest on trunk for x86_64. It think I can even use this one
for the other branches, assuming it passes regtest.
Thanks,
Dodji.
[-- Attachment #2: PR27574-patch.txt --]
[-- Type: text/plain, Size: 1849 bytes --]
gcc/ChangeLog/
2008-11-03 Dodji Seketeli <dodji@redhat.com>
* cgraph.c (cgraph_release_function_body): Do not kill DECL_INITIAL()
of the function decl. It's useful to emit debug info.
gcc/testsuite/ChangeLog:
2008-11-03 Dodji Seketeli <dodji@redhat.com>
* g++.dg/debug/dwarf2/local-var-in-contructor.C: New test.
diff --git a/gcc/cgraph.c b/gcc/cgraph.c
index 73dbfb3..6cbf87b 100644
--- a/gcc/cgraph.c
+++ b/gcc/cgraph.c
@@ -936,7 +936,6 @@ cgraph_release_function_body (struct cgraph_node *node)
DECL_STRUCT_FUNCTION (node->decl) = NULL;
}
DECL_SAVED_TREE (node->decl) = NULL;
- DECL_INITIAL (node->decl) = error_mark_node;
}
/* Remove the node from cgraph. */
diff --git a/gcc/final.c b/gcc/final.c
index e2d9e5a..59c310f 100644
--- a/gcc/final.c
+++ b/gcc/final.c
@@ -4186,6 +4186,9 @@ rest_of_handle_final (void)
timevar_push (TV_SYMOUT);
(*debug_hooks->function_decl) (current_function_decl);
+ /* Release the BLOCKs linked to DECL_INTIAL (current_function_decl)
+ to free up some memory. */
+ DECL_INITIAL (current_function_decl) = error_mark_node;
timevar_pop (TV_SYMOUT);
if (DECL_STATIC_CONSTRUCTOR (current_function_decl)
&& targetm.have_ctors_dtors)
diff --git a/gcc/testsuite/g++.dg/debug/dwarf2/local-var-in-contructor.C b/gcc/testsuite/g++.dg/debug/dwarf2/local-var-in-contructor.C
new file mode 100644
index 0000000..d61d27f
--- /dev/null
+++ b/gcc/testsuite/g++.dg/debug/dwarf2/local-var-in-contructor.C
@@ -0,0 +1,30 @@
+// Contributed by Dodji Seketeli <dodji@redhat.com>
+// Origin PR27574
+// { dg-do compile }
+// { dg-options "-O0 -g" }
+// { dg-final { scan-assembler "problem" } }
+
+void f (int *)
+{
+}
+
+class A
+{
+public:
+ A(int i);
+};
+
+A::A(int i)
+{
+ int *problem = new int(i);
+ f (problem);
+}
+
+int
+main (void)
+{
+ A a (0);
+
+ return 0;
+}
+
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH] PR c++/27574 (PING)
2008-11-03 20:56 ` Dodji Seketeli
@ 2008-11-03 22:17 ` Jason Merrill
2008-11-04 21:14 ` Dodji Seketeli
0 siblings, 1 reply; 32+ messages in thread
From: Jason Merrill @ 2008-11-03 22:17 UTC (permalink / raw)
To: Dodji Seketeli; +Cc: Jan Hubicka, Richard Guenther, Gcc Patch List
Seems like this won't solve the problem of wanting to discard the
information for unreachable functions.
Jason
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH] PR c++/27574 (PING)
2008-11-03 22:17 ` Jason Merrill
@ 2008-11-04 21:14 ` Dodji Seketeli
2008-11-05 16:40 ` Jason Merrill
2008-11-13 10:24 ` Eric Botcazou
0 siblings, 2 replies; 32+ messages in thread
From: Dodji Seketeli @ 2008-11-04 21:14 UTC (permalink / raw)
To: Jason Merrill; +Cc: Jan Hubicka, Richard Guenther, Gcc Patch List
[-- Attachment #1: Type: text/plain, Size: 602 bytes --]
Jason Merrill a écrit :
> Seems like this won't solve the problem of wanting to discard the
> information for unreachable functions.
Please find attached a reworked patch that merges the two previous
approaches.
Basically, I flag abstracts functions which clones are live. Then, at
function body clearing time, I just prevent the DECL_INITIAL() from
being nuked as well.
So the bodies of non live functions are always dropped incurring no text
size increase, but we should get the debug info for the abstract
functions that are needed.
It passes regtest on trunk for x86_64.
Thanks,
Dodji.
[-- Attachment #2: PR27574-patch.txt --]
[-- Type: text/plain, Size: 3935 bytes --]
gcc/ChangeLog:
2008-11-04 Dodji Seketeli <dodji@redhat.com>
PR debug/27574
* cgraph.h: New abstract_and_needed member to struct cgraph_node.
* cgraphunit.c (cgraph_analyze_functions): Flag abstract functions
- which clones are reachable - as "abstract and needed".
* cgraph.c (cgraph_release_function_body): If a node is "abstract and needed",
do not release its DECL_INITIAL() content because that will be needed to emit
debug info.
gcc/testsuite/ChangeLog:
2008-11-04 Dodji Seketeli <dodji@redhat.com>
PR debug/27574
* g++.dg/debug/dwarf2/local-var-in-contructor.C: New test.
diff --git a/gcc/cgraph.c b/gcc/cgraph.c
index 73dbfb3..43659cb 100644
--- a/gcc/cgraph.c
+++ b/gcc/cgraph.c
@@ -936,7 +936,11 @@ cgraph_release_function_body (struct cgraph_node *node)
DECL_STRUCT_FUNCTION (node->decl) = NULL;
}
DECL_SAVED_TREE (node->decl) = NULL;
- DECL_INITIAL (node->decl) = error_mark_node;
+ /* If the node is abstract and needed, then do not clear DECL_INITIAL
+ of its associated function function declaration because it's
+ needed to emit debug info later. */
+ if (!node->abstract_and_needed)
+ DECL_INITIAL (node->decl) = error_mark_node;
}
/* Remove the node from cgraph. */
diff --git a/gcc/cgraph.h b/gcc/cgraph.h
index a6018dc..7825804 100644
--- a/gcc/cgraph.h
+++ b/gcc/cgraph.h
@@ -166,6 +166,9 @@ struct cgraph_node GTY((chain_next ("%h.next"), chain_prev ("%h.previous")))
/* Set when function must be output - it is externally visible
or its address is taken. */
unsigned needed : 1;
+ /* Set when decl is an abstract function pointed to by the
+ ABSTRACT_DECL_ORIGIN of a reachable function. */
+ unsigned abstract_and_needed : 1;
/* Set when function is reachable by call from other function
that is either reachable or needed. */
unsigned reachable : 1;
diff --git a/gcc/cgraphunit.c b/gcc/cgraphunit.c
index 7c84573..cd58c2a 100644
--- a/gcc/cgraphunit.c
+++ b/gcc/cgraphunit.c
@@ -896,6 +896,15 @@ cgraph_analyze_functions (void)
if (!edge->callee->reachable)
cgraph_mark_reachable_node (edge->callee);
+ /* If decl is a clone of an abstract function, mark that abstract
+ function so that we don't release its body. The DECL_INITIAL() of that
+ abstract function declaration will be later needed to output debug info. */
+ if (DECL_ABSTRACT_ORIGIN (decl))
+ {
+ struct cgraph_node *origin_node = cgraph_node (DECL_ABSTRACT_ORIGIN (decl));
+ origin_node->abstract_and_needed = true;
+ }
+
/* We finalize local static variables during constructing callgraph
edges. Process their attributes too. */
process_function_and_variable_attributes (first_processed,
diff --git a/gcc/final.c b/gcc/final.c
index e2d9e5a..d007907 100644
--- a/gcc/final.c
+++ b/gcc/final.c
@@ -4187,6 +4187,10 @@ rest_of_handle_final (void)
timevar_push (TV_SYMOUT);
(*debug_hooks->function_decl) (current_function_decl);
timevar_pop (TV_SYMOUT);
+
+ /* Release the blocks that are linked to DECL_INITIAL() to free the memory. */
+ DECL_INITIAL (current_function_decl) = error_mark_node;
+
if (DECL_STATIC_CONSTRUCTOR (current_function_decl)
&& targetm.have_ctors_dtors)
targetm.asm_out.constructor (XEXP (DECL_RTL (current_function_decl), 0),
diff --git a/gcc/testsuite/g++.dg/debug/dwarf2/local-var-in-contructor.C b/gcc/testsuite/g++.dg/debug/dwarf2/local-var-in-contructor.C
new file mode 100644
index 0000000..d61d27f
--- /dev/null
+++ b/gcc/testsuite/g++.dg/debug/dwarf2/local-var-in-contructor.C
@@ -0,0 +1,30 @@
+// Contributed by Dodji Seketeli <dodji@redhat.com>
+// Origin PR27574
+// { dg-do compile }
+// { dg-options "-O0 -g" }
+// { dg-final { scan-assembler "problem" } }
+
+void f (int *)
+{
+}
+
+class A
+{
+public:
+ A(int i);
+};
+
+A::A(int i)
+{
+ int *problem = new int(i);
+ f (problem);
+}
+
+int
+main (void)
+{
+ A a (0);
+
+ return 0;
+}
+
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH] PR c++/27574 (PING)
2008-11-04 21:14 ` Dodji Seketeli
@ 2008-11-05 16:40 ` Jason Merrill
2008-11-13 10:24 ` Eric Botcazou
1 sibling, 0 replies; 32+ messages in thread
From: Jason Merrill @ 2008-11-05 16:40 UTC (permalink / raw)
To: Dodji Seketeli; +Cc: Jan Hubicka, Richard Guenther, Gcc Patch List
Dodji Seketeli wrote:
> Please find attached a reworked patch that merges the two previous
> approaches.
This looks good to me.
Jason
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH] PR c++/27574 (PING)
2008-11-04 21:14 ` Dodji Seketeli
2008-11-05 16:40 ` Jason Merrill
@ 2008-11-13 10:24 ` Eric Botcazou
2008-11-13 11:44 ` Dodji Seketeli
1 sibling, 1 reply; 32+ messages in thread
From: Eric Botcazou @ 2008-11-13 10:24 UTC (permalink / raw)
To: Dodji Seketeli; +Cc: gcc-patches, Jason Merrill, Jan Hubicka, Richard Guenther
> Please find attached a reworked patch that merges the two previous
> approaches.
PR debug/27574
* cgraph.h: New abstract_and_needed member to struct cgraph_node.
* cgraphunit.c (cgraph_analyze_functions): Flag abstract functions
- which clones are reachable - as "abstract and needed".
* cgraph.c (cgraph_release_function_body): If a node is "abstract and
needed",
do not release its DECL_INITIAL() content because that will be needed
to emit
debug info.
Lines are too long. And never use "because" in a ChangeLog entry as it should
only describe the "what", not the "why". The "why" should be in the sources.
--
Eric Botcazou
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH] PR c++/27574 (PING)
2008-11-13 10:24 ` Eric Botcazou
@ 2008-11-13 11:44 ` Dodji Seketeli
2008-11-14 19:52 ` Eric Botcazou
0 siblings, 1 reply; 32+ messages in thread
From: Dodji Seketeli @ 2008-11-13 11:44 UTC (permalink / raw)
To: Eric Botcazou; +Cc: gcc-patches, Jason Merrill, Jan Hubicka, Richard Guenther
Eric Botcazou a écrit :
[...]
> Lines are too long. And never use "because" in a ChangeLog entry as it should
> only describe the "what", not the "why". The "why" should be in the sources.
Fixed, thanks.
Dodji.
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH] PR c++/27574 (PING)
2008-11-13 11:44 ` Dodji Seketeli
@ 2008-11-14 19:52 ` Eric Botcazou
2008-11-14 20:01 ` Eric Botcazou
0 siblings, 1 reply; 32+ messages in thread
From: Eric Botcazou @ 2008-11-14 19:52 UTC (permalink / raw)
To: Dodji Seketeli; +Cc: gcc-patches, Jason Merrill, Jan Hubicka, Richard Guenther
> Fixed, thanks.
Another nit: the ChangeLog entry is incomplete, final.c is not mentioned.
--
Eric Botcazou
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH] PR c++/27574 (PING)
2008-11-14 19:52 ` Eric Botcazou
@ 2008-11-14 20:01 ` Eric Botcazou
0 siblings, 0 replies; 32+ messages in thread
From: Eric Botcazou @ 2008-11-14 20:01 UTC (permalink / raw)
To: Dodji Seketeli; +Cc: gcc-patches, Jason Merrill, Jan Hubicka, Richard Guenther
> Another nit: the ChangeLog entry is incomplete, final.c is not mentioned.
And the new comment line in final.c is too long, remove the superfluous ().
--
Eric Botcazou
^ permalink raw reply [flat|nested] 32+ messages in thread
end of thread, other threads:[~2008-11-14 19:35 UTC | newest]
Thread overview: 32+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2008-09-04 22:02 [PATCH] PR c++/27574 Dodji Seketeli
2008-09-05 8:56 ` Richard Guenther
2008-09-05 17:41 ` Jason Merrill
2008-09-23 18:57 ` Mark Mitchell
2008-09-24 23:15 ` Richard Guenther
2008-09-25 14:01 ` Mark Mitchell
2008-09-15 11:40 ` [PING] PATCH " Dodji Seketeli
2008-09-24 20:46 ` [PATCH] " Jason Merrill
2008-09-26 14:19 ` Dodji Seketeli
2008-09-26 23:51 ` Jason Merrill
2008-09-27 0:26 ` Jan Hubicka
2008-09-30 18:46 ` Jason Merrill
2008-10-06 19:46 ` Dodji Seketeli
2008-10-06 22:39 ` Jason Merrill
2008-10-09 7:54 ` Dodji Seketeli
2008-10-09 18:49 ` Jason Merrill
2008-11-03 11:28 ` [PATCH] PR c++/27574 (PING) Dodji Seketeli
2008-11-03 12:37 ` Richard Guenther
2008-11-03 13:56 ` Dodji Seketeli
2008-11-03 14:43 ` Richard Guenther
2008-11-03 17:14 ` Jan Hubicka
2008-11-03 18:02 ` Dodji Seketeli
2008-11-03 18:39 ` Jason Merrill
2008-11-03 20:56 ` Dodji Seketeli
2008-11-03 22:17 ` Jason Merrill
2008-11-04 21:14 ` Dodji Seketeli
2008-11-05 16:40 ` Jason Merrill
2008-11-13 10:24 ` Eric Botcazou
2008-11-13 11:44 ` Dodji Seketeli
2008-11-14 19:52 ` Eric Botcazou
2008-11-14 20:01 ` Eric Botcazou
2008-11-03 17:48 ` Dodji Seketeli
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).