| Message ID | d305514e-74da-64b3-8779-7bbed7cd3f94@suse.cz |
|---|---|
| State | New |
| Headers | show |
| Series | Support lower and upper limit for -fdbg-cnt flag. | expand |
On Wed, 16 May 2018, Martin Liška wrote: > Hi. > > I consider it handy sometimes to trigger just a single invocation of > an optimization driven by a debug counter. Doing that one needs to > be able to limit both lower and upper limit of a counter. It's implemented > in the patch. I'd like to offer some non-reviewer comments on the patch (below) > --- a/gcc/common.opt > +++ b/gcc/common.opt > @@ -1171,7 +1171,7 @@ List all available debugging counters with their limits and counts. > > fdbg-cnt= > Common RejectNegative Joined Var(common_deferred_options) Defer > --fdbg-cnt=<counter>:<limit>[,<counter>:<limit>,...] Set the debug counter limit. > +-fdbg-cnt=<counter>[:<lower_limit>]:<upper_limit>[,<counter>:<lower_limit>:<upper_limit>,...] Set the debug counter limit. This line has gotten quite long and repeating the same thing in the second brackets is not very helpful. Can we use something simpler like this? -fdbg-cnt=<counter>[:<lower_limit>]:<upper_limit>[,<counter>:...] > +#define DEBUG_COUNTER(a) 1, > +static unsigned int limit_low[debug_counter_number_of_counters] = > +{ > +#include "dbgcnt.def" > +}; > +#undef DEBUG_COUNTER > + > + > static unsigned int count[debug_counter_number_of_counters]; > > bool > dbg_cnt_is_enabled (enum debug_counter index) > { > - return count[index] <= limit[index]; > + return limit_low[index] <= count[index] && count[index] <= limit_high[index]; I recall Jakub recently applied a tree-wide change of A < B && B < C to read B > A && B < C. Please consider making limit_low non-inclusive by testing for strict inequality count[index] > limit_low[index]. This will allow to make limit_low[] array zero-initialized (taking up space only in BSS). > --- a/gcc/doc/invoke.texi > +++ b/gcc/doc/invoke.texi > @@ -14326,13 +14326,14 @@ Print the name and the counter upper bound for all debug counters. > > @item -fdbg-cnt=@var{counter-value-list} > @opindex fdbg-cnt > -Set the internal debug counter upper bound. @var{counter-value-list} > -is a comma-separated list of @var{name}:@var{value} pairs > -which sets the upper bound of each debug counter @var{name} to @var{value}. > +Set the internal debug counter lower and upper bound. @var{counter-value-list} > +is a comma-separated list of @var{name}:@var{lower_bound}:@var{upper_bound} > +tuples which sets the lower and the upper bound of each debug > +counter @var{name}. Shouldn't this mention that lower bound is optional? > All debug counters have the initial upper bound of @code{UINT_MAX}; > thus @code{dbg_cnt} returns true always unless the upper bound > is set by this option. > -For example, with @option{-fdbg-cnt=dce:10,tail_call:0}, > +For example, with @option{-fdbg-cnt=dce:9:10,tail_call:0}, > @code{dbg_cnt(dce)} returns true only for first 10 invocations. This seems confusing, you added a lower bound to the 'dce' counter, but the following text remains unchanged and says it's enabled for first 10 calls? Alexander
On 05/16/2018 09:47 AM, Alexander Monakov wrote: > On Wed, 16 May 2018, Martin Liška wrote: > >> Hi. >> >> I consider it handy sometimes to trigger just a single invocation of >> an optimization driven by a debug counter. Doing that one needs to >> be able to limit both lower and upper limit of a counter. It's implemented >> in the patch. > > I'd like to offer some non-reviewer comments on the patch (below) Hi. I always appreciate these comments! > >> --- a/gcc/common.opt >> +++ b/gcc/common.opt >> @@ -1171,7 +1171,7 @@ List all available debugging counters with their limits and counts. >> >> fdbg-cnt= >> Common RejectNegative Joined Var(common_deferred_options) Defer >> --fdbg-cnt=<counter>:<limit>[,<counter>:<limit>,...] Set the debug counter limit. >> +-fdbg-cnt=<counter>[:<lower_limit>]:<upper_limit>[,<counter>:<lower_limit>:<upper_limit>,...] Set the debug counter limit. > > This line has gotten quite long and repeating the same thing in the second > brackets is not very helpful. Can we use something simpler like this? > > -fdbg-cnt=<counter>[:<lower_limit>]:<upper_limit>[,<counter>:...] Yes, it's a nice simplfication. > >> +#define DEBUG_COUNTER(a) 1, >> +static unsigned int limit_low[debug_counter_number_of_counters] = >> +{ >> +#include "dbgcnt.def" >> +}; >> +#undef DEBUG_COUNTER >> + >> + >> static unsigned int count[debug_counter_number_of_counters]; >> >> bool >> dbg_cnt_is_enabled (enum debug_counter index) >> { >> - return count[index] <= limit[index]; >> + return limit_low[index] <= count[index] && count[index] <= limit_high[index]; > > I recall Jakub recently applied a tree-wide change of A < B && B < C to read > B > A && B < C. Can you please point to a revision where it was done? > > Please consider making limit_low non-inclusive by testing for strict inequality > count[index] > limit_low[index]. This will allow to make limit_low[] array > zero-initialized (taking up space only in BSS). Sure, nice idea. I did it, now all -dbg-cnt values are zero-based. I consider it easier to work with. And as the usage is quite internal I hope we can adjust the logic to be zero-based. > >> --- a/gcc/doc/invoke.texi >> +++ b/gcc/doc/invoke.texi >> @@ -14326,13 +14326,14 @@ Print the name and the counter upper bound for all debug counters. >> >> @item -fdbg-cnt=@var{counter-value-list} >> @opindex fdbg-cnt >> -Set the internal debug counter upper bound. @var{counter-value-list} >> -is a comma-separated list of @var{name}:@var{value} pairs >> -which sets the upper bound of each debug counter @var{name} to @var{value}. >> +Set the internal debug counter lower and upper bound. @var{counter-value-list} >> +is a comma-separated list of @var{name}:@var{lower_bound}:@var{upper_bound} >> +tuples which sets the lower and the upper bound of each debug >> +counter @var{name}. > > Shouldn't this mention that lower bound is optional? Yes, fixed. > >> All debug counters have the initial upper bound of @code{UINT_MAX}; >> thus @code{dbg_cnt} returns true always unless the upper bound >> is set by this option. >> -For example, with @option{-fdbg-cnt=dce:10,tail_call:0}, >> +For example, with @option{-fdbg-cnt=dce:9:10,tail_call:0}, >> @code{dbg_cnt(dce)} returns true only for first 10 invocations. > > This seems confusing, you added a lower bound to the 'dce' counter, > but the following text remains unchanged and says it's enabled for > first 10 calls? Yes, now fixed. Thanks again, Martin > > Alexander > From c57144bbe6cb339230f887918615b7a206716b82 Mon Sep 17 00:00:00 2001 From: marxin <mliska@suse.cz> Date: Tue, 15 May 2018 15:04:30 +0200 Subject: [PATCH] Support lower and upper limit for -fdbg-cnt flag. gcc/ChangeLog: 2018-05-15 Martin Liska <mliska@suse.cz> * dbgcnt.c (limit_low): Renamed from limit. (limit_high): New variable. (dbg_cnt_is_enabled): Check for upper limit. (dbg_cnt): Adjust dumping. (dbg_cnt_set_limit_by_index): Add new argument for high value. (dbg_cnt_set_limit_by_name): Likewise. (dbg_cnt_process_single_pair): Parse new format. (dbg_cnt_process_opt): Use strtok. (dbg_cnt_list_all_counters): Remove 'value' and add 'limit_high'. * doc/invoke.texi: Document changes. gcc/testsuite/ChangeLog: 2018-05-15 Martin Liska <mliska@suse.cz> * gcc.dg/ipa/ipa-icf-39.c: New test. * gcc.dg/pr68766.c: Adjust pruned output. --- gcc/common.opt | 2 +- gcc/dbgcnt.c | 135 +++++++++++++++++++++++----------- gcc/doc/invoke.texi | 13 ++-- gcc/testsuite/gcc.dg/ipa/ipa-icf-39.c | 33 +++++++++ gcc/testsuite/gcc.dg/pr68766.c | 2 +- 5 files changed, 135 insertions(+), 50 deletions(-) create mode 100644 gcc/testsuite/gcc.dg/ipa/ipa-icf-39.c diff --git a/gcc/common.opt b/gcc/common.opt index d6ef85928f3..13ab5c65d43 100644 --- a/gcc/common.opt +++ b/gcc/common.opt @@ -1171,7 +1171,7 @@ List all available debugging counters with their limits and counts. fdbg-cnt= Common RejectNegative Joined Var(common_deferred_options) Defer --fdbg-cnt=<counter>:<limit>[,<counter>:<limit>,...] Set the debug counter limit. +-fdbg-cnt=<counter>[:<lower_limit>]:<upper_limit>[,<counter>:...] Set the debug counter limit. fdebug-prefix-map= Common Joined RejectNegative Var(common_deferred_options) Defer diff --git a/gcc/dbgcnt.c b/gcc/dbgcnt.c index 96b3df28f5e..d6839c85c82 100644 --- a/gcc/dbgcnt.c +++ b/gcc/dbgcnt.c @@ -41,53 +41,90 @@ static struct string2counter_map map[debug_counter_number_of_counters] = #undef DEBUG_COUNTER #define DEBUG_COUNTER(a) UINT_MAX, -static unsigned int limit[debug_counter_number_of_counters] = +static unsigned int limit_high[debug_counter_number_of_counters] = { #include "dbgcnt.def" }; #undef DEBUG_COUNTER +#define DEBUG_COUNTER(a) 0, +static unsigned int limit_low[debug_counter_number_of_counters] = +{ +#include "dbgcnt.def" +}; +#undef DEBUG_COUNTER + + static unsigned int count[debug_counter_number_of_counters]; bool dbg_cnt_is_enabled (enum debug_counter index) { - return count[index] <= limit[index]; + unsigned v = count[index]; + return limit_low[index] <= v && v <= limit_high[index]; } bool dbg_cnt (enum debug_counter index) { + if (dump_file) + { + /* Do not print the info for default lower limit. */ + if (count[index] == limit_low[index] && limit_low[index] > 0) + fprintf (dump_file, "***dbgcnt: lower limit %d reached for %s.***\n", + limit_low[index], map[index].name); + else if (count[index] == limit_high[index]) + fprintf (dump_file, "***dbgcnt: upper limit %d reached for %s.***\n", + limit_high[index], map[index].name); + } + + bool r = dbg_cnt_is_enabled (index); count[index]++; - if (dump_file && count[index] == limit[index]) - fprintf (dump_file, "***dbgcnt: limit reached for %s.***\n", - map[index].name); - - return dbg_cnt_is_enabled (index); + return r; } - static void -dbg_cnt_set_limit_by_index (enum debug_counter index, int value) +dbg_cnt_set_limit_by_index (enum debug_counter index, int low, int high) { - limit[index] = value; + limit_low[index] = low; + limit_high[index] = high; - fprintf (stderr, "dbg_cnt '%s' set to %d\n", map[index].name, value); + fprintf (stderr, "dbg_cnt '%s' set to %d-%d\n", map[index].name, low, high); } static bool -dbg_cnt_set_limit_by_name (const char *name, int len, int value) +dbg_cnt_set_limit_by_name (const char *name, int low, int high) { + if (high < low) + { + error ("-fdbg-cnt=%s:%d:%d has smaller upper limit than the lower", + name, low, high); + return false; + } + + if (low < 0) + { + error ("Lower limit %d of -fdbg-cnt=%s must be a non-negative number", low, + name); + return false; + } + + if (high < 0) + { + error ("Upper limit %d of -fdbg-cnt=%s must be a non-negative number", high, + name); + return false; + } + int i; for (i = debug_counter_number_of_counters - 1; i >= 0; i--) - if (strncmp (map[i].name, name, len) == 0 - && map[i].name[len] == '\0') + if (strcmp (map[i].name, name) == 0) break; if (i < 0) return false; - dbg_cnt_set_limit_by_index ((enum debug_counter) i, value); + dbg_cnt_set_limit_by_index ((enum debug_counter) i, low, high); return true; } @@ -96,42 +133,53 @@ dbg_cnt_set_limit_by_name (const char *name, int len, int value) Returns NULL if there's no valid pair is found. Otherwise returns a pointer to the end of the pair. */ -static const char * +static bool dbg_cnt_process_single_pair (const char *arg) { - const char *colon = strchr (arg, ':'); - char *endptr = NULL; - int value; - - if (colon == NULL) - return NULL; - - value = strtol (colon + 1, &endptr, 10); - - if (endptr != NULL && endptr != colon + 1 - && dbg_cnt_set_limit_by_name (arg, colon - arg, value)) - return endptr; - - return NULL; + char *str = xstrdup (arg); + char *name = strtok (str, ":"); + char *value1 = strtok (NULL, ":"); + char *value2 = strtok (NULL, ":"); + + int high, low; + + if (value1 == NULL) + return NULL; + + if (value2 == NULL) + { + low = 0; + high = strtol (value1, NULL, 10); + } + else + { + low = strtol (value1, NULL, 10); + high = strtol (value2, NULL, 10); + } + + return dbg_cnt_set_limit_by_name (name, low, high); } void dbg_cnt_process_opt (const char *arg) { - const char *start = arg; - const char *next; + char *str = xstrdup (arg); + const char *next = strtok (str, ","); + unsigned int start = 0; + do { - next = dbg_cnt_process_single_pair (arg); - if (next == NULL) + if (!dbg_cnt_process_single_pair (arg)) break; - } while (*next == ',' && (arg = next + 1)); + start += strlen (arg) + 1; + next = strtok (NULL, ","); + } while (next != NULL); - if (next == NULL || *next != 0) + if (next != NULL) { - char *buffer = XALLOCAVEC (char, arg - start + 2); - sprintf (buffer, "%*c", (int)(1 + (arg - start)), '^'); + char *buffer = XALLOCAVEC (char, start + 2); + sprintf (buffer, "%*c", start + 1, '^'); error ("cannot find a valid counter:value pair:"); - error ("-fdbg-cnt=%s", start); + error ("-fdbg-cnt=%s", next); error (" %s", buffer); } } @@ -142,10 +190,11 @@ void dbg_cnt_list_all_counters (void) { int i; - printf (" %-30s %-5s %-5s\n", "counter name", "limit", "value"); - printf ("----------------------------------------------\n"); + printf (" %-32s %-11s %-12s\n", "counter name", "low limit", + "high limit"); + printf ("-----------------------------------------------------------------\n"); for (i = 0; i < debug_counter_number_of_counters; i++) - printf (" %-30s %5d %5u\n", - map[i].name, limit[map[i].counter], count[map[i].counter]); + printf (" %-30s %11u %12u\n", + map[i].name, limit_low[map[i].counter], limit_high[map[i].counter]); printf ("\n"); } diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi index ca3772bbebf..be55d28a38b 100644 --- a/gcc/doc/invoke.texi +++ b/gcc/doc/invoke.texi @@ -14326,14 +14326,17 @@ Print the name and the counter upper bound for all debug counters. @item -fdbg-cnt=@var{counter-value-list} @opindex fdbg-cnt -Set the internal debug counter upper bound. @var{counter-value-list} -is a comma-separated list of @var{name}:@var{value} pairs -which sets the upper bound of each debug counter @var{name} to @var{value}. +Set the internal debug counter lower and upper bound. @var{counter-value-list} +is a comma-separated list of @var{name}:@var{lower_bound}:@var{upper_bound} +tuples which sets the lower and the upper bound of each debug +counter @var{name}. The @var{lower_bound} is optional and is zero +initialized if not set. All debug counters have the initial upper bound of @code{UINT_MAX}; thus @code{dbg_cnt} returns true always unless the upper bound is set by this option. -For example, with @option{-fdbg-cnt=dce:10,tail_call:0}, -@code{dbg_cnt(dce)} returns true only for first 10 invocations. +For example, with @option{-fdbg-cnt=dce:9:10,tail_call:10}, +@code{dbg_cnt(dce)} returns true only for 10th and 11th invocation. +For @code{dbg_cnt(tail_call)} true is returned for first 11 invocations. @item -print-file-name=@var{library} @opindex print-file-name diff --git a/gcc/testsuite/gcc.dg/ipa/ipa-icf-39.c b/gcc/testsuite/gcc.dg/ipa/ipa-icf-39.c new file mode 100644 index 00000000000..8759108a2d5 --- /dev/null +++ b/gcc/testsuite/gcc.dg/ipa/ipa-icf-39.c @@ -0,0 +1,33 @@ +/* { dg-do compile } */ +/* { dg-options "-O2 -fdump-ipa-icf -fmerge-all-constants -fdbg-cnt=merged_ipa_icf:1:2" } */ +/* { dg-prune-output "dbg_cnt 'merged_ipa_icf' set to 1-2" } */ + +static int a; +static int b; +static const int c = 2; +static const int d = 2; +static char * e = "test"; +static char * f = "test"; +static int g[3]={1,2,3}; +static int h[3]={1,2,3}; +static const int *i=&c; +static const int *j=&c; +static const int *k=&d; +int t(int tt) +{ + switch (tt) + { + case 1: return a; + case 2: return b; + case 3: return c; + case 4: return d; + case 5: return e[1]; + case 6: return f[1]; + case 7: return g[1]; + case 8: return h[1]; + case 9: return i[0]; + case 10: return j[0]; + case 11: return k[0]; + } +} +/* { dg-final { scan-ipa-dump-times "Unified;" 2 "icf" } } */ diff --git a/gcc/testsuite/gcc.dg/pr68766.c b/gcc/testsuite/gcc.dg/pr68766.c index a0d549b946e..83f0e14b7d2 100644 --- a/gcc/testsuite/gcc.dg/pr68766.c +++ b/gcc/testsuite/gcc.dg/pr68766.c @@ -1,7 +1,7 @@ /* { dg-do compile } */ /* { dg-options "-O2 -ftree-vectorize -fdbg-cnt=vect_loop:1" } */ /* { dg-additional-options "-mavx2" { target { i?86-*-* x86_64-*-* } } } */ -/* { dg-prune-output "dbg_cnt 'vect_loop' set to 1" } */ +/* { dg-prune-output "dbg_cnt 'vect_loop' set to 1-1" } */ int a, b, g, h; int c[58];
On Wed, 16 May 2018, Martin Liška wrote: > > I recall Jakub recently applied a tree-wide change of A < B && B < C to read > > B > A && B < C. > > Can you please point to a revision where it was done? It is SVN r255831, mailing list thread here ("Replace Yoda conditions"): https://gcc.gnu.org/ml/gcc-patches/2017-12/msg01257.html In your revised patch: > --- a/gcc/dbgcnt.c > +++ b/gcc/dbgcnt.c > @@ -41,53 +41,90 @@ static struct string2counter_map map[debug_counter_number_of_counters] = > #undef DEBUG_COUNTER > > #define DEBUG_COUNTER(a) UINT_MAX, > -static unsigned int limit[debug_counter_number_of_counters] = > +static unsigned int limit_high[debug_counter_number_of_counters] = > { > #include "dbgcnt.def" > }; > #undef DEBUG_COUNTER > > +#define DEBUG_COUNTER(a) 0, > +static unsigned int limit_low[debug_counter_number_of_counters] = > +{ > +#include "dbgcnt.def" > +}; > +#undef DEBUG_COUNTER No need to spell out the all-zeros initializer of a static object: static unsigned int limit_low[debug_counter_number_of_counters]; > --- a/gcc/doc/invoke.texi > +++ b/gcc/doc/invoke.texi > @@ -14326,14 +14326,17 @@ Print the name and the counter upper bound for all debug counters. > > @item -fdbg-cnt=@var{counter-value-list} > @opindex fdbg-cnt > -Set the internal debug counter upper bound. @var{counter-value-list} > -is a comma-separated list of @var{name}:@var{value} pairs > -which sets the upper bound of each debug counter @var{name} to @var{value}. > +Set the internal debug counter lower and upper bound. @var{counter-value-list} > +is a comma-separated list of @var{name}:@var{lower_bound}:@var{upper_bound} > +tuples which sets the lower and the upper bound of each debug > +counter @var{name}. The @var{lower_bound} is optional and is zero > +initialized if not set. > All debug counters have the initial upper bound of @code{UINT_MAX}; > thus @code{dbg_cnt} returns true always unless the upper bound > is set by this option. > -For example, with @option{-fdbg-cnt=dce:10,tail_call:0}, > -@code{dbg_cnt(dce)} returns true only for first 10 invocations. > +For example, with @option{-fdbg-cnt=dce:9:10,tail_call:10}, > +@code{dbg_cnt(dce)} returns true only for 10th and 11th invocation. > +For @code{dbg_cnt(tail_call)} true is returned for first 11 invocations. Hm, is the off-by-one in the new explanatory text really intended? I think the previous text was accurate, and the new text should say "9th and 10th" and then "first 10 invocations", unless I'm missing something? Alexander
On 05/16/2018 02:56 PM, Alexander Monakov wrote: > On Wed, 16 May 2018, Martin Liška wrote: >>> I recall Jakub recently applied a tree-wide change of A < B && B < C to read >>> B > A && B < C. >> >> Can you please point to a revision where it was done? > > It is SVN r255831, mailing list thread here ("Replace Yoda conditions"): > https://gcc.gnu.org/ml/gcc-patches/2017-12/msg01257.html Thanks, fix according to that. > > In your revised patch: > >> --- a/gcc/dbgcnt.c >> +++ b/gcc/dbgcnt.c >> @@ -41,53 +41,90 @@ static struct string2counter_map map[debug_counter_number_of_counters] = >> #undef DEBUG_COUNTER >> >> #define DEBUG_COUNTER(a) UINT_MAX, >> -static unsigned int limit[debug_counter_number_of_counters] = >> +static unsigned int limit_high[debug_counter_number_of_counters] = >> { >> #include "dbgcnt.def" >> }; >> #undef DEBUG_COUNTER >> >> +#define DEBUG_COUNTER(a) 0, >> +static unsigned int limit_low[debug_counter_number_of_counters] = >> +{ >> +#include "dbgcnt.def" >> +}; >> +#undef DEBUG_COUNTER > > No need to spell out the all-zeros initializer of a static object: > > static unsigned int limit_low[debug_counter_number_of_counters]; Got it. > >> --- a/gcc/doc/invoke.texi >> +++ b/gcc/doc/invoke.texi >> @@ -14326,14 +14326,17 @@ Print the name and the counter upper bound for all debug counters. >> >> @item -fdbg-cnt=@var{counter-value-list} >> @opindex fdbg-cnt >> -Set the internal debug counter upper bound. @var{counter-value-list} >> -is a comma-separated list of @var{name}:@var{value} pairs >> -which sets the upper bound of each debug counter @var{name} to @var{value}. >> +Set the internal debug counter lower and upper bound. @var{counter-value-list} >> +is a comma-separated list of @var{name}:@var{lower_bound}:@var{upper_bound} >> +tuples which sets the lower and the upper bound of each debug >> +counter @var{name}. The @var{lower_bound} is optional and is zero >> +initialized if not set. >> All debug counters have the initial upper bound of @code{UINT_MAX}; >> thus @code{dbg_cnt} returns true always unless the upper bound >> is set by this option. >> -For example, with @option{-fdbg-cnt=dce:10,tail_call:0}, >> -@code{dbg_cnt(dce)} returns true only for first 10 invocations. >> +For example, with @option{-fdbg-cnt=dce:9:10,tail_call:10}, >> +@code{dbg_cnt(dce)} returns true only for 10th and 11th invocation. >> +For @code{dbg_cnt(tail_call)} true is returned for first 11 invocations. > > Hm, is the off-by-one in the new explanatory text really intended? I think > the previous text was accurate, and the new text should say "9th and 10th" > and then "first 10 invocations", unless I'm missing something? I've reconsidered that once more time and having zero-based values: * -fdbg-cnt=event:N - trigger event N-times * -fdbg-cnt=event:N:(N+M) - skip even N-times and then enable it M-1 times Does that make sense? Martin > > Alexander > From 49f185588d1f5c796f83bb9b546c6199c7e80d2f Mon Sep 17 00:00:00 2001 From: marxin <mliska@suse.cz> Date: Tue, 15 May 2018 15:04:30 +0200 Subject: [PATCH] Support lower and upper limit for -fdbg-cnt flag. gcc/ChangeLog: 2018-05-15 Martin Liska <mliska@suse.cz> * dbgcnt.c (limit_low): Renamed from limit. (limit_high): New variable. (dbg_cnt_is_enabled): Check for upper limit. (dbg_cnt): Adjust dumping. (dbg_cnt_set_limit_by_index): Add new argument for high value. (dbg_cnt_set_limit_by_name): Likewise. (dbg_cnt_process_single_pair): Parse new format. (dbg_cnt_process_opt): Use strtok. (dbg_cnt_list_all_counters): Remove 'value' and add 'limit_high'. * doc/invoke.texi: Document changes. gcc/testsuite/ChangeLog: 2018-05-15 Martin Liska <mliska@suse.cz> * gcc.dg/ipa/ipa-icf-39.c: New test. * gcc.dg/pr68766.c: Adjust pruned output. --- gcc/common.opt | 2 +- gcc/dbgcnt.c | 129 ++++++++++++++++++++++------------ gcc/doc/invoke.texi | 13 ++-- gcc/testsuite/gcc.dg/ipa/ipa-icf-39.c | 33 +++++++++ gcc/testsuite/gcc.dg/pr68766.c | 2 +- 5 files changed, 129 insertions(+), 50 deletions(-) create mode 100644 gcc/testsuite/gcc.dg/ipa/ipa-icf-39.c diff --git a/gcc/common.opt b/gcc/common.opt index d6ef85928f3..13ab5c65d43 100644 --- a/gcc/common.opt +++ b/gcc/common.opt @@ -1171,7 +1171,7 @@ List all available debugging counters with their limits and counts. fdbg-cnt= Common RejectNegative Joined Var(common_deferred_options) Defer --fdbg-cnt=<counter>:<limit>[,<counter>:<limit>,...] Set the debug counter limit. +-fdbg-cnt=<counter>[:<lower_limit>]:<upper_limit>[,<counter>:...] Set the debug counter limit. fdebug-prefix-map= Common Joined RejectNegative Var(common_deferred_options) Defer diff --git a/gcc/dbgcnt.c b/gcc/dbgcnt.c index 96b3df28f5e..e7c6da2fac0 100644 --- a/gcc/dbgcnt.c +++ b/gcc/dbgcnt.c @@ -41,53 +41,84 @@ static struct string2counter_map map[debug_counter_number_of_counters] = #undef DEBUG_COUNTER #define DEBUG_COUNTER(a) UINT_MAX, -static unsigned int limit[debug_counter_number_of_counters] = +static unsigned int limit_high[debug_counter_number_of_counters] = { #include "dbgcnt.def" }; #undef DEBUG_COUNTER +static unsigned int limit_low[debug_counter_number_of_counters]; + static unsigned int count[debug_counter_number_of_counters]; bool dbg_cnt_is_enabled (enum debug_counter index) { - return count[index] <= limit[index]; + unsigned v = count[index]; + return v >= limit_low[index] && v < limit_high[index]; } bool dbg_cnt (enum debug_counter index) { + if (dump_file) + { + /* Do not print the info for default lower limit. */ + if (count[index] == limit_low[index] && limit_low[index] > 0) + fprintf (dump_file, "***dbgcnt: lower limit %d reached for %s.***\n", + limit_low[index], map[index].name); + else if (count[index] == limit_high[index]) + fprintf (dump_file, "***dbgcnt: upper limit %d reached for %s.***\n", + limit_high[index], map[index].name); + } + + bool r = dbg_cnt_is_enabled (index); count[index]++; - if (dump_file && count[index] == limit[index]) - fprintf (dump_file, "***dbgcnt: limit reached for %s.***\n", - map[index].name); - - return dbg_cnt_is_enabled (index); + return r; } - static void -dbg_cnt_set_limit_by_index (enum debug_counter index, int value) +dbg_cnt_set_limit_by_index (enum debug_counter index, int low, int high) { - limit[index] = value; + limit_low[index] = low; + limit_high[index] = high; - fprintf (stderr, "dbg_cnt '%s' set to %d\n", map[index].name, value); + fprintf (stderr, "dbg_cnt '%s' set to %d-%d\n", map[index].name, low, high); } static bool -dbg_cnt_set_limit_by_name (const char *name, int len, int value) +dbg_cnt_set_limit_by_name (const char *name, int low, int high) { + if (high < low) + { + error ("-fdbg-cnt=%s:%d:%d has smaller upper limit than the lower", + name, low, high); + return false; + } + + if (low < 0) + { + error ("Lower limit %d of -fdbg-cnt=%s must be a non-negative number", low, + name); + return false; + } + + if (high < 0) + { + error ("Upper limit %d of -fdbg-cnt=%s must be a non-negative number", high, + name); + return false; + } + int i; for (i = debug_counter_number_of_counters - 1; i >= 0; i--) - if (strncmp (map[i].name, name, len) == 0 - && map[i].name[len] == '\0') + if (strcmp (map[i].name, name) == 0) break; if (i < 0) return false; - dbg_cnt_set_limit_by_index ((enum debug_counter) i, value); + dbg_cnt_set_limit_by_index ((enum debug_counter) i, low, high); return true; } @@ -96,42 +127,53 @@ dbg_cnt_set_limit_by_name (const char *name, int len, int value) Returns NULL if there's no valid pair is found. Otherwise returns a pointer to the end of the pair. */ -static const char * +static bool dbg_cnt_process_single_pair (const char *arg) { - const char *colon = strchr (arg, ':'); - char *endptr = NULL; - int value; - - if (colon == NULL) - return NULL; - - value = strtol (colon + 1, &endptr, 10); - - if (endptr != NULL && endptr != colon + 1 - && dbg_cnt_set_limit_by_name (arg, colon - arg, value)) - return endptr; - - return NULL; + char *str = xstrdup (arg); + char *name = strtok (str, ":"); + char *value1 = strtok (NULL, ":"); + char *value2 = strtok (NULL, ":"); + + int high, low; + + if (value1 == NULL) + return NULL; + + if (value2 == NULL) + { + low = 0; + high = strtol (value1, NULL, 10); + } + else + { + low = strtol (value1, NULL, 10); + high = strtol (value2, NULL, 10); + } + + return dbg_cnt_set_limit_by_name (name, low, high); } void dbg_cnt_process_opt (const char *arg) { - const char *start = arg; - const char *next; + char *str = xstrdup (arg); + const char *next = strtok (str, ","); + unsigned int start = 0; + do { - next = dbg_cnt_process_single_pair (arg); - if (next == NULL) + if (!dbg_cnt_process_single_pair (arg)) break; - } while (*next == ',' && (arg = next + 1)); + start += strlen (arg) + 1; + next = strtok (NULL, ","); + } while (next != NULL); - if (next == NULL || *next != 0) + if (next != NULL) { - char *buffer = XALLOCAVEC (char, arg - start + 2); - sprintf (buffer, "%*c", (int)(1 + (arg - start)), '^'); + char *buffer = XALLOCAVEC (char, start + 2); + sprintf (buffer, "%*c", start + 1, '^'); error ("cannot find a valid counter:value pair:"); - error ("-fdbg-cnt=%s", start); + error ("-fdbg-cnt=%s", next); error (" %s", buffer); } } @@ -142,10 +184,11 @@ void dbg_cnt_list_all_counters (void) { int i; - printf (" %-30s %-5s %-5s\n", "counter name", "limit", "value"); - printf ("----------------------------------------------\n"); + printf (" %-32s %-11s %-12s\n", "counter name", "low limit", + "high limit"); + printf ("-----------------------------------------------------------------\n"); for (i = 0; i < debug_counter_number_of_counters; i++) - printf (" %-30s %5d %5u\n", - map[i].name, limit[map[i].counter], count[map[i].counter]); + printf (" %-30s %11u %12u\n", + map[i].name, limit_low[map[i].counter], limit_high[map[i].counter]); printf ("\n"); } diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi index ca3772bbebf..86b9c86252f 100644 --- a/gcc/doc/invoke.texi +++ b/gcc/doc/invoke.texi @@ -14326,14 +14326,17 @@ Print the name and the counter upper bound for all debug counters. @item -fdbg-cnt=@var{counter-value-list} @opindex fdbg-cnt -Set the internal debug counter upper bound. @var{counter-value-list} -is a comma-separated list of @var{name}:@var{value} pairs -which sets the upper bound of each debug counter @var{name} to @var{value}. +Set the internal debug counter lower and upper bound. @var{counter-value-list} +is a comma-separated list of @var{name}:@var{lower_bound}:@var{upper_bound} +tuples which sets the lower and the upper bound of each debug +counter @var{name}. The @var{lower_bound} is optional and is zero +initialized if not set. All debug counters have the initial upper bound of @code{UINT_MAX}; thus @code{dbg_cnt} returns true always unless the upper bound is set by this option. -For example, with @option{-fdbg-cnt=dce:10,tail_call:0}, -@code{dbg_cnt(dce)} returns true only for first 10 invocations. +For example, with @option{-fdbg-cnt=dce:2:4,tail_call:10}, +@code{dbg_cnt(dce)} returns true only for third and fourth invocation. +For @code{dbg_cnt(tail_call)} true is returned for first 10 invocations. @item -print-file-name=@var{library} @opindex print-file-name diff --git a/gcc/testsuite/gcc.dg/ipa/ipa-icf-39.c b/gcc/testsuite/gcc.dg/ipa/ipa-icf-39.c new file mode 100644 index 00000000000..aa7c28706d3 --- /dev/null +++ b/gcc/testsuite/gcc.dg/ipa/ipa-icf-39.c @@ -0,0 +1,33 @@ +/* { dg-do compile } */ +/* { dg-options "-O2 -fdump-ipa-icf -fmerge-all-constants -fdbg-cnt=merged_ipa_icf:1:3" } */ +/* { dg-prune-output "dbg_cnt 'merged_ipa_icf' set to 1-3" } */ + +static int a; +static int b; +static const int c = 2; +static const int d = 2; +static char * e = "test"; +static char * f = "test"; +static int g[3]={1,2,3}; +static int h[3]={1,2,3}; +static const int *i=&c; +static const int *j=&c; +static const int *k=&d; +int t(int tt) +{ + switch (tt) + { + case 1: return a; + case 2: return b; + case 3: return c; + case 4: return d; + case 5: return e[1]; + case 6: return f[1]; + case 7: return g[1]; + case 8: return h[1]; + case 9: return i[0]; + case 10: return j[0]; + case 11: return k[0]; + } +} +/* { dg-final { scan-ipa-dump-times "Unified;" 2 "icf" } } */ diff --git a/gcc/testsuite/gcc.dg/pr68766.c b/gcc/testsuite/gcc.dg/pr68766.c index a0d549b946e..83f0e14b7d2 100644 --- a/gcc/testsuite/gcc.dg/pr68766.c +++ b/gcc/testsuite/gcc.dg/pr68766.c @@ -1,7 +1,7 @@ /* { dg-do compile } */ /* { dg-options "-O2 -ftree-vectorize -fdbg-cnt=vect_loop:1" } */ /* { dg-additional-options "-mavx2" { target { i?86-*-* x86_64-*-* } } } */ -/* { dg-prune-output "dbg_cnt 'vect_loop' set to 1" } */ +/* { dg-prune-output "dbg_cnt 'vect_loop' set to 1-1" } */ int a, b, g, h; int c[58];
On Wed, 16 May 2018, Martin Liška wrote: > > Hm, is the off-by-one in the new explanatory text really intended? I think > > the previous text was accurate, and the new text should say "9th and 10th" > > and then "first 10 invocations", unless I'm missing something? > > I've reconsidered that once more time and having zero-based values: > * -fdbg-cnt=event:N - trigger event N-times > * -fdbg-cnt=event:N:(N+M) - skip even N-times and then enable it M-1 times > > Does that make sense? Yes, I like this, but I think the implementation does not match. New docs say: > -For example, with @option{-fdbg-cnt=dce:10,tail_call:0}, > -@code{dbg_cnt(dce)} returns true only for first 10 invocations. > +For example, with @option{-fdbg-cnt=dce:2:4,tail_call:10}, > +@code{dbg_cnt(dce)} returns true only for third and fourth invocation. > +For @code{dbg_cnt(tail_call)} true is returned for first 10 invocations. which is good, but the implementation reads: > bool > dbg_cnt_is_enabled (enum debug_counter index) > { > - return count[index] <= limit[index]; > + unsigned v = count[index]; > + return v >= limit_low[index] && v < limit_high[index]; > } which I believe is misaligned with the docs' intention. It should be the other way around: return v > limit_low[index] && v <= limit_high[index]; Alexander
On 05/16/2018 03:39 PM, Alexander Monakov wrote: > On Wed, 16 May 2018, Martin Liška wrote: >>> Hm, is the off-by-one in the new explanatory text really intended? I think >>> the previous text was accurate, and the new text should say "9th and 10th" >>> and then "first 10 invocations", unless I'm missing something? >> >> I've reconsidered that once more time and having zero-based values: >> * -fdbg-cnt=event:N - trigger event N-times >> * -fdbg-cnt=event:N:(N+M) - skip even N-times and then enable it M-1 times >> >> Does that make sense? > > Yes, I like this, but I think the implementation does not match. New docs say: > >> -For example, with @option{-fdbg-cnt=dce:10,tail_call:0}, >> -@code{dbg_cnt(dce)} returns true only for first 10 invocations. >> +For example, with @option{-fdbg-cnt=dce:2:4,tail_call:10}, >> +@code{dbg_cnt(dce)} returns true only for third and fourth invocation. >> +For @code{dbg_cnt(tail_call)} true is returned for first 10 invocations. > > which is good, but the implementation reads: > >> bool >> dbg_cnt_is_enabled (enum debug_counter index) >> { >> - return count[index] <= limit[index]; >> + unsigned v = count[index]; >> + return v >= limit_low[index] && v < limit_high[index]; >> } > > which I believe is misaligned with the docs' intention. It should be the > other way around: > > return v > limit_low[index] && v <= limit_high[index]; Note that I changed count[index]++ to happen after dbg_cnt_is_enabled. I'm reverting that and now it works fine with your condition. Martin > > Alexander > From 8d21c709fdc956f951454e910557cd86f73d6a98 Mon Sep 17 00:00:00 2001 From: marxin <mliska@suse.cz> Date: Tue, 15 May 2018 15:04:30 +0200 Subject: [PATCH] Support lower and upper limit for -fdbg-cnt flag. gcc/ChangeLog: 2018-05-15 Martin Liska <mliska@suse.cz> * dbgcnt.c (limit_low): Renamed from limit. (limit_high): New variable. (dbg_cnt_is_enabled): Check for upper limit. (dbg_cnt): Adjust dumping. (dbg_cnt_set_limit_by_index): Add new argument for high value. (dbg_cnt_set_limit_by_name): Likewise. (dbg_cnt_process_single_pair): Parse new format. (dbg_cnt_process_opt): Use strtok. (dbg_cnt_list_all_counters): Remove 'value' and add 'limit_high'. * doc/invoke.texi: Document changes. gcc/testsuite/ChangeLog: 2018-05-15 Martin Liska <mliska@suse.cz> * gcc.dg/ipa/ipa-icf-39.c: New test. * gcc.dg/pr68766.c: Adjust pruned output. --- gcc/common.opt | 2 +- gcc/dbgcnt.c | 125 +++++++++++++++++++++++----------- gcc/doc/invoke.texi | 13 ++-- gcc/testsuite/gcc.dg/ipa/ipa-icf-39.c | 33 +++++++++ gcc/testsuite/gcc.dg/pr68766.c | 2 +- 5 files changed, 127 insertions(+), 48 deletions(-) create mode 100644 gcc/testsuite/gcc.dg/ipa/ipa-icf-39.c diff --git a/gcc/common.opt b/gcc/common.opt index d6ef85928f3..13ab5c65d43 100644 --- a/gcc/common.opt +++ b/gcc/common.opt @@ -1171,7 +1171,7 @@ List all available debugging counters with their limits and counts. fdbg-cnt= Common RejectNegative Joined Var(common_deferred_options) Defer --fdbg-cnt=<counter>:<limit>[,<counter>:<limit>,...] Set the debug counter limit. +-fdbg-cnt=<counter>[:<lower_limit>]:<upper_limit>[,<counter>:...] Set the debug counter limit. fdebug-prefix-map= Common Joined RejectNegative Var(common_deferred_options) Defer diff --git a/gcc/dbgcnt.c b/gcc/dbgcnt.c index 96b3df28f5e..ddb0e8e76d9 100644 --- a/gcc/dbgcnt.c +++ b/gcc/dbgcnt.c @@ -41,53 +41,84 @@ static struct string2counter_map map[debug_counter_number_of_counters] = #undef DEBUG_COUNTER #define DEBUG_COUNTER(a) UINT_MAX, -static unsigned int limit[debug_counter_number_of_counters] = +static unsigned int limit_high[debug_counter_number_of_counters] = { #include "dbgcnt.def" }; #undef DEBUG_COUNTER +static unsigned int limit_low[debug_counter_number_of_counters]; + static unsigned int count[debug_counter_number_of_counters]; bool dbg_cnt_is_enabled (enum debug_counter index) { - return count[index] <= limit[index]; + unsigned v = count[index]; + return v > limit_low[index] && v <= limit_high[index]; } bool dbg_cnt (enum debug_counter index) { count[index]++; - if (dump_file && count[index] == limit[index]) - fprintf (dump_file, "***dbgcnt: limit reached for %s.***\n", - map[index].name); + + if (dump_file) + { + /* Do not print the info for default lower limit. */ + if (count[index] == limit_low[index] && limit_low[index] > 0) + fprintf (dump_file, "***dbgcnt: lower limit %d reached for %s.***\n", + limit_low[index], map[index].name); + else if (count[index] == limit_high[index]) + fprintf (dump_file, "***dbgcnt: upper limit %d reached for %s.***\n", + limit_high[index], map[index].name); + } return dbg_cnt_is_enabled (index); } - static void -dbg_cnt_set_limit_by_index (enum debug_counter index, int value) +dbg_cnt_set_limit_by_index (enum debug_counter index, int low, int high) { - limit[index] = value; + limit_low[index] = low; + limit_high[index] = high; - fprintf (stderr, "dbg_cnt '%s' set to %d\n", map[index].name, value); + fprintf (stderr, "dbg_cnt '%s' set to %d-%d\n", map[index].name, low, high); } static bool -dbg_cnt_set_limit_by_name (const char *name, int len, int value) +dbg_cnt_set_limit_by_name (const char *name, int low, int high) { + if (high < low) + { + error ("-fdbg-cnt=%s:%d:%d has smaller upper limit than the lower", + name, low, high); + return false; + } + + if (low < 0) + { + error ("Lower limit %d of -fdbg-cnt=%s must be a non-negative number", low, + name); + return false; + } + + if (high < 0) + { + error ("Upper limit %d of -fdbg-cnt=%s must be a non-negative number", high, + name); + return false; + } + int i; for (i = debug_counter_number_of_counters - 1; i >= 0; i--) - if (strncmp (map[i].name, name, len) == 0 - && map[i].name[len] == '\0') + if (strcmp (map[i].name, name) == 0) break; if (i < 0) return false; - dbg_cnt_set_limit_by_index ((enum debug_counter) i, value); + dbg_cnt_set_limit_by_index ((enum debug_counter) i, low, high); return true; } @@ -96,42 +127,53 @@ dbg_cnt_set_limit_by_name (const char *name, int len, int value) Returns NULL if there's no valid pair is found. Otherwise returns a pointer to the end of the pair. */ -static const char * +static bool dbg_cnt_process_single_pair (const char *arg) { - const char *colon = strchr (arg, ':'); - char *endptr = NULL; - int value; - - if (colon == NULL) - return NULL; - - value = strtol (colon + 1, &endptr, 10); - - if (endptr != NULL && endptr != colon + 1 - && dbg_cnt_set_limit_by_name (arg, colon - arg, value)) - return endptr; - - return NULL; + char *str = xstrdup (arg); + char *name = strtok (str, ":"); + char *value1 = strtok (NULL, ":"); + char *value2 = strtok (NULL, ":"); + + int high, low; + + if (value1 == NULL) + return NULL; + + if (value2 == NULL) + { + low = 0; + high = strtol (value1, NULL, 10); + } + else + { + low = strtol (value1, NULL, 10); + high = strtol (value2, NULL, 10); + } + + return dbg_cnt_set_limit_by_name (name, low, high); } void dbg_cnt_process_opt (const char *arg) { - const char *start = arg; - const char *next; + char *str = xstrdup (arg); + const char *next = strtok (str, ","); + unsigned int start = 0; + do { - next = dbg_cnt_process_single_pair (arg); - if (next == NULL) + if (!dbg_cnt_process_single_pair (arg)) break; - } while (*next == ',' && (arg = next + 1)); + start += strlen (arg) + 1; + next = strtok (NULL, ","); + } while (next != NULL); - if (next == NULL || *next != 0) + if (next != NULL) { - char *buffer = XALLOCAVEC (char, arg - start + 2); - sprintf (buffer, "%*c", (int)(1 + (arg - start)), '^'); + char *buffer = XALLOCAVEC (char, start + 2); + sprintf (buffer, "%*c", start + 1, '^'); error ("cannot find a valid counter:value pair:"); - error ("-fdbg-cnt=%s", start); + error ("-fdbg-cnt=%s", next); error (" %s", buffer); } } @@ -142,10 +184,11 @@ void dbg_cnt_list_all_counters (void) { int i; - printf (" %-30s %-5s %-5s\n", "counter name", "limit", "value"); - printf ("----------------------------------------------\n"); + printf (" %-32s %-11s %-12s\n", "counter name", "low limit", + "high limit"); + printf ("-----------------------------------------------------------------\n"); for (i = 0; i < debug_counter_number_of_counters; i++) - printf (" %-30s %5d %5u\n", - map[i].name, limit[map[i].counter], count[map[i].counter]); + printf (" %-30s %11u %12u\n", + map[i].name, limit_low[map[i].counter], limit_high[map[i].counter]); printf ("\n"); } diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi index ca3772bbebf..86b9c86252f 100644 --- a/gcc/doc/invoke.texi +++ b/gcc/doc/invoke.texi @@ -14326,14 +14326,17 @@ Print the name and the counter upper bound for all debug counters. @item -fdbg-cnt=@var{counter-value-list} @opindex fdbg-cnt -Set the internal debug counter upper bound. @var{counter-value-list} -is a comma-separated list of @var{name}:@var{value} pairs -which sets the upper bound of each debug counter @var{name} to @var{value}. +Set the internal debug counter lower and upper bound. @var{counter-value-list} +is a comma-separated list of @var{name}:@var{lower_bound}:@var{upper_bound} +tuples which sets the lower and the upper bound of each debug +counter @var{name}. The @var{lower_bound} is optional and is zero +initialized if not set. All debug counters have the initial upper bound of @code{UINT_MAX}; thus @code{dbg_cnt} returns true always unless the upper bound is set by this option. -For example, with @option{-fdbg-cnt=dce:10,tail_call:0}, -@code{dbg_cnt(dce)} returns true only for first 10 invocations. +For example, with @option{-fdbg-cnt=dce:2:4,tail_call:10}, +@code{dbg_cnt(dce)} returns true only for third and fourth invocation. +For @code{dbg_cnt(tail_call)} true is returned for first 10 invocations. @item -print-file-name=@var{library} @opindex print-file-name diff --git a/gcc/testsuite/gcc.dg/ipa/ipa-icf-39.c b/gcc/testsuite/gcc.dg/ipa/ipa-icf-39.c new file mode 100644 index 00000000000..aa7c28706d3 --- /dev/null +++ b/gcc/testsuite/gcc.dg/ipa/ipa-icf-39.c @@ -0,0 +1,33 @@ +/* { dg-do compile } */ +/* { dg-options "-O2 -fdump-ipa-icf -fmerge-all-constants -fdbg-cnt=merged_ipa_icf:1:3" } */ +/* { dg-prune-output "dbg_cnt 'merged_ipa_icf' set to 1-3" } */ + +static int a; +static int b; +static const int c = 2; +static const int d = 2; +static char * e = "test"; +static char * f = "test"; +static int g[3]={1,2,3}; +static int h[3]={1,2,3}; +static const int *i=&c; +static const int *j=&c; +static const int *k=&d; +int t(int tt) +{ + switch (tt) + { + case 1: return a; + case 2: return b; + case 3: return c; + case 4: return d; + case 5: return e[1]; + case 6: return f[1]; + case 7: return g[1]; + case 8: return h[1]; + case 9: return i[0]; + case 10: return j[0]; + case 11: return k[0]; + } +} +/* { dg-final { scan-ipa-dump-times "Unified;" 2 "icf" } } */ diff --git a/gcc/testsuite/gcc.dg/pr68766.c b/gcc/testsuite/gcc.dg/pr68766.c index a0d549b946e..83f0e14b7d2 100644 --- a/gcc/testsuite/gcc.dg/pr68766.c +++ b/gcc/testsuite/gcc.dg/pr68766.c @@ -1,7 +1,7 @@ /* { dg-do compile } */ /* { dg-options "-O2 -ftree-vectorize -fdbg-cnt=vect_loop:1" } */ /* { dg-additional-options "-mavx2" { target { i?86-*-* x86_64-*-* } } } */ -/* { dg-prune-output "dbg_cnt 'vect_loop' set to 1" } */ +/* { dg-prune-output "dbg_cnt 'vect_loop' set to 1-1" } */ int a, b, g, h; int c[58];
On Wed, May 16, 2018 at 3:54 PM Martin Liška <mliska@suse.cz> wrote: > On 05/16/2018 03:39 PM, Alexander Monakov wrote: > > On Wed, 16 May 2018, Martin Liška wrote: > >>> Hm, is the off-by-one in the new explanatory text really intended? I think > >>> the previous text was accurate, and the new text should say "9th and 10th" > >>> and then "first 10 invocations", unless I'm missing something? > >> > >> I've reconsidered that once more time and having zero-based values: > >> * -fdbg-cnt=event:N - trigger event N-times > >> * -fdbg-cnt=event:N:(N+M) - skip even N-times and then enable it M-1 times > >> > >> Does that make sense? > > > > Yes, I like this, but I think the implementation does not match. New docs say: > > > >> -For example, with @option{-fdbg-cnt=dce:10,tail_call:0}, > >> -@code{dbg_cnt(dce)} returns true only for first 10 invocations. > >> +For example, with @option{-fdbg-cnt=dce:2:4,tail_call:10}, > >> +@code{dbg_cnt(dce)} returns true only for third and fourth invocation. > >> +For @code{dbg_cnt(tail_call)} true is returned for first 10 invocations. > > > > which is good, but the implementation reads: > > > >> bool > >> dbg_cnt_is_enabled (enum debug_counter index) > >> { > >> - return count[index] <= limit[index]; > >> + unsigned v = count[index]; > >> + return v >= limit_low[index] && v < limit_high[index]; > >> } > > > > which I believe is misaligned with the docs' intention. It should be the > > other way around: > > > > return v > limit_low[index] && v <= limit_high[index]; > Note that I changed count[index]++ to happen after dbg_cnt_is_enabled. I'm reverting that > and now it works fine with your condition. OK. Richard. > Martin > > > > Alexander > >
Hi Martin, > On 05/16/2018 03:39 PM, Alexander Monakov wrote: >> On Wed, 16 May 2018, Martin Liška wrote: >>>> Hm, is the off-by-one in the new explanatory text really intended? I think >>>> the previous text was accurate, and the new text should say "9th and 10th" >>>> and then "first 10 invocations", unless I'm missing something? >>> >>> I've reconsidered that once more time and having zero-based values: >>> * -fdbg-cnt=event:N - trigger event N-times >>> * -fdbg-cnt=event:N:(N+M) - skip even N-times and then enable it M-1 times >>> >>> Does that make sense? >> >> Yes, I like this, but I think the implementation does not match. New docs say: >> >>> -For example, with @option{-fdbg-cnt=dce:10,tail_call:0}, >>> -@code{dbg_cnt(dce)} returns true only for first 10 invocations. >>> +For example, with @option{-fdbg-cnt=dce:2:4,tail_call:10}, >>> +@code{dbg_cnt(dce)} returns true only for third and fourth invocation. >>> +For @code{dbg_cnt(tail_call)} true is returned for first 10 invocations. >> >> which is good, but the implementation reads: >> >>> bool >>> dbg_cnt_is_enabled (enum debug_counter index) >>> { >>> - return count[index] <= limit[index]; >>> + unsigned v = count[index]; >>> + return v >= limit_low[index] && v < limit_high[index]; >>> } >> >> which I believe is misaligned with the docs' intention. It should be the >> other way around: >> >> return v > limit_low[index] && v <= limit_high[index]; > > Note that I changed count[index]++ to happen after dbg_cnt_is_enabled. I'm > reverting that > and now it works fine with your condition. I'm seeing a testsuite regression for +FAIL: gcc.dg/pr68766.c (test for excess errors) on 32 and 64-bit Solaris/x86, Linux/x86_64, also seen on i686-pc-linux-gnu, powerpc64le-unknown-linux-gnu. Excess errors: dbg_cnt 'vect_loop' set to 0-1 Either the adjustment to gcc.dg/pr68766.c is wrong or not generic. Please fix. Rainer
On 05/18/2018 04:07 PM, Rainer Orth wrote: > Please fix. Thanks for the report, will install obvious fix. Martin From af8fbed777a1583fc2ac865e0e15cbb24fee6a81 Mon Sep 17 00:00:00 2001 From: marxin <mliska@suse.cz> Date: Fri, 18 May 2018 16:27:22 +0200 Subject: [PATCH] Fix typo in test-case. gcc/testsuite/ChangeLog: 2018-05-18 Martin Liska <mliska@suse.cz> * gcc.dg/pr68766.c: Change pruned output. --- gcc/testsuite/gcc.dg/pr68766.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gcc/testsuite/gcc.dg/pr68766.c b/gcc/testsuite/gcc.dg/pr68766.c index 83f0e14b7d2..097e555eb01 100644 --- a/gcc/testsuite/gcc.dg/pr68766.c +++ b/gcc/testsuite/gcc.dg/pr68766.c @@ -1,7 +1,7 @@ /* { dg-do compile } */ /* { dg-options "-O2 -ftree-vectorize -fdbg-cnt=vect_loop:1" } */ /* { dg-additional-options "-mavx2" { target { i?86-*-* x86_64-*-* } } } */ -/* { dg-prune-output "dbg_cnt 'vect_loop' set to 1-1" } */ +/* { dg-prune-output "dbg_cnt 'vect_loop' set to 0-1" } */ int a, b, g, h; int c[58];
diff --git a/gcc/common.opt b/gcc/common.opt index d6ef85928f3..d2f8736a62d 100644 --- a/gcc/common.opt +++ b/gcc/common.opt @@ -1171,7 +1171,7 @@ List all available debugging counters with their limits and counts. fdbg-cnt= Common RejectNegative Joined Var(common_deferred_options) Defer --fdbg-cnt=<counter>:<limit>[,<counter>:<limit>,...] Set the debug counter limit. +-fdbg-cnt=<counter>[:<lower_limit>]:<upper_limit>[,<counter>:<lower_limit>:<upper_limit>,...] Set the debug counter limit. fdebug-prefix-map= Common Joined RejectNegative Var(common_deferred_options) Defer diff --git a/gcc/dbgcnt.c b/gcc/dbgcnt.c index 96b3df28f5e..13d0ad8190a 100644 --- a/gcc/dbgcnt.c +++ b/gcc/dbgcnt.c @@ -41,53 +41,72 @@ static struct string2counter_map map[debug_counter_number_of_counters] = #undef DEBUG_COUNTER #define DEBUG_COUNTER(a) UINT_MAX, -static unsigned int limit[debug_counter_number_of_counters] = +static unsigned int limit_high[debug_counter_number_of_counters] = { #include "dbgcnt.def" }; #undef DEBUG_COUNTER +#define DEBUG_COUNTER(a) 1, +static unsigned int limit_low[debug_counter_number_of_counters] = +{ +#include "dbgcnt.def" +}; +#undef DEBUG_COUNTER + + static unsigned int count[debug_counter_number_of_counters]; bool dbg_cnt_is_enabled (enum debug_counter index) { - return count[index] <= limit[index]; + return limit_low[index] <= count[index] && count[index] <= limit_high[index]; } bool dbg_cnt (enum debug_counter index) { count[index]++; - if (dump_file && count[index] == limit[index]) - fprintf (dump_file, "***dbgcnt: limit reached for %s.***\n", - map[index].name); + + if (dump_file) + { + /* Do not print the info for default lower limit. */ + if (count[index] == limit_low[index] && limit_low[index] > 1) + fprintf (dump_file, "***dbgcnt: lower limit %d reached for %s.***\n", + limit_low[index], map[index].name); + else if (count[index] == limit_high[index]) + fprintf (dump_file, "***dbgcnt: upper limit %d reached for %s.***\n", + limit_high[index], map[index].name); + } return dbg_cnt_is_enabled (index); } - static void -dbg_cnt_set_limit_by_index (enum debug_counter index, int value) +dbg_cnt_set_limit_by_index (enum debug_counter index, int low, int high) { - limit[index] = value; + limit_low[index] = low; + limit_high[index] = high; - fprintf (stderr, "dbg_cnt '%s' set to %d\n", map[index].name, value); + fprintf (stderr, "dbg_cnt '%s' set to %d-%d\n", map[index].name, low, high); } static bool -dbg_cnt_set_limit_by_name (const char *name, int len, int value) +dbg_cnt_set_limit_by_name (const char *name, int low, int high) { + if (high < low) + error ("-fdbg-cnt=%s:%d:%d has smaller upper limit than the lower", + name, low, high); + int i; for (i = debug_counter_number_of_counters - 1; i >= 0; i--) - if (strncmp (map[i].name, name, len) == 0 - && map[i].name[len] == '\0') + if (strcmp (map[i].name, name) == 0) break; if (i < 0) return false; - dbg_cnt_set_limit_by_index ((enum debug_counter) i, value); + dbg_cnt_set_limit_by_index ((enum debug_counter) i, low, high); return true; } @@ -96,42 +115,53 @@ dbg_cnt_set_limit_by_name (const char *name, int len, int value) Returns NULL if there's no valid pair is found. Otherwise returns a pointer to the end of the pair. */ -static const char * +static bool dbg_cnt_process_single_pair (const char *arg) { - const char *colon = strchr (arg, ':'); - char *endptr = NULL; - int value; - - if (colon == NULL) - return NULL; - - value = strtol (colon + 1, &endptr, 10); - - if (endptr != NULL && endptr != colon + 1 - && dbg_cnt_set_limit_by_name (arg, colon - arg, value)) - return endptr; - - return NULL; + char *str = xstrdup (arg); + char *name = strtok (str, ":"); + char *value1 = strtok (NULL, ":"); + char *value2 = strtok (NULL, ":"); + + int high, low; + + if (value1 == NULL) + return NULL; + + if (value2 == NULL) + { + low = 1; + high = strtol (value1, NULL, 10); + } + else + { + low = strtol (value1, NULL, 10); + high = strtol (value2, NULL, 10); + } + + return dbg_cnt_set_limit_by_name (name, low, high); } void dbg_cnt_process_opt (const char *arg) { - const char *start = arg; - const char *next; + char *str = xstrdup (arg); + const char *next = strtok (str, ","); + unsigned int start = 0; + do { - next = dbg_cnt_process_single_pair (arg); - if (next == NULL) + if (!dbg_cnt_process_single_pair (arg)) break; - } while (*next == ',' && (arg = next + 1)); + start += strlen (arg) + 1; + next = strtok (NULL, ","); + } while (next != NULL); - if (next == NULL || *next != 0) + if (next != NULL) { - char *buffer = XALLOCAVEC (char, arg - start + 2); - sprintf (buffer, "%*c", (int)(1 + (arg - start)), '^'); + char *buffer = XALLOCAVEC (char, start + 2); + sprintf (buffer, "%*c", start + 1, '^'); error ("cannot find a valid counter:value pair:"); - error ("-fdbg-cnt=%s", start); + error ("-fdbg-cnt=%s", next); error (" %s", buffer); } } @@ -142,10 +172,11 @@ void dbg_cnt_list_all_counters (void) { int i; - printf (" %-30s %-5s %-5s\n", "counter name", "limit", "value"); - printf ("----------------------------------------------\n"); + printf (" %-32s %-11s %-12s\n", "counter name", "low limit", + "high limit"); + printf ("-----------------------------------------------------------------\n"); for (i = 0; i < debug_counter_number_of_counters; i++) - printf (" %-30s %5d %5u\n", - map[i].name, limit[map[i].counter], count[map[i].counter]); + printf (" %-30s %11u %12u\n", + map[i].name, limit_low[map[i].counter], limit_high[map[i].counter]); printf ("\n"); } diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi index ca3772bbebf..d7c5e1aa716 100644 --- a/gcc/doc/invoke.texi +++ b/gcc/doc/invoke.texi @@ -14326,13 +14326,14 @@ Print the name and the counter upper bound for all debug counters. @item -fdbg-cnt=@var{counter-value-list} @opindex fdbg-cnt -Set the internal debug counter upper bound. @var{counter-value-list} -is a comma-separated list of @var{name}:@var{value} pairs -which sets the upper bound of each debug counter @var{name} to @var{value}. +Set the internal debug counter lower and upper bound. @var{counter-value-list} +is a comma-separated list of @var{name}:@var{lower_bound}:@var{upper_bound} +tuples which sets the lower and the upper bound of each debug +counter @var{name}. All debug counters have the initial upper bound of @code{UINT_MAX}; thus @code{dbg_cnt} returns true always unless the upper bound is set by this option. -For example, with @option{-fdbg-cnt=dce:10,tail_call:0}, +For example, with @option{-fdbg-cnt=dce:9:10,tail_call:0}, @code{dbg_cnt(dce)} returns true only for first 10 invocations. @item -print-file-name=@var{library} diff --git a/gcc/testsuite/gcc.dg/ipa/ipa-icf-39.c b/gcc/testsuite/gcc.dg/ipa/ipa-icf-39.c new file mode 100644 index 00000000000..8759108a2d5 --- /dev/null +++ b/gcc/testsuite/gcc.dg/ipa/ipa-icf-39.c @@ -0,0 +1,33 @@ +/* { dg-do compile } */ +/* { dg-options "-O2 -fdump-ipa-icf -fmerge-all-constants -fdbg-cnt=merged_ipa_icf:1:2" } */ +/* { dg-prune-output "dbg_cnt 'merged_ipa_icf' set to 1-2" } */ + +static int a; +static int b; +static const int c = 2; +static const int d = 2; +static char * e = "test"; +static char * f = "test"; +static int g[3]={1,2,3}; +static int h[3]={1,2,3}; +static const int *i=&c; +static const int *j=&c; +static const int *k=&d; +int t(int tt) +{ + switch (tt) + { + case 1: return a; + case 2: return b; + case 3: return c; + case 4: return d; + case 5: return e[1]; + case 6: return f[1]; + case 7: return g[1]; + case 8: return h[1]; + case 9: return i[0]; + case 10: return j[0]; + case 11: return k[0]; + } +} +/* { dg-final { scan-ipa-dump-times "Unified;" 2 "icf" } } */ diff --git a/gcc/testsuite/gcc.dg/pr68766.c b/gcc/testsuite/gcc.dg/pr68766.c index a0d549b946e..83f0e14b7d2 100644 --- a/gcc/testsuite/gcc.dg/pr68766.c +++ b/gcc/testsuite/gcc.dg/pr68766.c @@ -1,7 +1,7 @@ /* { dg-do compile } */ /* { dg-options "-O2 -ftree-vectorize -fdbg-cnt=vect_loop:1" } */ /* { dg-additional-options "-mavx2" { target { i?86-*-* x86_64-*-* } } } */ -/* { dg-prune-output "dbg_cnt 'vect_loop' set to 1" } */ +/* { dg-prune-output "dbg_cnt 'vect_loop' set to 1-1" } */ int a, b, g, h; int c[58];