Re: [PATCH 4/7] GCOV: add -j argument (human readable format).

["Martin Liška" <mliska@suse.cz>]

[-- Attachment #1: Type: text/plain, Size: 2288 bytes --]

On 10/30/2017 01:35 PM, Nathan Sidwell wrote:
> On 10/26/2017 04:11 AM, marxin wrote:
>> Human readable format is quite useful in my opinion. There's example:
>>
>>          -:    1:unsigned
>>     14.00K:    2:loop (unsigned n, int value)
> 
> My first thought is 'why 2 decimal places'?  That seems excessive.  Zero surely suffices?

Hi.

Agree.

> 
>> Question is do we want to do it by default, or a new option is fine?
>> Note that all external tools using gcov should use intermediate format
>> which is obviously unchanged.
> 
> I don't think other tools do it by default.

Works for me/

> 
> 
>> +     [@option{-j}|@option{--human-numbers}]
> 
> man ls:
>        -h, --human-readable
>                with -l and/or -s, print human readable sizes (e.g., 1K 234M 2G)
> 
> Sadly '-h' is help (IIRC).  but we could at least copy the long form.

Yep, copied.

> 
> 
>> +  const char *units[] = {"", "K", "M", "G", "Y", "P", "E", "Z"};
> 
> Those aren't right  KMGTPEZY
> http://www.npl.co.uk/reference/measurement-units/si-prefixes/

Ha, SI units. Last time I heart about it was at grammar school during Physics classes ;)
Following that.

> 
> 
>> +  for (unsigned i = 0; i < sizeof (units); i++)
>> +    {
>> +      if (v < 1000.0f)
>> +    {
>> +      sprintf (buffer, "%3.2f%s", v, units[i]);
>> +      return buffer;
>> +    }
>> +
>> +      v /= 1000.0f;
>> +    }
> 
> that's going to fail on certain roundings.  You're doing multiple divisions by 1000, which itself will have roundings.  But more importantly, numbers after scaling like 999.999 will round to 1000, and you probably don't want that.  This kind of formatting is tricky.  My inclination is to keep it in the integer domain.  Determine a scaling factor.  Divide once by that and then explicitly round to nearest even[*] with a check for the 999.9 case.  Then print as an unsigned.

Done that.

Is the patch fine to be installed?

Martin

> 
> nathan
> 
> [*] I presume you're familiar with RNE?  That's probably overkill and plain RN would be adequate.
> 


[-- Attachment #2: 0004-GCOV-add-j-argument-human-readable-format-v2.patch --]
[-- Type: text/x-patch, Size: 7543 bytes --]

From b14bed2b376dc60eb35c07c5521007d16b137c78 Mon Sep 17 00:00:00 2001
From: marxin <mliska@suse.cz>
Date: Thu, 26 Oct 2017 10:11:16 +0200
Subject: [PATCH 4/8] GCOV: add -j argument (human readable format).

Human readable format is quite useful in my opinion. There's example:

        -:    4:unsigned
      14k:    5:loop (unsigned n, int value)		  /* count(14k) */
        -:    6:{
      21M:    7:  for (unsigned i = 0; i < n - 1; i++)
        -:    8:  {
      21M:    9:    value += i;				  /* count(21M) */
        -:   10:  }
        -:   11:
      14k:   12:  return value;
        -:   13:}
        -:   14:
        1:   15:int main(int argc, char **argv)
        -:   16:{
        1:   17:  unsigned sum = 0;
       7k:   18:  for (unsigned i = 0; i < 7 * 1000; i++)
        -:   19:  {
       7k:   20:    sum += loop (1000, sum);
       7k:   21:    sum += loop (2000, sum);		  /* count(7k) */
        -:   22:  }
        -:   23:
        1:   24:  return 0;				  /* count(1) */
        -:   25:}
        -:   26:

gcc/ChangeLog:

2017-10-23  Martin Liska  <mliska@suse.cz>

	* doc/gcov.texi: Document new option.
	* gcov.c (print_usage): Likewise print it.
	(process_args): Support the argument.
	(format_count): New function.
	(format_gcov): Use the function.

gcc/testsuite/ChangeLog:

2017-10-23  Martin Liska  <mliska@suse.cz>

	* g++.dg/gcov/loop.C: New test.
	* lib/gcov.exp: Support human readable format for counts.
---
 gcc/doc/gcov.texi                |  5 +++++
 gcc/gcov.c                       | 45 ++++++++++++++++++++++++++++++++++++++--
 gcc/testsuite/g++.dg/gcov/loop.C | 27 ++++++++++++++++++++++++
 gcc/testsuite/lib/gcov.exp       |  2 +-
 4 files changed, 76 insertions(+), 3 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/gcov/loop.C

diff --git a/gcc/doc/gcov.texi b/gcc/doc/gcov.texi
index e186ac6e1ea..5c4ba8a51a7 100644
--- a/gcc/doc/gcov.texi
+++ b/gcc/doc/gcov.texi
@@ -125,6 +125,7 @@ gcov [@option{-v}|@option{--version}] [@option{-h}|@option{--help}]
      [@option{-d}|@option{--display-progress}]
      [@option{-f}|@option{--function-summaries}]
      [@option{-i}|@option{--intermediate-format}]
+     [@option{-j}|@option{--human-readable}]
      [@option{-k}|@option{--use-colors}]
      [@option{-l}|@option{--long-file-names}]
      [@option{-m}|@option{--demangled-names}]
@@ -186,6 +187,10 @@ be used by @command{lcov} or other tools. The output is a single
 The format of the intermediate @file{.gcov} file is plain text with
 one entry per line
 
+@item -j
+@itemx --human-readable
+Write counts in human readable format (like 24k).
+
 @smallexample
 file:@var{source_file_name}
 function:@var{line_number},@var{execution_count},@var{function_name}
diff --git a/gcc/gcov.c b/gcc/gcov.c
index f9334f96eb3..ff246c104e4 100644
--- a/gcc/gcov.c
+++ b/gcc/gcov.c
@@ -44,6 +44,7 @@ along with Gcov; see the file COPYING3.  If not see
 #include "color-macros.h"
 
 #include <getopt.h>
+#include <math.h>
 
 #include "md5.h"
 
@@ -393,6 +394,10 @@ static int flag_use_colors = 0;
 
 static int flag_all_blocks = 0;
 
+/* Output human readable numbers.  */
+
+static int flag_human_readable_numbers = 0;
+
 /* Output summary info for each function.  */
 
 static int flag_function_summary = 0;
@@ -710,6 +715,7 @@ print_usage (int error_p)
   fnotice (file, "  -f, --function-summaries        Output summaries for each function\n");
   fnotice (file, "  -h, --help                      Print this help, then exit\n");
   fnotice (file, "  -i, --intermediate-format       Output .gcov file in intermediate text format\n");
+  fnotice (file, "  -j, --human-readable            Output human readable numbers\n");
   fnotice (file, "  -k, --use-colors                Emit colored output\n");
   fnotice (file, "  -l, --long-file-names           Use long output file names for included\n\
                                     source files\n");
@@ -752,6 +758,7 @@ static const struct option options[] =
   { "branch-probabilities", no_argument,       NULL, 'b' },
   { "branch-counts",        no_argument,       NULL, 'c' },
   { "intermediate-format",  no_argument,       NULL, 'i' },
+  { "human-readable",	    no_argument,       NULL, 'j' },
   { "no-output",            no_argument,       NULL, 'n' },
   { "long-file-names",      no_argument,       NULL, 'l' },
   { "function-summaries",   no_argument,       NULL, 'f' },
@@ -775,7 +782,7 @@ process_args (int argc, char **argv)
 {
   int opt;
 
-  const char *opts = "abcdfhiklmno:prs:uvwx";
+  const char *opts = "abcdfhijklmno:prs:uvwx";
   while ((opt = getopt_long (argc, argv, opts, options, NULL)) != -1)
     {
       switch (opt)
@@ -798,6 +805,9 @@ process_args (int argc, char **argv)
 	case 'l':
 	  flag_long_names = 1;
 	  break;
+	case 'j':
+	  flag_human_readable_numbers = 1;
+	  break;
 	case 'k':
 	  flag_use_colors = 1;
 	  break;
@@ -1938,6 +1948,37 @@ add_branch_counts (coverage_t *coverage, const arc_t *arc)
     }
 }
 
+/* Format COUNT, if flag_human_readable_numbers is set, return it human
+   readable format.  */
+
+static char const *
+format_count (gcov_type count)
+{
+  static char buffer[64];
+  float v = count;
+  const char *units = " kMGTPEZY";
+
+  if (count < 1000 || !flag_human_readable_numbers)
+    {
+      sprintf (buffer, "%" PRId64, count);
+      return buffer;
+    }
+
+  gcov_type divisor = 1;
+  for (unsigned i = 0; i < strlen (units); i++)
+    {
+      if (v < (1000 * divisor))
+	{
+	  gcov_type r = (gcov_type)roundf (v / divisor);
+	  sprintf (buffer, "%" PRId64 "%c", r, units[i]);
+	  return buffer;
+	}
+      divisor *= 1000;
+    }
+
+  gcc_unreachable ();
+}
+
 /* Format a GCOV_TYPE integer as either a percent ratio, or absolute
    count.  If dp >= 0, format TOP/BOTTOM * 100 to DP decimal places.
    If DP is zero, no decimal point is printed. Only print 100% when
@@ -1985,7 +2026,7 @@ format_gcov (gcov_type top, gcov_type bottom, int dp)
 	}
     }
   else
-    sprintf (buffer, "%" PRId64, (int64_t)top);
+    return format_count (top);
 
   return buffer;
 }
diff --git a/gcc/testsuite/g++.dg/gcov/loop.C b/gcc/testsuite/g++.dg/gcov/loop.C
new file mode 100644
index 00000000000..7f3be5587af
--- /dev/null
+++ b/gcc/testsuite/g++.dg/gcov/loop.C
@@ -0,0 +1,27 @@
+/* { dg-options "-fprofile-arcs -ftest-coverage" } */
+/* { dg-do run { target native } } */
+
+unsigned
+loop (unsigned n, int value)		  /* count(14k) */
+{
+  for (unsigned i = 0; i < n - 1; i++)
+  {
+    value += i;				  /* count(21M) */
+  }
+
+  return value;
+}
+
+int main(int argc, char **argv)
+{
+  unsigned sum = 0;
+  for (unsigned i = 0; i < 7 * 1000; i++)
+  {
+    sum += loop (1000, sum);
+    sum += loop (2000, sum);		  /* count(7k) */
+  }
+
+  return 0;				  /* count(1) */
+}
+
+/* { dg-final { run-gcov branches { -abj loop.C } } } */
diff --git a/gcc/testsuite/lib/gcov.exp b/gcc/testsuite/lib/gcov.exp
index 18adc71351a..ede01e70212 100644
--- a/gcc/testsuite/lib/gcov.exp
+++ b/gcc/testsuite/lib/gcov.exp
@@ -59,7 +59,7 @@ proc verify-lines { testname testcase file } {
     while { [gets $fd line] >= 0 } {
         # We want to match both "-" and "#####" as count as well as numbers,
         # since we want to detect lines that shouldn't be marked as covered.
-	if [regexp "^ *(\[^:]*): *(\[0-9\\-#]+):.*count\\((\[0-9\\-#=]+)\\)(.*)" \
+	if [regexp "^ *(\[^:]*): *(\[0-9\\-#]+):.*count\\((\[0-9\\-#=\\.kMGTPEZY]+)\\)(.*)" \
 		"$line" all is n shouldbe rest] {
 	    if [regexp "^ *{(.*)}" $rest all xfailed] {
 		switch [dg-process-target $xfailed] {
-- 
2.14.3