| Message ID | 20170517021647.8495-1-andi@firstfloor.org |
|---|---|
| State | New |
| Headers | show |
On Tue, May 16, 2017 at 7:16 PM, Andi Kleen <andi@firstfloor.org> wrote: > From: Andi Kleen <ak@linux.intel.com> > > When running creduce on an ICE substantial amounts of the total > CPU time go to backtrace_qsort() (sorting dwarf of the compiler) for > printing the backtrace of the ICE. When running a reduction we don't need the > backtrace. So add a -dB option to turn it off, and make reduction > a bit faster. The other thing which you could is strip the binaries. :) j/k. I think this is a good patch and a good idea. Thanks, Andrew > > Passes bootstrap and testing on x86_64-linux > > gcc/: > > 2017-05-16 Andi Kleen <ak@linux.intel.com> > > * diagnostic.c (diagnostic_initialize): Set disable_backtrace to > false. > (diagnostic_action_after_output): Check disable_backtrace. > * diagnostic.h (struct diagnostic_context): Add > disable_backtrace. > * doc/invoke.texi (-dB): Document. > * opts.c (decode_d_option): Handle -dB. > --- > gcc/diagnostic.c | 16 ++++++++++------ > gcc/diagnostic.h | 7 +++++++ > gcc/doc/invoke.texi | 6 ++++++ > gcc/opts.c | 3 +++ > 4 files changed, 26 insertions(+), 6 deletions(-) > > diff --git a/gcc/diagnostic.c b/gcc/diagnostic.c > index 158519606f8..bbac3f384d4 100644 > --- a/gcc/diagnostic.c > +++ b/gcc/diagnostic.c > @@ -156,6 +156,7 @@ diagnostic_initialize (diagnostic_context *context, int n_opts) > context->caret_chars[i] = '^'; > context->show_option_requested = false; > context->abort_on_error = false; > + context->disable_backtrace = false; > context->show_column = false; > context->pedantic_errors = false; > context->permissive = false; > @@ -500,13 +501,16 @@ diagnostic_action_after_output (diagnostic_context *context, > case DK_ICE: > case DK_ICE_NOBT: > { > - struct backtrace_state *state = NULL; > - if (diag_kind == DK_ICE) > - state = backtrace_create_state (NULL, 0, bt_err_callback, NULL); > int count = 0; > - if (state != NULL) > - backtrace_full (state, 2, bt_callback, bt_err_callback, > - (void *) &count); > + if (!context->disable_backtrace) > + { > + struct backtrace_state *state = NULL; > + if (diag_kind == DK_ICE) > + state = backtrace_create_state (NULL, 0, bt_err_callback, NULL); > + if (state != NULL) > + backtrace_full (state, 2, bt_callback, bt_err_callback, > + (void *) &count); > + } > > if (context->abort_on_error) > real_abort (); > diff --git a/gcc/diagnostic.h b/gcc/diagnostic.h > index dbd1703e0ef..440b547c6da 100644 > --- a/gcc/diagnostic.h > +++ b/gcc/diagnostic.h > @@ -120,6 +120,9 @@ struct diagnostic_context > /* True if we should raise a SIGABRT on errors. */ > bool abort_on_error; > > + /* If true don't print backtraces for internal errors. */ > + bool disable_backtrace; > + > /* True if we should show the column number on diagnostics. */ > bool show_column; > > @@ -245,6 +248,10 @@ diagnostic_inhibit_notes (diagnostic_context * context) > #define diagnostic_abort_on_error(DC) \ > (DC)->abort_on_error = true > > +/* Don't print backtraces for internal errors. */ > +#define diagnostic_disable_backtrace(DC) \ > + (DC)->disable_backtrace = true; > + > /* This diagnostic_context is used by front-ends that directly output > diagnostic messages without going through `error', `warning', > and similar functions. */ > diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi > index 715830a1a43..677e78c7078 100644 > --- a/gcc/doc/invoke.texi > +++ b/gcc/doc/invoke.texi > @@ -12889,6 +12889,12 @@ Produce all the dumps listed above. > @opindex dA > Annotate the assembler output with miscellaneous debugging information. > > +@item -dB > +@opindex dB > +Do not print backtraces on compiler crashes. This speeds up reducing > +test cases for compiler crashes, because printing backtraces is relatively > +slow. > + > @item -dD > @opindex dD > Dump all macro definitions, at the end of preprocessing, in addition to > diff --git a/gcc/opts.c b/gcc/opts.c > index ffedb10f18f..0c1da107714 100644 > --- a/gcc/opts.c > +++ b/gcc/opts.c > @@ -2593,6 +2593,9 @@ decode_d_option (const char *arg, struct gcc_options *opts, > case 'a': > opts->x_flag_dump_all_passed = true; > break; > + case 'B': > + diagnostic_disable_backtrace (dc); > + break; > > default: > warning_at (loc, 0, "unrecognized gcc debugging option: %c", c); > -- > 2.12.2 >
On Tue, May 16, 2017 at 7:16 PM, Andi Kleen <andi@firstfloor.org> wrote: > From: Andi Kleen <ak@linux.intel.com> > > When running creduce on an ICE substantial amounts of the total > CPU time go to backtrace_qsort() (sorting dwarf of the compiler) for > printing the backtrace of the ICE. When running a reduction we don't need the > backtrace. So add a -dB option to turn it off, and make reduction > a bit faster. Now let me comment on the patch itself because I think it can be simplified a lot. > > Passes bootstrap and testing on x86_64-linux > > gcc/: > > 2017-05-16 Andi Kleen <ak@linux.intel.com> > > * diagnostic.c (diagnostic_initialize): Set disable_backtrace to > false. > (diagnostic_action_after_output): Check disable_backtrace. > * diagnostic.h (struct diagnostic_context): Add > disable_backtrace. > * doc/invoke.texi (-dB): Document. > * opts.c (decode_d_option): Handle -dB. > --- > gcc/diagnostic.c | 16 ++++++++++------ > gcc/diagnostic.h | 7 +++++++ > gcc/doc/invoke.texi | 6 ++++++ > gcc/opts.c | 3 +++ > 4 files changed, 26 insertions(+), 6 deletions(-) > > diff --git a/gcc/diagnostic.c b/gcc/diagnostic.c > index 158519606f8..bbac3f384d4 100644 > --- a/gcc/diagnostic.c > +++ b/gcc/diagnostic.c > @@ -156,6 +156,7 @@ diagnostic_initialize (diagnostic_context *context, int n_opts) > context->caret_chars[i] = '^'; > context->show_option_requested = false; > context->abort_on_error = false; > + context->disable_backtrace = false; > context->show_column = false; > context->pedantic_errors = false; > context->permissive = false; > @@ -500,13 +501,16 @@ diagnostic_action_after_output (diagnostic_context *context, > case DK_ICE: > case DK_ICE_NOBT: > { > - struct backtrace_state *state = NULL; > - if (diag_kind == DK_ICE) > - state = backtrace_create_state (NULL, 0, bt_err_callback, NULL); > int count = 0; > - if (state != NULL) > - backtrace_full (state, 2, bt_callback, bt_err_callback, > - (void *) &count); > + if (!context->disable_backtrace) > + { > + struct backtrace_state *state = NULL; > + if (diag_kind == DK_ICE) Why not put && !context->disable_backtrace in this if statement instead of wrapping the whole thing in an another if statement? That is: struct backtrace_state *state = NULL; if (diag_kind == DK_ICE && !context->disable_backtrace) state = backtrace_create_state (NULL, 0, bt_err_callback, NULL); int count = 0; if (state != NULL) backtrace_full (state, 2, bt_callback, bt_err_callback, (void *) &count); Changing only one line and instead of many. Thanks, Andrew Pinski > + state = backtrace_create_state (NULL, 0, bt_err_callback, NULL); > + if (state != NULL) > + backtrace_full (state, 2, bt_callback, bt_err_callback, > + (void *) &count); > + } > > if (context->abort_on_error) > real_abort (); > diff --git a/gcc/diagnostic.h b/gcc/diagnostic.h > index dbd1703e0ef..440b547c6da 100644 > --- a/gcc/diagnostic.h > +++ b/gcc/diagnostic.h > @@ -120,6 +120,9 @@ struct diagnostic_context > /* True if we should raise a SIGABRT on errors. */ > bool abort_on_error; > > + /* If true don't print backtraces for internal errors. */ > + bool disable_backtrace; > + > /* True if we should show the column number on diagnostics. */ > bool show_column; > > @@ -245,6 +248,10 @@ diagnostic_inhibit_notes (diagnostic_context * context) > #define diagnostic_abort_on_error(DC) \ > (DC)->abort_on_error = true > > +/* Don't print backtraces for internal errors. */ > +#define diagnostic_disable_backtrace(DC) \ > + (DC)->disable_backtrace = true; > + > /* This diagnostic_context is used by front-ends that directly output > diagnostic messages without going through `error', `warning', > and similar functions. */ > diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi > index 715830a1a43..677e78c7078 100644 > --- a/gcc/doc/invoke.texi > +++ b/gcc/doc/invoke.texi > @@ -12889,6 +12889,12 @@ Produce all the dumps listed above. > @opindex dA > Annotate the assembler output with miscellaneous debugging information. > > +@item -dB > +@opindex dB > +Do not print backtraces on compiler crashes. This speeds up reducing > +test cases for compiler crashes, because printing backtraces is relatively > +slow. > + > @item -dD > @opindex dD > Dump all macro definitions, at the end of preprocessing, in addition to > diff --git a/gcc/opts.c b/gcc/opts.c > index ffedb10f18f..0c1da107714 100644 > --- a/gcc/opts.c > +++ b/gcc/opts.c > @@ -2593,6 +2593,9 @@ decode_d_option (const char *arg, struct gcc_options *opts, > case 'a': > opts->x_flag_dump_all_passed = true; > break; > + case 'B': > + diagnostic_disable_backtrace (dc); > + break; > > default: > warning_at (loc, 0, "unrecognized gcc debugging option: %c", c); > -- > 2.12.2 >
On 2017.05.16 at 19:16 -0700, Andi Kleen wrote: > From: Andi Kleen <ak@linux.intel.com> > > When running creduce on an ICE substantial amounts of the total > CPU time go to backtrace_qsort() (sorting dwarf of the compiler) for > printing the backtrace of the ICE. When running a reduction we don't need the > backtrace. So add a -dB option to turn it off, and make reduction > a bit faster. It is useful when reducing compiler segfaults, because here one should grep for a symbol in the backtrace to not end up with an unrelated crash.
On Tue, May 16, 2017 at 08:40:02PM -0700, Andrew Pinski wrote: > On Tue, May 16, 2017 at 7:16 PM, Andi Kleen <andi@firstfloor.org> wrote: > > From: Andi Kleen <ak@linux.intel.com> > > > > When running creduce on an ICE substantial amounts of the total > > CPU time go to backtrace_qsort() (sorting dwarf of the compiler) for > > printing the backtrace of the ICE. When running a reduction we don't need the > > backtrace. So add a -dB option to turn it off, and make reduction > > a bit faster. > > The other thing which you could is strip the binaries. :) > j/k. I think this is a good patch and a good idea. AFAIK the sort is for the unwind tables. strip removes .dwarf*, but not .eh_* It can't because that would break C++ exceptions. -Andi
On Wed, May 17, 2017 at 7:13 AM, Andi Kleen <ak@linux.intel.com> wrote: > On Tue, May 16, 2017 at 08:40:02PM -0700, Andrew Pinski wrote: >> On Tue, May 16, 2017 at 7:16 PM, Andi Kleen <andi@firstfloor.org> wrote: >> > From: Andi Kleen <ak@linux.intel.com> >> > >> > When running creduce on an ICE substantial amounts of the total >> > CPU time go to backtrace_qsort() (sorting dwarf of the compiler) for >> > printing the backtrace of the ICE. When running a reduction we don't need the >> > backtrace. So add a -dB option to turn it off, and make reduction >> > a bit faster. >> >> The other thing which you could is strip the binaries. :) >> j/k. I think this is a good patch and a good idea. > > AFAIK the sort is for the unwind tables. strip removes .dwarf*, but not .eh_* > It can't because that would break C++ exceptions. This is libbacktrace. The calls to backtrace_qsort are for the DWARF information. libbacktrace sorts the debug info so it can do faster lookups of PC values. That said I'm surprised it takes that much time. The DWARF info is usually mostly sorted, and backtrace_qsort is optimized for information that is mostly sorted. If you feel like spending time on this it might be worth figuring out why it is slow. Ian
diff --git a/gcc/diagnostic.c b/gcc/diagnostic.c index 158519606f8..bbac3f384d4 100644 --- a/gcc/diagnostic.c +++ b/gcc/diagnostic.c @@ -156,6 +156,7 @@ diagnostic_initialize (diagnostic_context *context, int n_opts) context->caret_chars[i] = '^'; context->show_option_requested = false; context->abort_on_error = false; + context->disable_backtrace = false; context->show_column = false; context->pedantic_errors = false; context->permissive = false; @@ -500,13 +501,16 @@ diagnostic_action_after_output (diagnostic_context *context, case DK_ICE: case DK_ICE_NOBT: { - struct backtrace_state *state = NULL; - if (diag_kind == DK_ICE) - state = backtrace_create_state (NULL, 0, bt_err_callback, NULL); int count = 0; - if (state != NULL) - backtrace_full (state, 2, bt_callback, bt_err_callback, - (void *) &count); + if (!context->disable_backtrace) + { + struct backtrace_state *state = NULL; + if (diag_kind == DK_ICE) + state = backtrace_create_state (NULL, 0, bt_err_callback, NULL); + if (state != NULL) + backtrace_full (state, 2, bt_callback, bt_err_callback, + (void *) &count); + } if (context->abort_on_error) real_abort (); diff --git a/gcc/diagnostic.h b/gcc/diagnostic.h index dbd1703e0ef..440b547c6da 100644 --- a/gcc/diagnostic.h +++ b/gcc/diagnostic.h @@ -120,6 +120,9 @@ struct diagnostic_context /* True if we should raise a SIGABRT on errors. */ bool abort_on_error; + /* If true don't print backtraces for internal errors. */ + bool disable_backtrace; + /* True if we should show the column number on diagnostics. */ bool show_column; @@ -245,6 +248,10 @@ diagnostic_inhibit_notes (diagnostic_context * context) #define diagnostic_abort_on_error(DC) \ (DC)->abort_on_error = true +/* Don't print backtraces for internal errors. */ +#define diagnostic_disable_backtrace(DC) \ + (DC)->disable_backtrace = true; + /* This diagnostic_context is used by front-ends that directly output diagnostic messages without going through `error', `warning', and similar functions. */ diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi index 715830a1a43..677e78c7078 100644 --- a/gcc/doc/invoke.texi +++ b/gcc/doc/invoke.texi @@ -12889,6 +12889,12 @@ Produce all the dumps listed above. @opindex dA Annotate the assembler output with miscellaneous debugging information. +@item -dB +@opindex dB +Do not print backtraces on compiler crashes. This speeds up reducing +test cases for compiler crashes, because printing backtraces is relatively +slow. + @item -dD @opindex dD Dump all macro definitions, at the end of preprocessing, in addition to diff --git a/gcc/opts.c b/gcc/opts.c index ffedb10f18f..0c1da107714 100644 --- a/gcc/opts.c +++ b/gcc/opts.c @@ -2593,6 +2593,9 @@ decode_d_option (const char *arg, struct gcc_options *opts, case 'a': opts->x_flag_dump_all_passed = true; break; + case 'B': + diagnostic_disable_backtrace (dc); + break; default: warning_at (loc, 0, "unrecognized gcc debugging option: %c", c);
From: Andi Kleen <ak@linux.intel.com> When running creduce on an ICE substantial amounts of the total CPU time go to backtrace_qsort() (sorting dwarf of the compiler) for printing the backtrace of the ICE. When running a reduction we don't need the backtrace. So add a -dB option to turn it off, and make reduction a bit faster. Passes bootstrap and testing on x86_64-linux gcc/: 2017-05-16 Andi Kleen <ak@linux.intel.com> * diagnostic.c (diagnostic_initialize): Set disable_backtrace to false. (diagnostic_action_after_output): Check disable_backtrace. * diagnostic.h (struct diagnostic_context): Add disable_backtrace. * doc/invoke.texi (-dB): Document. * opts.c (decode_d_option): Handle -dB. --- gcc/diagnostic.c | 16 ++++++++++------ gcc/diagnostic.h | 7 +++++++ gcc/doc/invoke.texi | 6 ++++++ gcc/opts.c | 3 +++ 4 files changed, 26 insertions(+), 6 deletions(-)