From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 50246 invoked by alias); 16 May 2018 12:26:16 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 50237 invoked by uid 89); 16 May 2018 12:26:16 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-26.1 required=5.0 tests=BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,KAM_ASCII_DIVIDERS,SPF_PASS autolearn=ham version=3.3.2 spammy=10th X-HELO: mx2.suse.de Received: from mx2.suse.de (HELO mx2.suse.de) (195.135.220.15) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 16 May 2018 12:26:13 +0000 Received: from relay2.suse.de (charybdis-ext.suse.de [195.135.220.254]) by mx2.suse.de (Postfix) with ESMTP id 862B8ADB0; Wed, 16 May 2018 12:26:11 +0000 (UTC) Subject: Re: [PATCH] Support lower and upper limit for -fdbg-cnt flag. To: Alexander Monakov Cc: gcc-patches@gcc.gnu.org References: From: =?UTF-8?Q?Martin_Li=c5=a1ka?= Message-ID: <5c9ebc47-d4ff-108f-d4df-e361c1d31dae@suse.cz> Date: Wed, 16 May 2018 12:42:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.7.0 MIME-Version: 1.0 In-Reply-To: Content-Type: multipart/mixed; boundary="------------37B01ACEA845DA98F83912CD" X-IsSubscribed: yes X-SW-Source: 2018-05/txt/msg00731.txt.bz2 This is a multi-part message in MIME format. --------------37B01ACEA845DA98F83912CD Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Content-length: 3505 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=:[,:,...] Set the debug counter limit. >> +-fdbg-cnt=[:]:[,::,...] 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=[:]:[,:...] 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 > --------------37B01ACEA845DA98F83912CD Content-Type: text/x-patch; name="0001-Support-lower-and-upper-limit-for-fdbg-cnt-flag.patch" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename*0="0001-Support-lower-and-upper-limit-for-fdbg-cnt-flag.patch" Content-length: 10500 >From c57144bbe6cb339230f887918615b7a206716b82 Mon Sep 17 00:00:00 2001 From: marxin Date: Tue, 15 May 2018 15:04:30 +0200 Subject: [PATCH] Support lower and upper limit for -fdbg-cnt flag. gcc/ChangeLog: 2018-05-15 Martin Liska * dbgcnt.c (limit_low): Renamed from limit. (limit_high): New variable. (dbg_cnt_is_enabled): Check for upper limit. (dbg_cnt): Adjust dumping. (dbg_cnt_set_limit_by_index): Add new argument for high value. (dbg_cnt_set_limit_by_name): Likewise. (dbg_cnt_process_single_pair): Parse new format. (dbg_cnt_process_opt): Use strtok. (dbg_cnt_list_all_counters): Remove 'value' and add 'limit_high'. * doc/invoke.texi: Document changes. gcc/testsuite/ChangeLog: 2018-05-15 Martin Liska * gcc.dg/ipa/ipa-icf-39.c: New test. * gcc.dg/pr68766.c: Adjust pruned output. --- gcc/common.opt | 2 +- gcc/dbgcnt.c | 135 +++++++++++++++++++++++----------- gcc/doc/invoke.texi | 13 ++-- gcc/testsuite/gcc.dg/ipa/ipa-icf-39.c | 33 +++++++++ gcc/testsuite/gcc.dg/pr68766.c | 2 +- 5 files changed, 135 insertions(+), 50 deletions(-) create mode 100644 gcc/testsuite/gcc.dg/ipa/ipa-icf-39.c diff --git a/gcc/common.opt b/gcc/common.opt index d6ef85928f3..13ab5c65d43 100644 --- 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=:[,:,...] 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 96b3df28f5e..d6839c85c82 100644 --- a/gcc/dbgcnt.c +++ b/gcc/dbgcnt.c @@ -41,53 +41,90 @@ static struct string2counter_map map[debug_counter_number_of_counters] = #undef DEBUG_COUNTER #define DEBUG_COUNTER(a) UINT_MAX, -static unsigned int limit[debug_counter_number_of_counters] = +static unsigned int limit_high[debug_counter_number_of_counters] = { #include "dbgcnt.def" }; #undef DEBUG_COUNTER +#define DEBUG_COUNTER(a) 0, +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]; + unsigned v = count[index]; + return limit_low[index] <= v && v <= limit_high[index]; } bool dbg_cnt (enum debug_counter index) { + if (dump_file) + { + /* Do not print the info for default lower limit. */ + if (count[index] == limit_low[index] && limit_low[index] > 0) + fprintf (dump_file, "***dbgcnt: lower limit %d reached for %s.***\n", + limit_low[index], map[index].name); + else if (count[index] == limit_high[index]) + fprintf (dump_file, "***dbgcnt: upper limit %d reached for %s.***\n", + limit_high[index], map[index].name); + } + + bool r = dbg_cnt_is_enabled (index); count[index]++; - if (dump_file && count[index] == limit[index]) - fprintf (dump_file, "***dbgcnt: limit reached for %s.***\n", - map[index].name); - - return dbg_cnt_is_enabled (index); + return r; } - static void -dbg_cnt_set_limit_by_index (enum debug_counter index, int value) +dbg_cnt_set_limit_by_index (enum debug_counter index, int low, int high) { - limit[index] = value; + limit_low[index] = low; + limit_high[index] = high; - fprintf (stderr, "dbg_cnt '%s' set to %d\n", map[index].name, value); + fprintf (stderr, "dbg_cnt '%s' set to %d-%d\n", map[index].name, low, high); } static bool -dbg_cnt_set_limit_by_name (const char *name, int len, int value) +dbg_cnt_set_limit_by_name (const char *name, int low, int high) { + if (high < low) + { + error ("-fdbg-cnt=%s:%d:%d has smaller upper limit than the lower", + name, low, high); + return false; + } + + if (low < 0) + { + error ("Lower limit %d of -fdbg-cnt=%s must be a non-negative number", low, + name); + return false; + } + + if (high < 0) + { + error ("Upper limit %d of -fdbg-cnt=%s must be a non-negative number", high, + name); + return false; + } + int i; for (i = debug_counter_number_of_counters - 1; i >= 0; i--) - if (strncmp (map[i].name, name, len) == 0 - && map[i].name[len] == '\0') + if (strcmp (map[i].name, name) == 0) break; if (i < 0) return false; - dbg_cnt_set_limit_by_index ((enum debug_counter) i, value); + dbg_cnt_set_limit_by_index ((enum debug_counter) i, low, high); return true; } @@ -96,42 +133,53 @@ dbg_cnt_set_limit_by_name (const char *name, int len, int value) Returns NULL if there's no valid pair is found. Otherwise returns a pointer to the end of the pair. */ -static const char * +static bool dbg_cnt_process_single_pair (const char *arg) { - const char *colon = strchr (arg, ':'); - char *endptr = NULL; - int value; - - if (colon == NULL) - return NULL; - - value = strtol (colon + 1, &endptr, 10); - - if (endptr != NULL && endptr != colon + 1 - && dbg_cnt_set_limit_by_name (arg, colon - arg, value)) - return endptr; - - return NULL; + char *str = xstrdup (arg); + char *name = strtok (str, ":"); + char *value1 = strtok (NULL, ":"); + char *value2 = strtok (NULL, ":"); + + int high, low; + + if (value1 == NULL) + return NULL; + + if (value2 == NULL) + { + low = 0; + high = strtol (value1, NULL, 10); + } + else + { + low = strtol (value1, NULL, 10); + high = strtol (value2, NULL, 10); + } + + return dbg_cnt_set_limit_by_name (name, low, high); } void dbg_cnt_process_opt (const char *arg) { - const char *start = arg; - const char *next; + char *str = xstrdup (arg); + const char *next = strtok (str, ","); + unsigned int start = 0; + do { - next = dbg_cnt_process_single_pair (arg); - if (next == NULL) + if (!dbg_cnt_process_single_pair (arg)) break; - } while (*next == ',' && (arg = next + 1)); + start += strlen (arg) + 1; + next = strtok (NULL, ","); + } while (next != NULL); - if (next == NULL || *next != 0) + if (next != NULL) { - char *buffer = XALLOCAVEC (char, arg - start + 2); - sprintf (buffer, "%*c", (int)(1 + (arg - start)), '^'); + char *buffer = XALLOCAVEC (char, start + 2); + sprintf (buffer, "%*c", start + 1, '^'); error ("cannot find a valid counter:value pair:"); - error ("-fdbg-cnt=%s", start); + error ("-fdbg-cnt=%s", next); error (" %s", buffer); } } @@ -142,10 +190,11 @@ void dbg_cnt_list_all_counters (void) { int i; - printf (" %-30s %-5s %-5s\n", "counter name", "limit", "value"); - printf ("----------------------------------------------\n"); + printf (" %-32s %-11s %-12s\n", "counter name", "low limit", + "high limit"); + printf ("-----------------------------------------------------------------\n"); for (i = 0; i < debug_counter_number_of_counters; i++) - printf (" %-30s %5d %5u\n", - map[i].name, limit[map[i].counter], count[map[i].counter]); + printf (" %-30s %11u %12u\n", + map[i].name, limit_low[map[i].counter], limit_high[map[i].counter]); printf ("\n"); } diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi index ca3772bbebf..be55d28a38b 100644 --- a/gcc/doc/invoke.texi +++ b/gcc/doc/invoke.texi @@ -14326,14 +14326,17 @@ 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}. 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:10,tail_call:0}, -@code{dbg_cnt(dce)} returns true only for first 10 invocations. +For example, with @option{-fdbg-cnt=dce:9:10,tail_call:10}, +@code{dbg_cnt(dce)} returns true only for 10th and 11th invocation. +For @code{dbg_cnt(tail_call)} true is returned for first 11 invocations. @item -print-file-name=@var{library} @opindex print-file-name diff --git a/gcc/testsuite/gcc.dg/ipa/ipa-icf-39.c b/gcc/testsuite/gcc.dg/ipa/ipa-icf-39.c new file mode 100644 index 00000000000..8759108a2d5 --- /dev/null +++ b/gcc/testsuite/gcc.dg/ipa/ipa-icf-39.c @@ -0,0 +1,33 @@ +/* { dg-do compile } */ +/* { dg-options "-O2 -fdump-ipa-icf -fmerge-all-constants -fdbg-cnt=merged_ipa_icf:1:2" } */ +/* { dg-prune-output "dbg_cnt 'merged_ipa_icf' set to 1-2" } */ + +static int a; +static int b; +static const int c = 2; +static const int d = 2; +static char * e = "test"; +static char * f = "test"; +static int g[3]={1,2,3}; +static int h[3]={1,2,3}; +static const int *i=&c; +static const int *j=&c; +static const int *k=&d; +int t(int tt) +{ + switch (tt) + { + case 1: return a; + case 2: return b; + case 3: return c; + case 4: return d; + case 5: return e[1]; + case 6: return f[1]; + case 7: return g[1]; + case 8: return h[1]; + case 9: return i[0]; + case 10: return j[0]; + case 11: return k[0]; + } +} +/* { dg-final { scan-ipa-dump-times "Unified;" 2 "icf" } } */ diff --git a/gcc/testsuite/gcc.dg/pr68766.c b/gcc/testsuite/gcc.dg/pr68766.c index a0d549b946e..83f0e14b7d2 100644 --- a/gcc/testsuite/gcc.dg/pr68766.c +++ b/gcc/testsuite/gcc.dg/pr68766.c @@ -1,7 +1,7 @@ /* { 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 1" } */ +/* { dg-prune-output "dbg_cnt 'vect_loop' set to 1-1" } */ int a, b, g, h; int c[58]; -- 2.16.3 --------------37B01ACEA845DA98F83912CD--