[PATCH] Support lower and upper limit for -fdbg-cnt flag.
Martin Liška
mliska@suse.cz
Wed May 16 12:42:00 GMT 2018
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
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Support-lower-and-upper-limit-for-fdbg-cnt-flag.patch
Type: text/x-patch
Size: 10500 bytes
Desc: not available
URL: <http://gcc.gnu.org/pipermail/gcc-patches/attachments/20180516/75fc70d0/attachment.bin>
More information about the Gcc-patches
mailing list