Re: [PATCH] Enhance syntax of -fdbg-cnt.

[Richard Biener <richard dot guenther at gmail dot com>]

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?  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<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)[];

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(-)
>
>