This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] Enhance syntax of -fdbg-cnt.
- From: Martin Liška <mliska at suse dot cz>
- To: Richard Biener <richard dot guenther at gmail dot com>
- Cc: GCC Patches <gcc-patches at gcc dot gnu dot org>
- Date: Mon, 11 Nov 2019 16:25:11 +0100
- Subject: Re: [PATCH] Enhance syntax of -fdbg-cnt.
- References: <2fda9845-c1c2-0fdc-0ac6-f1fbb875d4f4@suse.cz> <CAFiYyc3tyfLjnDsFjU=_g7A1Fsx1rCdGtioLPmor-0+269egRw@mail.gmail.com> <f147c8a6-a268-d89b-8f1d-4f25137f6006@suse.cz> <CAFiYyc2HHPkAGHSO2bHokf5iNUHwU7ODziw6g=xXn6HKtKSQGg@mail.gmail.com>
On 11/11/19 3:50 PM, Richard Biener wrote:
On Mon, Nov 11, 2019 at 3:33 PM Martin Liška <mliska@suse.cz> wrote:
On 11/11/19 3:19 PM, Richard Biener wrote:
On Mon, Nov 11, 2019 at 9:17 AM Martin Liška <mliska@suse.cz> wrote:
Hi.
The patch makes debug counter more usable. In particular, one can now
list multiple closed intervals and -fdbg-cnt-list can reflect that.
Based on the discussion with Richard, I decided to leave semi-closed
intervals and make it closed, it's more intuitive.
Example:
$ g++ -O2 tramp3d-v4.ii -fdbg-cnt=match:1-2:6-10:11-20,dce:100:105-200,merged_ipa_icf:300-300 -c -fdbg-cnt-list
...
counter name closed intervals
-----------------------------------------------------------------
...
dce [1, 100], [105, 200]
...
match [1, 2], [6, 10], [11, 20]
merged_ipa_icf [300, 300]
Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
Ready to be installed?
+ /* Reverse intervals exactly once. */
+ if (v == 1)
+ limits[index]->reverse ();
why not do this at option processing time?
Because we don't have there any dbgcnt API. Right now one can use multiple
-fdbg-cnt options on a command line and that's why I'm doing that lazily.
But there's dbg_cnt_process_opt, that one can clearly insert intervals
in appropriately sorted way.
Yes, I'm doing that right now.
I think sorted insertion
is reasonable here (I don't see you sorting them at all?).
I do expect a sorted intervals by user. If it's not provided, an error message
is directly emitted.
I see, I find it overzealous esp. for multiple options like
-fdbg-cnt=foo:5-7 -fdbg-cnt=foo:2-3?
But yeah, it simplifies things. Still you could insert at the head
and avoid reversal.
I decided to sort the intervals for during each insertion.
It should not affect performance as one does not insert hundreds of intervals.
Doing that I can do following error detection:
$ g++ -O2 tramp3d-v4.ii -fdbg-cnt=match:2-10 -fdbg-cnt=match:200-300 -fdbg-cnt=match:6-7 -fdbg-cnt-list
cc1plus: error: Interval overlap of ‘-fdbg-cnt=match’: [2, 10] and [6, 7]
I'm going to test the patch.
Martin
Btw, there's vec::bsearch which might be convenient for sorted
insertion (even though
that doesn't return an index but a pointer to the element).
-static unsigned int limit_low[debug_counter_number_of_counters];
+static auto_vec<limit_tuple>
*limits[debug_counter_number_of_counters] = {NULL};
I think you can avoid one indirection here (vec<> is just one pointer) by doing
static vec<limit_tuple> limits[debug_counter_number_of_counters] = { vNULL };
? Verify you get no runtime ctor, not sure if that's correct C++ for
initializing
all elements by vNULL, not just the first...
Alternatively I'd lazily allocate the whole vector rather than each individual
vector (at option processing time again), so
static vec<limit_tuple> (*limits)[];
I fully agree with that we should reduce one level of indirection.
Lemme take a look..
Thanks,
Martin
Thanks,
Richard.
Thanks,
Martin
gcc/ChangeLog:
2019-11-08 Martin Liska <mliska@suse.cz>
* common.opt: Document change of -fdbg-cnt option.
* dbgcnt.c (DEBUG_COUNTER): Remove.
(dbg_cnt_is_enabled): Remove.
(dbg_cnt): Work with new intervals.
(dbg_cnt_set_limit_by_index): Set to new
list of intervals.
(dbg_cnt_set_limit_by_name): Likewise.
(dbg_cnt_process_single_pair): Process new format.
(dbg_cnt_process_opt): Likewise.
(dbg_cnt_list_all_counters): Likewise.
* doc/invoke.texi: Document change of -fdbg-cnt option.
gcc/testsuite/ChangeLog:
2019-11-08 Martin Liska <mliska@suse.cz>
* gcc.dg/ipa/ipa-icf-39.c: Update -fdbg-cnt to the new format.
* gcc.dg/pr68766.c: Likewise.
---
gcc/common.opt | 2 +-
gcc/dbgcnt.c | 157 +++++++++++++++-----------
gcc/doc/invoke.texi | 15 ++-
gcc/testsuite/gcc.dg/ipa/ipa-icf-39.c | 3 +-
gcc/testsuite/gcc.dg/pr68766.c | 1 -
5 files changed, 103 insertions(+), 75 deletions(-)
>From f62eeec40c31c2c5dddc855b1c05cf5c6ee7f66d Mon Sep 17 00:00:00 2001
From: Martin Liska <mliska@suse.cz>
Date: Fri, 8 Nov 2019 16:05:00 +0100
Subject: [PATCH] Enhance syntax of -fdbg-cnt.
gcc/ChangeLog:
2019-11-08 Martin Liska <mliska@suse.cz>
* common.opt: Document change of -fdbg-cnt option.
* dbgcnt.c (DEBUG_COUNTER): Remove.
(dbg_cnt_is_enabled): Remove.
(dbg_cnt): Work with new intervals.
(dbg_cnt_set_limit_by_index): Set to new
list of intervals.
(dbg_cnt_set_limit_by_name): Likewise.
(dbg_cnt_process_single_pair): Process new format.
(dbg_cnt_process_opt): Likewise.
(dbg_cnt_list_all_counters): Likewise.
* doc/invoke.texi: Document change of -fdbg-cnt option.
(cmp_tuples): New.
gcc/testsuite/ChangeLog:
2019-11-08 Martin Liska <mliska@suse.cz>
* gcc.dg/ipa/ipa-icf-39.c: Update -fdbg-cnt to the new format.
* gcc.dg/pr68766.c: Likewise.
---
gcc/common.opt | 2 +-
gcc/dbgcnt.c | 168 ++++++++++++++++----------
gcc/doc/invoke.texi | 15 ++-
gcc/testsuite/gcc.dg/ipa/ipa-icf-39.c | 3 +-
gcc/testsuite/gcc.dg/pr68766.c | 1 -
5 files changed, 115 insertions(+), 74 deletions(-)
diff --git a/gcc/common.opt b/gcc/common.opt
index 12c0083964e..d4442565c73 100644
--- a/gcc/common.opt
+++ b/gcc/common.opt
@@ -1198,7 +1198,7 @@ List all available debugging counters with their limits and counts.
fdbg-cnt=
Common RejectNegative Joined Var(common_deferred_options) Defer
--fdbg-cnt=<counter>[:<lower_limit>]:<upper_limit>[,<counter>:...] Set the debug counter limit.
+-fdbg-cnt=<counter>[:<lower_limit1>-]<upper_limit1>[:<lower_limit2>-<upper_limit2>:...][,<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 15ffe115a4f..410411f28d7 100644
--- a/gcc/dbgcnt.c
+++ b/gcc/dbgcnt.c
@@ -40,24 +40,12 @@ static struct string2counter_map map[debug_counter_number_of_counters] =
};
#undef DEBUG_COUNTER
-#define DEBUG_COUNTER(a) UINT_MAX,
-static unsigned int limit_high[debug_counter_number_of_counters] =
-{
-#include "dbgcnt.def"
-};
-#undef DEBUG_COUNTER
+typedef std::pair<unsigned int, unsigned int> limit_tuple;
-static unsigned int limit_low[debug_counter_number_of_counters];
+static auto_vec<limit_tuple> *limits[debug_counter_number_of_counters] = {NULL};
static unsigned int count[debug_counter_number_of_counters];
-bool
-dbg_cnt_is_enabled (enum debug_counter index)
-{
- unsigned v = count[index];
- return v > limit_low[index] && v <= limit_high[index];
-}
-
static void
print_limit_reach (const char *counter, int limit, bool upper_p)
{
@@ -72,47 +60,87 @@ print_limit_reach (const char *counter, int limit, bool upper_p)
bool
dbg_cnt (enum debug_counter index)
{
- count[index]++;
+ unsigned v = ++count[index];
+
+ if (limits[index] == NULL)
+ return true;
+ else if (limits[index]->is_empty ())
+ return false;
- /* Do not print the info for default lower limit. */
- if (count[index] == limit_low[index] && limit_low[index] > 0)
- print_limit_reach (map[index].name, limit_low[index], false);
- else if (count[index] == limit_high[index])
- print_limit_reach (map[index].name, limit_high[index], true);
+ unsigned last = limits[index]->length () - 1;
+ unsigned int min = (*limits[index])[last].first;
+ unsigned int max = (*limits[index])[last].second;
- return dbg_cnt_is_enabled (index);
+ if (v < min)
+ return false;
+ else if (v == min)
+ {
+ print_limit_reach (map[index].name, v, false);
+ if (min == max)
+ limits[index]->pop ();
+ return true;
+ }
+ else if (v < max)
+ return true;
+ else if (v == max)
+ {
+ print_limit_reach (map[index].name, v, true);
+ limits[index]->pop ();
+ return true;
+ }
+ else
+ return false;
}
-static void
-dbg_cnt_set_limit_by_index (enum debug_counter index, int low, int high)
-{
- limit_low[index] = low;
- limit_high[index] = high;
+/* Compare limit_tuple intervals by first item in descending order. */
- fprintf (stderr, "dbg_cnt '%s' set to %d-%d\n", map[index].name, low, high);
+static int
+cmp_tuples (const void *ptr1, const void *ptr2)
+{
+ const limit_tuple *p1 = (const limit_tuple *)ptr1;
+ const limit_tuple *p2 = (const limit_tuple *)ptr2;
+
+ if (p1->first < p2->first)
+ return 1;
+ else if (p1->first > p2->first)
+ return -1;
+ return 0;
}
static bool
-dbg_cnt_set_limit_by_name (const char *name, int low, int high)
+dbg_cnt_set_limit_by_index (enum debug_counter index, const char *name,
+ unsigned int low, unsigned int high)
{
- if (high < low)
- {
- error ("%<-fdbg-cnt=%s:%d:%d%> has smaller upper limit than the lower",
- name, low, high);
- return false;
- }
+ if (limits[index] == NULL)
+ limits[index] = new auto_vec<limit_tuple> ();
- if (low < 0)
+ limits[index]->safe_push (limit_tuple (low, high));
+ limits[index]->qsort (cmp_tuples);
+
+ for (unsigned i = 0; i < limits[index]->length () - 1; i++)
{
- error ("Lower limit %d of %<-fdbg-cnt=%s%> must be a non-negative "
- "number", low, name);
- return false;
+ auto_vec<limit_tuple> *intervals = limits[index];
+ limit_tuple t1 = (*intervals)[i];
+ limit_tuple t2 = (*intervals)[i + 1];
+ if (t1.first <= t2.second)
+ {
+ error ("Interval overlap of %<-fdbg-cnt=%s%>: [%u, %u] and "
+ "[%u, %u]\n", name, t2.first, t2.second, t1.first, t1.second);
+ return false;
+ }
}
- if (high < 0)
+ return true;
+}
+
+static bool
+dbg_cnt_set_limit_by_name (const char *name, unsigned int low,
+ unsigned int high)
+{
+ if (high < low)
{
- error ("Upper limit %d of %<-fdbg-cnt=%s%> must be a non-negative "
- "number", high, name);
+ error ("%<-fdbg-cnt=%s:%d-%d%> has smaller upper limit than the lower",
+ name, low, high);
return false;
}
@@ -124,31 +152,27 @@ dbg_cnt_set_limit_by_name (const char *name, int low, int high)
if (i < 0)
return false;
- dbg_cnt_set_limit_by_index ((enum debug_counter) i, low, high);
- return true;
+ return dbg_cnt_set_limit_by_index ((enum debug_counter) i, name, low, high);
}
-
-/* Process a single "name:value" pair.
+/* Process a single "low:high" pair.
Returns NULL if there's no valid pair is found.
Otherwise returns a pointer to the end of the pair. */
static bool
-dbg_cnt_process_single_pair (const char *arg)
+dbg_cnt_process_single_pair (char *name, char *str)
{
- char *str = xstrdup (arg);
- char *name = strtok (str, ":");
- char *value1 = strtok (NULL, ":");
- char *value2 = strtok (NULL, ":");
+ char *value1 = strtok (str, "-");
+ char *value2 = strtok (NULL, "-");
- int high, low;
+ unsigned int high, low;
if (value1 == NULL)
return false;
if (value2 == NULL)
{
- low = 0;
+ low = 1;
high = strtol (value1, NULL, 10);
}
else
@@ -166,17 +190,24 @@ dbg_cnt_process_opt (const char *arg)
char *str = xstrdup (arg);
unsigned int start = 0;
- auto_vec<const char *> tokens;
- for (const char *next = strtok (str, ","); next != NULL;
- next = strtok (NULL, ","))
+ auto_vec<char *> tokens;
+ for (char *next = strtok (str, ","); next != NULL; next = strtok (NULL, ","))
tokens.safe_push (next);
unsigned i;
for (i = 0; i < tokens.length (); i++)
{
- if (!dbg_cnt_process_single_pair (tokens[i]))
- break;
- start += strlen (tokens[i]) + 1;
+ auto_vec<char *> ranges;
+ char *name = strtok (tokens[i], ":");
+ for (char *part = strtok (NULL, ":"); part; part = strtok (NULL, ":"))
+ ranges.safe_push (part);
+
+ for (unsigned j = 0; j < ranges.length (); j++)
+ {
+ if (!dbg_cnt_process_single_pair (name, ranges[j]))
+ break;
+ }
+ start += strlen (tokens[i]) + 1;
}
if (i != tokens.length ())
@@ -195,11 +226,24 @@ void
dbg_cnt_list_all_counters (void)
{
int i;
- printf (" %-32s %-11s %-12s\n", "counter name", "low limit",
- "high limit");
+ printf (" %-30s %s\n", "counter name", "closed intervals");
printf ("-----------------------------------------------------------------\n");
for (i = 0; i < debug_counter_number_of_counters; i++)
- printf (" %-30s %11u %12u\n",
- map[i].name, limit_low[map[i].counter], limit_high[map[i].counter]);
+ {
+ printf (" %-30s ", map[i].name);
+ if (limits[i] != NULL)
+ {
+ for (int j = limits[i]->length () - 1; j >= 0; j--)
+ {
+ printf ("[%u, %u]", (*limits[i])[j].first,
+ (*limits[i])[j].second);
+ if (j > 0)
+ printf (", ");
+ }
+ putchar ('\n');
+ }
+ else
+ printf ("unset\n");
+ }
printf ("\n");
}
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index 00eb7e77808..a5a06262848 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -15656,15 +15656,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 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
+is a comma-separated list of @var{name}:@var{lower_bound1}-@var{upper_bound1}
+[:@var{lower_bound2}-@var{upper_bound2}...] tuples which sets
+the name of the counter and list of closed intervals.
+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:2:4,tail_call:10},
-@code{dbg_cnt(dce)} returns true only for third and fourth invocation.
+For example, with @option{-fdbg-cnt=dce:2-4:10-11,tail_call:10},
+@code{dbg_cnt(dce)} returns true only for second, third, fourth, tenth and
+eleventh invocation.
For @code{dbg_cnt(tail_call)} true is returned 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
index 83ccd20ac56..cf03894c4e7 100644
--- a/gcc/testsuite/gcc.dg/ipa/ipa-icf-39.c
+++ b/gcc/testsuite/gcc.dg/ipa/ipa-icf-39.c
@@ -1,7 +1,6 @@
/* { dg-do compile } */
/* { dg-require-alias "" } */
-/* { dg-options "-O2 -fdump-ipa-icf-optimized -fmerge-all-constants -fdbg-cnt=merged_ipa_icf:1:3" } */
-/* { dg-prune-output "dbg_cnt 'merged_ipa_icf' set to 1-3" } */
+/* { dg-options "-O2 -fdump-ipa-icf-optimized -fmerge-all-constants -fdbg-cnt=merged_ipa_icf:1-2" } */
/* { dg-prune-output "\\*\\*\\*dbgcnt:.*limit.*reached" } */
static int a;
diff --git a/gcc/testsuite/gcc.dg/pr68766.c b/gcc/testsuite/gcc.dg/pr68766.c
index 5308b23d30b..82b54d4743b 100644
--- a/gcc/testsuite/gcc.dg/pr68766.c
+++ b/gcc/testsuite/gcc.dg/pr68766.c
@@ -1,7 +1,6 @@
/* { 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 0-1" } */
/* { dg-prune-output "\\*\\*\\*dbgcnt:.*limit.*reached" } */
int a, b, g, h;
--
2.24.0