public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] Add -dB option to disable backtraces
@ 2017-05-17  2:18 Andi Kleen
  2017-05-17  3:43 ` Andrew Pinski
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Andi Kleen @ 2017-05-17  2:18 UTC (permalink / raw)
  To: gcc-patches; +Cc: Andi Kleen

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(-)

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] Add -dB option to disable backtraces
  2017-05-17  2:18 [PATCH] Add -dB option to disable backtraces Andi Kleen
@ 2017-05-17  3:43 ` Andrew Pinski
  2017-05-17 14:49   ` Andi Kleen
  2017-05-17  5:26 ` Andrew Pinski
  2017-05-17  5:34 ` Markus Trippelsdorf
  2 siblings, 1 reply; 6+ messages in thread
From: Andrew Pinski @ 2017-05-17  3:43 UTC (permalink / raw)
  To: Andi Kleen; +Cc: GCC Patches, Andi Kleen

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
>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] Add -dB option to disable backtraces
  2017-05-17  2:18 [PATCH] Add -dB option to disable backtraces Andi Kleen
  2017-05-17  3:43 ` Andrew Pinski
@ 2017-05-17  5:26 ` Andrew Pinski
  2017-05-17  5:34 ` Markus Trippelsdorf
  2 siblings, 0 replies; 6+ messages in thread
From: Andrew Pinski @ 2017-05-17  5:26 UTC (permalink / raw)
  To: Andi Kleen; +Cc: GCC Patches, Andi Kleen

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
>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] Add -dB option to disable backtraces
  2017-05-17  2:18 [PATCH] Add -dB option to disable backtraces Andi Kleen
  2017-05-17  3:43 ` Andrew Pinski
  2017-05-17  5:26 ` Andrew Pinski
@ 2017-05-17  5:34 ` Markus Trippelsdorf
  2 siblings, 0 replies; 6+ messages in thread
From: Markus Trippelsdorf @ 2017-05-17  5:34 UTC (permalink / raw)
  To: Andi Kleen; +Cc: gcc-patches, Andi Kleen

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.

-- 
Markus

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] Add -dB option to disable backtraces
  2017-05-17  3:43 ` Andrew Pinski
@ 2017-05-17 14:49   ` Andi Kleen
  2017-05-17 14:59     ` Ian Lance Taylor via gcc-patches
  0 siblings, 1 reply; 6+ messages in thread
From: Andi Kleen @ 2017-05-17 14:49 UTC (permalink / raw)
  To: Andrew Pinski; +Cc: Andi Kleen, GCC Patches

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] Add -dB option to disable backtraces
  2017-05-17 14:49   ` Andi Kleen
@ 2017-05-17 14:59     ` Ian Lance Taylor via gcc-patches
  0 siblings, 0 replies; 6+ messages in thread
From: Ian Lance Taylor via gcc-patches @ 2017-05-17 14:59 UTC (permalink / raw)
  To: Andi Kleen; +Cc: Andrew Pinski, Andi Kleen, GCC Patches

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2017-05-17 14:49 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-05-17  2:18 [PATCH] Add -dB option to disable backtraces Andi Kleen
2017-05-17  3:43 ` Andrew Pinski
2017-05-17 14:49   ` Andi Kleen
2017-05-17 14:59     ` Ian Lance Taylor via gcc-patches
2017-05-17  5:26 ` Andrew Pinski
2017-05-17  5:34 ` Markus Trippelsdorf

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).