diff mbox

PING PATCH: break lines in announce_function

Message ID 20120516131833.GA839@ours.starynkevitch.net
State New
Headers show

Commit Message

Basile Starynkevitch May 16, 2012, 1:18 p.m. UTC
On Wed, May 16, 2012 at 03:02:39PM +0200, Richard Guenther wrote:
> On Wed, May 16, 2012 at 2:46 PM, Basile Starynkevitch
> <basile@starynkevitch.net> wrote:
> > Hello All,
> >
> > I am pinging the patch http://gcc.gnu.org/ml/gcc-patches/2012-02/msg00474.html
> > below for trunk svn 187587



> So - why?  I like it the way it is.

Because, as I explained in  http://gcc.gnu.org/ml/gcc-patches/2012-02/msg00474.html without that patch 
you have arbitrarily long output lines, and that is unpleasant, in particular when running under gdb 
or under emacs (also, there may be buffering issues: if GCC misbehave, stderr might not be flushed 
often enough...)

The announce_function is quite rarely really used (because quiet_flag is almost always true), 
and it is used mostly to debug GCC (or plugins), and in that case having not too large output 
is quite useful in practice. The patch above is quick & dirty but seems enough.
Do you want me to add a comment like /* Hack to avoid very large output lines.  */ before?

Regards.

Comments

Richard Biener May 16, 2012, 1:29 p.m. UTC | #1
On Wed, May 16, 2012 at 3:18 PM, Basile Starynkevitch
<basile@starynkevitch.net> wrote:
> On Wed, May 16, 2012 at 03:02:39PM +0200, Richard Guenther wrote:
>> On Wed, May 16, 2012 at 2:46 PM, Basile Starynkevitch
>> <basile@starynkevitch.net> wrote:
>> > Hello All,
>> >
>> > I am pinging the patch http://gcc.gnu.org/ml/gcc-patches/2012-02/msg00474.html
>> > below for trunk svn 187587
>
> --- gcc/toplev.c        (revision 187587)
> +++ gcc/toplev.c        (working copy)
> @@ -229,6 +229,11 @@ announce_function (tree decl)
>  {
>   if (!quiet_flag)
>     {
> +      static long count;
> +      count++;
> +      if (count % 8 == 0)
> +        putc('\n', stderr);
> +
>       if (rtl_dump_and_exit)
>        fprintf (stderr, "%s ",
>                 identifier_to_locale (IDENTIFIER_POINTER (DECL_NAME (decl))));
>
>
>> So - why?  I like it the way it is.
>
> Because, as I explained in  http://gcc.gnu.org/ml/gcc-patches/2012-02/msg00474.html without that patch
> you have arbitrarily long output lines, and that is unpleasant, in particular when running under gdb
> or under emacs (also, there may be buffering issues: if GCC misbehave, stderr might not be flushed
> often enough...)

stderr is unbuffered

> The announce_function is quite rarely really used (because quiet_flag is almost always true),
> and it is used mostly to debug GCC (or plugins), and in that case having not too large output
> is quite useful in practice. The patch above is quick & dirty but seems enough.
> Do you want me to add a comment like /* Hack to avoid very large output lines.  */ before?

No, I want the large line to stay as-is.  It's pleasant for me.

Richard.

> Regards.
>
>
> --
> Basile STARYNKEVITCH         http://starynkevitch.net/Basile/
> email: basile<at>starynkevitch<dot>net mobile: +33 6 8501 2359
> 8, rue de la Faiencerie, 92340 Bourg La Reine, France
> *** opinions {are only mines, sont seulement les miennes} ***
Basile Starynkevitch May 16, 2012, 1:38 p.m. UTC | #2
On Wed, May 16, 2012 at 03:29:12PM +0200, Richard Guenther wrote:
> On Wed, May 16, 2012 at 3:18 PM, Basile Starynkevitch
> <basile@starynkevitch.net> wrote:
> > On Wed, May 16, 2012 at 03:02:39PM +0200, Richard Guenther wrote:
> >> On Wed, May 16, 2012 at 2:46 PM, Basile Starynkevitch
> >> <basile@starynkevitch.net> wrote:
> >> > Hello All,
> >> >
> >> > I am pinging the patch http://gcc.gnu.org/ml/gcc-patches/2012-02/msg00474.html
> >> > below for trunk svn 187587
> >
> > --- gcc/toplev.c        (revision 187587)
> > +++ gcc/toplev.c        (working copy)
> > @@ -229,6 +229,11 @@ announce_function (tree decl)
> >  {
> >   if (!quiet_flag)
> >     {
> > +      static long count;
> > +      count++;
> > +      if (count % 8 == 0)
> > +        putc('\n', stderr);
> > +
> >       if (rtl_dump_and_exit)
> >        fprintf (stderr, "%s ",
> >                 identifier_to_locale (IDENTIFIER_POINTER (DECL_NAME (decl))));
> >
> >
> >> So - why?  I like it the way it is.
> >
> > Because, as I explained in  http://gcc.gnu.org/ml/gcc-patches/2012-02/msg00474.html without that patch
> > you have arbitrarily long output lines, and that is unpleasant, in particular when running under gdb
> > or under emacs (also, there may be buffering issues: if GCC misbehave, stderr might not be flushed
> > often enough...)
> 
> stderr is unbuffered
> 
> > The announce_function is quite rarely really used (because quiet_flag is almost always true),
> > and it is used mostly to debug GCC (or plugins), and in that case having not too large output
> > is quite useful in practice. The patch above is quick & dirty but seems enough.
> > Do you want me to add a comment like /* Hack to avoid very large output lines.  */ before?
> 
> No, I want the large line to stay as-is.  It's pleasant for me.


Do you have any hints or tricks to have this work well when running cc1 under emacs 
(eg in a *shell* or *gud-cc1* Emacs buffer)?

Without that patch displaying happen too late (and eats a lot of Emacs CPU)!!

Regards
Richard Biener May 16, 2012, 1:40 p.m. UTC | #3
On Wed, May 16, 2012 at 3:38 PM, Basile Starynkevitch
<basile@starynkevitch.net> wrote:
> On Wed, May 16, 2012 at 03:29:12PM +0200, Richard Guenther wrote:
>> On Wed, May 16, 2012 at 3:18 PM, Basile Starynkevitch
>> <basile@starynkevitch.net> wrote:
>> > On Wed, May 16, 2012 at 03:02:39PM +0200, Richard Guenther wrote:
>> >> On Wed, May 16, 2012 at 2:46 PM, Basile Starynkevitch
>> >> <basile@starynkevitch.net> wrote:
>> >> > Hello All,
>> >> >
>> >> > I am pinging the patch http://gcc.gnu.org/ml/gcc-patches/2012-02/msg00474.html
>> >> > below for trunk svn 187587
>> >
>> > --- gcc/toplev.c        (revision 187587)
>> > +++ gcc/toplev.c        (working copy)
>> > @@ -229,6 +229,11 @@ announce_function (tree decl)
>> >  {
>> >   if (!quiet_flag)
>> >     {
>> > +      static long count;
>> > +      count++;
>> > +      if (count % 8 == 0)
>> > +        putc('\n', stderr);
>> > +
>> >       if (rtl_dump_and_exit)
>> >        fprintf (stderr, "%s ",
>> >                 identifier_to_locale (IDENTIFIER_POINTER (DECL_NAME (decl))));
>> >
>> >
>> >> So - why?  I like it the way it is.
>> >
>> > Because, as I explained in  http://gcc.gnu.org/ml/gcc-patches/2012-02/msg00474.html without that patch
>> > you have arbitrarily long output lines, and that is unpleasant, in particular when running under gdb
>> > or under emacs (also, there may be buffering issues: if GCC misbehave, stderr might not be flushed
>> > often enough...)
>>
>> stderr is unbuffered
>>
>> > The announce_function is quite rarely really used (because quiet_flag is almost always true),
>> > and it is used mostly to debug GCC (or plugins), and in that case having not too large output
>> > is quite useful in practice. The patch above is quick & dirty but seems enough.
>> > Do you want me to add a comment like /* Hack to avoid very large output lines.  */ before?
>>
>> No, I want the large line to stay as-is.  It's pleasant for me.
>
>
> Do you have any hints or tricks to have this work well when running cc1 under emacs
> (eg in a *shell* or *gud-cc1* Emacs buffer)?
>
> Without that patch displaying happen too late (and eats a lot of Emacs CPU)!!

1) Fix emacs (do not buffer stderr)
2) do not omit -quiet when running from inside emacs

Richard.
Manuel López-Ibáñez May 16, 2012, 1:56 p.m. UTC | #4
On 16 May 2012 15:40, Richard Guenther <richard.guenther@gmail.com> wrote:
>>
>> Without that patch displaying happen too late (and eats a lot of Emacs CPU)!!
>
> 1) Fix emacs (do not buffer stderr)
> 2) do not omit -quiet when running from inside emacs

Actually, I wonder how you (Richard) and other GCC hackers work with
and debug GCC, because it is a real pain in the ass.

* All the TREE_ macros don't work.

* __extension__ prevents GDB from evaluating many things.

* announce_function dumps slow down Emacs (and the shell when working
via SSH) when debugging anything related to libstdc++ (or any large
testcase).

* Hitting auto-completion in GDB means staring at the window for 5-10
minutes until it decides to stop listing stuff.

There are quite a few other annoyances, but these are the most common
(perhaps I should make a list in the wiki).

Cheers,

Manuel.
Richard Biener May 16, 2012, 2:02 p.m. UTC | #5
On Wed, May 16, 2012 at 3:56 PM, Manuel López-Ibáñez
<lopezibanez@gmail.com> wrote:
> On 16 May 2012 15:40, Richard Guenther <richard.guenther@gmail.com> wrote:
>>>
>>> Without that patch displaying happen too late (and eats a lot of Emacs CPU)!!
>>
>> 1) Fix emacs (do not buffer stderr)
>> 2) do not omit -quiet when running from inside emacs
>
> Actually, I wonder how you (Richard) and other GCC hackers work with
> and debug GCC, because it is a real pain in the ass.
>
> * All the TREE_ macros don't work.

I have memoized all sub-fields of tree, so I don't use those macros
(still other people use them with some gdbinit macdefs and -g3).

> * __extension__ prevents GDB from evaluating many things.

I did not run into this one yet - maybe open a GDB bug?

> * announce_function dumps slow down Emacs (and the shell when working
> via SSH) when debugging anything related to libstdc++ (or any large
> testcase).

I always use -quiet, even when debugging.  I do not use -quite when tracking
down compile-time hog issues.

> * Hitting auto-completion in GDB means staring at the window for 5-10
> minutes until it decides to stop listing stuff.

That works for me.  Though my debug-cc1 is built with a C compiler
(yes, with C++ this will get a pain in the ass ...)

> There are quite a few other annoyances, but these are the most common
> (perhaps I should make a list in the wiki).

The most common annoyance for me is stepping into trivial inline functions
when trying to step into an "important function", thus

  foo (tree_code_length (x), a, b);

something people want to make even more prominent :(

Richard.

> Cheers,
>
> Manuel.
Manuel López-Ibáñez May 16, 2012, 3:57 p.m. UTC | #6
On 16 May 2012 16:02, Richard Guenther <richard.guenther@gmail.com> wrote:
> On Wed, May 16, 2012 at 3:56 PM, Manuel López-Ibáñez
> <lopezibanez@gmail.com> wrote:
>> On 16 May 2012 15:40, Richard Guenther <richard.guenther@gmail.com> wrote:
>>>>
>>>> Without that patch displaying happen too late (and eats a lot of Emacs CPU)!!
>>>
>>> 1) Fix emacs (do not buffer stderr)
>>> 2) do not omit -quiet when running from inside emacs
>>
>> Actually, I wonder how you (Richard) and other GCC hackers work with
>> and debug GCC, because it is a real pain in the ass.
>>
>> * All the TREE_ macros don't work.
>
> I have memoized all sub-fields of tree, so I don't use those macros
> (still other people use them with some gdbinit macdefs and -g3).

Could these macdefs be loaded by default?

I see now that any change in the underlying implementation becomes a
serious annoyance for you.

>
>> * __extension__ prevents GDB from evaluating many things.
>
> I did not run into this one yet - maybe open a GDB bug?

It seems it will never work for statement expressions:

http://article.gmane.org/gmane.comp.gcc.devel/107339

>> * announce_function dumps slow down Emacs (and the shell when working
>> via SSH) when debugging anything related to libstdc++ (or any large
>> testcase).
>
> I always use -quiet, even when debugging.  I do not use -quite when tracking
> down compile-time hog issues.

So why not make that the default?

>> * Hitting auto-completion in GDB means staring at the window for 5-10
>> minutes until it decides to stop listing stuff.
>
> That works for me.  Though my debug-cc1 is built with a C compiler
> (yes, with C++ this will get a pain in the ass ...)
>
>> There are quite a few other annoyances, but these are the most common
>> (perhaps I should make a list in the wiki).
>
> The most common annoyance for me is stepping into trivial inline functions
> when trying to step into an "important function", thus
>
>  foo (tree_code_length (x), a, b);
>
> something people want to make even more prominent :(

I guess 'n' doesn't work because they are really inlined. Open a GDB
bug? I guess this answer does not work both ways... :-(

I see how these small functions can quickly become annoying. Is it
really that hard to make GDB ignore some functions?

On a more general note, there is really a conflict between the
old-timers who know every little detail and have grown their custom
recipes for going around annoyances, so they resist any change that
breaks those recipes or their hard-won knowledge of GCC internals. And
any new potential contributor, who feels that trivial things are
extremely hard and time-consuming and wants to make things simpler and
easier. Of course, it is not really a conflict. New contributors have
little saying, which is normal because obviously they contribute the
least. However, the mere fact that it is hard to contribute prevents
them to contribute more. So we have 2 or 3 people that do 90% of the
work, and not a long tail but a very small number of people that
contribute the other 10%. Not a good prospect for the future...

On this topic, I found this talk particularly interesting:
http://percival-music.ca/blog/2010-08-01-sustainable-development.html

However, I don't have any solutions to offer, but the situation has
been a source of frustration for me since a long time, and I think
also for others.

Cheers,

Manuel.
Tom Tromey May 16, 2012, 6:41 p.m. UTC | #7
>>>>> "Manuel" == Manuel López-Ibáñez <lopezibanez@gmail.com> writes:

Manuel> Actually, I wonder how you (Richard) and other GCC hackers work with
Manuel> and debug GCC, because it is a real pain in the ass.

Manuel> * All the TREE_ macros don't work.

Manuel> * __extension__ prevents GDB from evaluating many things.

See that other thread...

When I was debugging gcc regularly I added a bunch of "macro define"s to
.gdbinit, and built with -g3.  This fixed the macro problem and the
__extension__ problem.

Manuel> * Hitting auto-completion in GDB means staring at the window for 5-10
Manuel> minutes until it decides to stop listing stuff.

Report completion bugs to gdb.  There's only so much gdb can do here.
But maybe we could have some mode to be more selective about what to
complete.

Tom
Tom Tromey May 16, 2012, 6:44 p.m. UTC | #8
>>>>> "Manuel" == Manuel López-Ibáñez <lopezibanez@gmail.com> writes:

Manuel> It seems it will never work for statement expressions:
Manuel> http://article.gmane.org/gmane.comp.gcc.devel/107339

It could be done, but it is non-trivial for sure.

Manuel> I see how these small functions can quickly become annoying. Is it
Manuel> really that hard to make GDB ignore some functions?

There's a new "skip" feature in gdb specifically for this.
It is in gdb 7.4.

Tom
Manuel López-Ibáñez May 17, 2012, 11:54 a.m. UTC | #9
On 16 May 2012 20:41, Tom Tromey <tromey@redhat.com> wrote:
>
> Manuel> * Hitting auto-completion in GDB means staring at the window for 5-10
> Manuel> minutes until it decides to stop listing stuff.
>
> Report completion bugs to gdb.  There's only so much gdb can do here.
> But maybe we could have some mode to be more selective about what to
> complete.

Well, it could do like bash. Give a message if there are more than N
options and ask y/n, no?

Cheers,

Manuel.
Mike Stump May 17, 2012, 3:24 p.m. UTC | #10
On May 16, 2012, at 6:56 AM, Manuel López-Ibáñez <lopezibanez@gmail.com> wrote:
> On 16 May 2012 15:40, Richard Guenther <richard.guenther@gmail.com> wrote:
>>> 
>>> Without that patch displaying happen too late (and eats a lot of Emacs CPU)!!
>> 
>> 1) Fix emacs (do not buffer stderr)
>> 2) do not omit -quiet when running from inside emacs
> 
> Actually, I wonder how you (Richard) and other GCC hackers work with
> and debug GCC, because it is a real pain in the ass.
> 
> * All the TREE_ macros don't work.
> * __extension__ prevents GDB from evaluating many things.

I posted a solution to this just this week.  If you like it, make someone check it in.  The concept of using inline functions in C has already been proposed and agreed on in spirit.

> * announce_function dumps slow down Emacs (and the shell when working
> via SSH) when debugging anything related to libstdc++ (or any large
> testcase).

Let's make -quiet the default.  People that want the spew, can ask for it.  I, 99 times out of 100 don't want it.
Gabriel Dos Reis May 17, 2012, 3:34 p.m. UTC | #11
On Thu, May 17, 2012 at 10:24 AM, Mike Stump <mikestump@comcast.net> wrote:

>> * announce_function dumps slow down Emacs (and the shell when working
>> via SSH) when debugging anything related to libstdc++ (or any large
>> testcase).
>
> Let's make -quiet the default.

Agreed.

(I had always reflexively used -quiet so that it wasn't a proble, ofr me.)

-- Gaby
Mike Stump May 17, 2012, 3:46 p.m. UTC | #12
On May 16, 2012, at 8:57 AM, Manuel López-Ibáñez <lopezibanez@gmail.com> wrote:
> I see now that any change in the underlying implementation becomes a
> serious annoyance for you.

No, it is easy, we just need a low cost solution by which people can use the normal accessors.  Once that is provided, then I think it is easy to change the underlying bits.  Though, it is kinda sad that 20 years later, things are still as they were, with no improvement.  I wish the steering committe would find a way to allow improvements to be made more easily in this area.  I would have checked in my changes to support -g3 debugging if it were easier.  I'll note, I'm not the first person to do the work, just the last.
Mike Stump May 17, 2012, 3:48 p.m. UTC | #13
On May 16, 2012, at 11:44 AM, Tom Tromey <tromey@redhat.com> wrote:

>>>>>> "Manuel" == Manuel López-Ibáñez <lopezibanez@gmail.com> writes:
> 
> Manuel> It seems it will never work for statement expressions:
> Manuel> http://article.gmane.org/gmane.comp.gcc.devel/107339
> 
> It could be done, but it is non-trivial for sure.
> 
> Manuel> I see how these small functions can quickly become annoying. Is it
> Manuel> really that hard to make GDB ignore some functions?
> 
> There's a new "skip" feature in gdb specifically for this.
> It is in gdb 7.4.

Or, you can mark them with an attribute, if you use Darwin, we've had the feature for almost a decade.
diff mbox

Patch

--- gcc/toplev.c	(revision 187587)
+++ gcc/toplev.c	(working copy)
@@ -229,6 +229,11 @@  announce_function (tree decl)
 {
   if (!quiet_flag)
     {
+      static long count;
+      count++;
+      if (count % 8 == 0)
+        putc('\n', stderr);
+
       if (rtl_dump_and_exit)
 	fprintf (stderr, "%s ",
 		 identifier_to_locale (IDENTIFIER_POINTER (DECL_NAME (decl))));