Re: [PATCH] Enhance syntax of -fdbg-cnt.
On Wed, Nov 13, 2019 at 10:15 AM Martin Liška wrote: > > On 11/12/19 10:00 AM, Martin Liška wrote: > > There's one another version that I'm testing right now. > > It survives regression tests and bootstrap. > > Richi, may I install it the patch? Yes. Thanks, Richard. > > Thanks, > Martin
Re: [PATCH] Enhance syntax of -fdbg-cnt.
On 11/12/19 10:00 AM, Martin Liška wrote: There's one another version that I'm testing right now. It survives regression tests and bootstrap. Richi, may I install it the patch? Thanks, Martin
Re: [PATCH] Enhance syntax of -fdbg-cnt.
On 11/12/19 9:35 AM, Richard Biener wrote:
On Mon, Nov 11, 2019 at 3:56 PM Martin Liška wrote:
On 11/11/19 3:19 PM, Richard Biener wrote:
-static unsigned int limit_low[debug_counter_number_of_counters];
+static auto_vec
*limits[debug_counter_number_of_counters] = {NULL};
Hm, apparently it's not working. I see a stack corruption when calling
dbgcnt. I also explicitly called .create (2) for all vectors, but the
ICE still happens.
You did use vec<> instead of auto_vec<> , did you? Using auto_vec<>
here IMHO is bogus anyways.
Anyway, I would probably go with the original patch. It's handy for me
to also have limits[index] == NULL to indicate a counter that is not set.
But there's vec.exists_p () which just checks whether the m_vec member
is NULL.
Since file-scope statics are zero-initialized by default even
static vec limits[debug_counter_number_of_counters];
should work.
Works for me ;)
There's one another version that I'm testing right now.
Martin
That's a different state from limits[index].is_empty() which means all
previous intervals were popped and we should return false.
Yes, is_empty() vs. exists_p().
Martin
>From 386fae51b258bd51b700596697b223e492960ff0 Mon Sep 17 00:00:00 2001
From: Martin Liska
Date: Fri, 8 Nov 2019 16:05:00 +0100
Subject: [PATCH] Enhance syntax of -fdbg-cnt.
gcc/ChangeLog:
2019-11-08 Martin Liska
* 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
* 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 | 166 --
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, 113 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=[:]:[,:...] Set the debug counter limit.
+-fdbg-cnt=[:-][:-:...][,:...] 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..283a4575425 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 limit_tuple;
-static unsigned int limit_low[debug_counter_number_of_counters];
+static vec limits[debug_counter_number_of_counters];
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,86 @@ 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].exists ())
+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 inde
Re: [PATCH] Enhance syntax of -fdbg-cnt.
On Mon, Nov 11, 2019 at 3:56 PM Martin Liška wrote:
>
> On 11/11/19 3:19 PM, Richard Biener wrote:
> > -static unsigned int limit_low[debug_counter_number_of_counters];
> > +static auto_vec
> > *limits[debug_counter_number_of_counters] = {NULL};
>
> Hm, apparently it's not working. I see a stack corruption when calling
> dbgcnt. I also explicitly called .create (2) for all vectors, but the
> ICE still happens.
You did use vec<> instead of auto_vec<> , did you? Using auto_vec<>
here IMHO is bogus anyways.
> Anyway, I would probably go with the original patch. It's handy for me
> to also have limits[index] == NULL to indicate a counter that is not set.
But there's vec.exists_p () which just checks whether the m_vec member
is NULL.
Since file-scope statics are zero-initialized by default even
static vec limits[debug_counter_number_of_counters];
should work.
> That's a different state from limits[index].is_empty() which means all
> previous intervals were popped and we should return false.
Yes, is_empty() vs. exists_p().
> Martin
Re: [PATCH] Enhance syntax of -fdbg-cnt.
On 11/11/19 4:25 PM, Martin Liška wrote:
I'm going to test the patch.
Martin
There's one more version where I use more references to mitigate
indirection of 'counter' vectors.
Martin
>From e3b8b3edfed1b5ba320d0fe85686908c5c37c22a Mon Sep 17 00:00:00 2001
From: Martin Liska
Date: Fri, 8 Nov 2019 16:05:00 +0100
Subject: [PATCH] Enhance syntax of -fdbg-cnt.
gcc/ChangeLog:
2019-11-08 Martin Liska
* 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
* 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 | 170 --
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, 117 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=[:]:[,:...] Set the debug counter limit.
+-fdbg-cnt=[:-][:-:...][,:...] 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..711ecc23619 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 limit_tuple;
-static unsigned int limit_low[debug_counter_number_of_counters];
+static auto_vec *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,89 @@ 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;
+
+ auto_vec &intervals = *limits[index];
+ if (intervals.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 = intervals.length () - 1;
+ unsigned int min = intervals[last].first;
+ unsigned int max = intervals[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)
+ intervals.pop ();
+ return true;
+}
+ else if (v < max)
+return true;
+ else if (v == max)
+{
+ print_limit_reach (map[index].name, v, true);
+ intervals.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)
+
Re: [PATCH] Enhance syntax of -fdbg-cnt.
On 11/11/19 3:50 PM, Richard Biener wrote:
On Mon, Nov 11, 2019 at 3:33 PM Martin Liška wrote:
On 11/11/19 3:19 PM, Richard Biener wrote:
On Mon, Nov 11, 2019 at 9:17 AM Martin Liška 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
*limits[debug_counter_number_of_counters] = {NULL};
I think you can avoid one indirection here (vec<> is just one pointer) by doing
static vec 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 (*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
* 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
* 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
Date: Fri, 8 Nov 2019 16:05:00 +0100
Subject: [PATCH] Enhance syntax of -fdbg-cnt.
gcc/ChangeLog:
2019-11-08 Martin Liska
* 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
* gcc.dg/ipa/ipa-icf-39.c: Update -f
Re: [PATCH] Enhance syntax of -fdbg-cnt.
On 11/11/19 3:19 PM, Richard Biener wrote:
-static unsigned int limit_low[debug_counter_number_of_counters];
+static auto_vec
*limits[debug_counter_number_of_counters] = {NULL};
Hm, apparently it's not working. I see a stack corruption when calling
dbgcnt. I also explicitly called .create (2) for all vectors, but the
ICE still happens.
Anyway, I would probably go with the original patch. It's handy for me
to also have limits[index] == NULL to indicate a counter that is not set.
That's a different state from limits[index].is_empty() which means all
previous intervals were popped and we should return false.
Martin
Re: [PATCH] Enhance syntax of -fdbg-cnt.
On Mon, Nov 11, 2019 at 3:33 PM Martin Liška wrote:
>
> On 11/11/19 3:19 PM, Richard Biener wrote:
> > On Mon, Nov 11, 2019 at 9:17 AM Martin Liška 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.
> > 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.
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
> > *limits[debug_counter_number_of_counters] = {NULL};
> >
> > I think you can avoid one indirection here (vec<> is just one pointer) by
> > doing
> >
> > static vec 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 (*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
> >>
> >> * 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
> >>
> >> * 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(-)
> >>
> >>
>
Re: [PATCH] Enhance syntax of -fdbg-cnt.
On 11/11/19 3:19 PM, Richard Biener wrote:
On Mon, Nov 11, 2019 at 9:17 AM Martin Liška 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.
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.
-static unsigned int limit_low[debug_counter_number_of_counters];
+static auto_vec
*limits[debug_counter_number_of_counters] = {NULL};
I think you can avoid one indirection here (vec<> is just one pointer) by doing
static vec 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 (*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
* 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
* 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(-)
Re: [PATCH] Enhance syntax of -fdbg-cnt.
On Mon, Nov 11, 2019 at 9:17 AM Martin Liška 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? I think sorted insertion
is reasonable here (I don't see you sorting them at all?).
-static unsigned int limit_low[debug_counter_number_of_counters];
+static auto_vec
*limits[debug_counter_number_of_counters] = {NULL};
I think you can avoid one indirection here (vec<> is just one pointer) by doing
static vec 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 (*limits)[];
Thanks,
Richard.
> Thanks,
> Martin
>
> gcc/ChangeLog:
>
> 2019-11-08 Martin Liska
>
> * 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
>
> * 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(-)
>
>
[PATCH] Enhance syntax of -fdbg-cnt.
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?
Thanks,
Martin
gcc/ChangeLog:
2019-11-08 Martin Liska
* 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
* 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(-)
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=[:]:[,:...] Set the debug counter limit.
+-fdbg-cnt=[:-][:-:...][,:...] 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..2a024b1b88d 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 limit_tuple;
-static unsigned int limit_low[debug_counter_number_of_counters];
+static auto_vec *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,74 @@ print_limit_reach (const char *counter, int limit, bool upper_p)
bool
dbg_cnt (enum debug_counter index)
{
- count[index]++;
+ unsigned v = ++count[index];
- /* 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);
+ if (limits[index] == NULL)
+return true;
+ else if (limits[index]->is_empty ())
+return false;
- return dbg_cnt_is_enabled (index);
-}
+ /* Reverse intervals exactly once. */
+ if (v == 1)
+limits[index]->reverse ();
-static void
-dbg_cnt_set_limit_by_index (enum debug_counter index, int low, int high)
-{
- limit_low[index] = low;
- limit_high[index] = high;
+ unsigned last = limits[index]->length () - 1;
+ unsigned int min = (*limits[index])[last].first;
+ unsigned int max = (*limits[index])[last].second;
- fprintf (stderr, "dbg_cnt '%s' set to %d-%d\n", map[index].name, low, high);
+ 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 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 lowe
