Support lower and upper limit for -fdbg-cnt flag.

Message ID d305514e-74da-64b3-8779-7bbed7cd3f94@suse.cz
State New
Headers show
Series
  • Support lower and upper limit for -fdbg-cnt flag.
Related show

Commit Message

Martin Liška May 16, 2018, 7:15 a.m.
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.

Patch can bootstrap on x86_64-linux-gnu and survives regression tests.

Ready to be installed?
Martin

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                          | 113 ++++++++++++++++++++++------------
 gcc/doc/invoke.texi                   |   9 +--
 gcc/testsuite/gcc.dg/ipa/ipa-icf-39.c |  33 ++++++++++
 gcc/testsuite/gcc.dg/pr68766.c        |   2 +-
 5 files changed, 112 insertions(+), 47 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/ipa/ipa-icf-39.c

Comments

Alexander Monakov May 16, 2018, 7:47 a.m. | #1
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
Martin Liška May 16, 2018, 12:26 p.m. | #2
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];
Alexander Monakov May 16, 2018, 12:56 p.m. | #3
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
Martin Liška May 16, 2018, 1:24 p.m. | #4
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];
Alexander Monakov May 16, 2018, 1:39 p.m. | #5
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
Martin Liška May 16, 2018, 1:54 p.m. | #6
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];
Richard Biener May 17, 2018, 1:25 p.m. | #7
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
> >
Rainer Orth May 18, 2018, 2:07 p.m. | #8
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
Martin Liška May 18, 2018, 7:49 p.m. | #9
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];

Patch

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];