Re: [PATCH] Support lower and upper limit for -fdbg-cnt flag.

[Alexander Monakov <amonakov at ispras dot ru>]

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