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 <mli...@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 <mli...@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 <mli...@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];
--
2.16.3
