public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: "Martin Liška" <mliska@suse.cz>
To: Jan Hubicka <hubicka@ucw.cz>
Cc: Richard Biener <richard.guenther@gmail.com>, gcc-patches@gcc.gnu.org
Subject: Re: [PATCH] gcov: add info about "calls" to JSON output format
Date: Tue, 25 Apr 2023 13:05:32 +0200	[thread overview]
Message-ID: <f055fae0-c048-307d-d22b-13f56be3834e@suse.cz> (raw)
In-Reply-To: <ZDlITkrR0xeiMKqX@kam.mff.cuni.cz>

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

On 4/14/23 14:34, Jan Hubicka wrote:
>> On 4/11/23 11:23, Richard Biener wrote:
>>> On Thu, Apr 6, 2023 at 3:58 PM Martin Liška <mliska@suse.cz> wrote:
>>>>
>>>> Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
>>>>
>>>> Ready to be installed after stage1 opens?
>>>
>>> Did we release a compiler with version 1?  If not we might want to sneak
>>
>> Yes, all compilers starting with 9.1 emit version 1.
>>
>>> this in before 13.1 ...
>>
>> Yep, I would welcome sneaking in.
>>
>>>
>>> Up to Honza.
>>
>> PING: Honza!
> 
> THe motivation is for external tools to effectively track execution
> counts after function calls that does not return normally?

Yes, they wanted basically equal information as we present in the human-readable
format.

> What about other cases we produce fake edges for, like trapping
> statements or volatile asms (via stmt_can_terminate_bb_p). Are those
> handled as branches with "throw" flag?

That's probably something for future. There was not demand for it right now.

> 
> Calls not returning is just one of special cases where control of
> function may terminate.  Also do we want to handle function returning
> twice (fork and setjmp?)

Similarly here.

Anyway, I'm going to commit a bit enhanced version where I:
- added blocks_ids for each line number
- for each branch and call source and destination block is added
- in human-readable format I change block::index to block::id as the
  index does not makes sense much

Martin

> 
> Patch looks OK.
> Honza
>>
>> Thanks,
>> Martin
>>
>>>
>>> Thanks,
>>> Richard.
>>>
>>>> Thanks,
>>>> Martin
>>>>
>>>> gcc/ChangeLog:
>>>>
>>>>         * doc/gcov.texi: Document the new "calls" field and document
>>>>         the API bump.
>>>>         * gcov.cc (output_intermediate_json_line): Output info about
>>>>         calls.
>>>>         (generate_results): Bump version to 2.
>>>>
>>>> gcc/testsuite/ChangeLog:
>>>>
>>>>         * g++.dg/gcov/gcov-17.C: Add call to a noreturn function.
>>>>         * g++.dg/gcov/test-gcov-17.py: Cover new format.
>>>>         * lib/gcov.exp: Add options for gcov that emit the extra info.
>>>> ---
>>>>  gcc/doc/gcov.texi                         | 27 +++++++++++++++++++++--
>>>>  gcc/gcov.cc                               | 12 +++++++++-
>>>>  gcc/testsuite/g++.dg/gcov/gcov-17.C       |  7 ++++++
>>>>  gcc/testsuite/g++.dg/gcov/test-gcov-17.py | 17 ++++++++++----
>>>>  gcc/testsuite/lib/gcov.exp                |  2 +-
>>>>  5 files changed, 57 insertions(+), 8 deletions(-)
>>>>
>>>> diff --git a/gcc/doc/gcov.texi b/gcc/doc/gcov.texi
>>>> index d39cce3a683..6739ebb3643 100644
>>>> --- a/gcc/doc/gcov.texi
>>>> +++ b/gcc/doc/gcov.texi
>>>> @@ -195,7 +195,7 @@ Structure of the JSON is following:
>>>>  @{
>>>>    "current_working_directory": "foo/bar",
>>>>    "data_file": "a.out",
>>>> -  "format_version": "1",
>>>> +  "format_version": "2",
>>>>    "gcc_version": "11.1.1 20210510"
>>>>    "files": ["$file"]
>>>>  @}
>>>> @@ -214,6 +214,12 @@ a compilation unit was compiled
>>>>  @item
>>>>  @var{format_version}: semantic version of the format
>>>>
>>>> +Changes in version @emph{2}:
>>>> +@itemize @bullet
>>>> +@item
>>>> +@var{calls}: information about function calls is added
>>>> +@end itemize
>>>> +
>>>>  @item
>>>>  @var{gcc_version}: version of the GCC compiler
>>>>  @end itemize
>>>> @@ -292,6 +298,7 @@ Each @var{line} has the following form:
>>>>  @smallexample
>>>>  @{
>>>>    "branches": ["$branch"],
>>>> +  "calls": ["$call"],
>>>>    "count": 2,
>>>>    "line_number": 15,
>>>>    "unexecuted_block": false,
>>>> @@ -299,7 +306,7 @@ Each @var{line} has the following form:
>>>>  @}
>>>>  @end smallexample
>>>>
>>>> -Branches are present only with @var{-b} option.
>>>> +Branches and calls are present only with @var{-b} option.
>>>>  Fields of the @var{line} element have following semantics:
>>>>
>>>>  @itemize @bullet
>>>> @@ -341,6 +348,22 @@ Fields of the @var{branch} element have following semantics:
>>>>  @var{throw}: true when the branch is an exceptional branch
>>>>  @end itemize
>>>>
>>>> +Each @var{call} has the following form:
>>>> +
>>>> +@smallexample
>>>> +@{
>>>> +  "returned": 11,
>>>> +@}
>>>> +@end smallexample
>>>> +
>>>> +Fields of the @var{call} element have following semantics:
>>>> +
>>>> +@itemize @bullet
>>>> +@item
>>>> +@var{returned}: number of times a function call returned (call count is equal
>>>> +to @var{line::count})
>>>> +@end itemize
>>>> +
>>>>  @item -H
>>>>  @itemx --human-readable
>>>>  Write counts in human readable format (like 24.6k).
>>>> diff --git a/gcc/gcov.cc b/gcc/gcov.cc
>>>> index 2ec7248cc0e..88324143640 100644
>>>> --- a/gcc/gcov.cc
>>>> +++ b/gcc/gcov.cc
>>>> @@ -1116,6 +1116,9 @@ output_intermediate_json_line (json::array *object,
>>>>    json::array *branches = new json::array ();
>>>>    lineo->set ("branches", branches);
>>>>
>>>> +  json::array *calls = new json::array ();
>>>> +  lineo->set ("calls", calls);
>>>> +
>>>>    vector<arc_info *>::const_iterator it;
>>>>    if (flag_branches)
>>>>      for (it = line->branches.begin (); it != line->branches.end ();
>>>> @@ -1130,6 +1133,13 @@ output_intermediate_json_line (json::array *object,
>>>>                          new json::literal ((*it)->fall_through));
>>>>             branches->append (branch);
>>>>           }
>>>> +       else if ((*it)->is_call_non_return)
>>>> +         {
>>>> +           json::object *call = new json::object ();
>>>> +           gcov_type returns = (*it)->src->count - (*it)->count;
>>>> +           call->set ("returned", new json::integer_number (returns));
>>>> +           calls->append (call);
>>>> +         }
>>>>        }
>>>>
>>>>    object->append (lineo);
>>>> @@ -1523,7 +1533,7 @@ generate_results (const char *file_name)
>>>>    gcov_intermediate_filename = get_gcov_intermediate_filename (file_name);
>>>>
>>>>    json::object *root = new json::object ();
>>>> -  root->set ("format_version", new json::string ("1"));
>>>> +  root->set ("format_version", new json::string ("2"));
>>>>    root->set ("gcc_version", new json::string (version_string));
>>>>
>>>>    if (bbg_cwd != NULL)
>>>> diff --git a/gcc/testsuite/g++.dg/gcov/gcov-17.C b/gcc/testsuite/g++.dg/gcov/gcov-17.C
>>>> index d11883cfd39..efe019599a5 100644
>>>> --- a/gcc/testsuite/g++.dg/gcov/gcov-17.C
>>>> +++ b/gcc/testsuite/g++.dg/gcov/gcov-17.C
>>>> @@ -15,6 +15,11 @@ private:
>>>>  template class Foo<int>;
>>>>  template class Foo<char>;
>>>>
>>>> +static void noret()
>>>> +{
>>>> +  __builtin_exit (0);
>>>> +}
>>>> +
>>>>  int
>>>>  main (void)
>>>>  {
>>>> @@ -34,6 +39,8 @@ main (void)
>>>>      __builtin_printf ("Failure\n");
>>>>    else
>>>>      __builtin_printf ("Success\n");
>>>> +
>>>> +  noret ();
>>>>    return 0;
>>>>  }
>>>>
>>>> diff --git a/gcc/testsuite/g++.dg/gcov/test-gcov-17.py b/gcc/testsuite/g++.dg/gcov/test-gcov-17.py
>>>> index ec5df3dec03..a0b8b09b85c 100644
>>>> --- a/gcc/testsuite/g++.dg/gcov/test-gcov-17.py
>>>> +++ b/gcc/testsuite/g++.dg/gcov/test-gcov-17.py
>>>> @@ -12,7 +12,7 @@ def test_basics(gcov):
>>>>      files = gcov['files']
>>>>      assert len(files) == 1
>>>>      functions = files[0]['functions']
>>>> -    assert len(functions) == 5
>>>> +    assert len(functions) == 6
>>>>
>>>>
>>>>  def test_lines(gcov):
>>>> @@ -31,7 +31,16 @@ def test_lines(gcov):
>>>>      assert line9[1]['count'] == 2
>>>>      assert line9[0]['unexecuted_block']
>>>>      assert not line9[1]['unexecuted_block']
>>>> -    assert linesdict[31][0]['unexecuted_block']
>>>> -    assert linesdict[34][0]['unexecuted_block']
>>>> -    assert not linesdict[37][0]['unexecuted_block']
>>>> +    assert linesdict[36][0]['unexecuted_block']
>>>> +    assert linesdict[39][0]['unexecuted_block']
>>>> +    assert not linesdict[41][0]['unexecuted_block']
>>>>      assert 32 not in linesdict
>>>> +    print(lines)
>>>> +
>>>> +    line41 = linesdict[41][0]
>>>> +    assert line41['count'] == 1
>>>> +    assert line41['calls'] == [{'returned': 1}]
>>>> +
>>>> +    line43 = linesdict[43][0]
>>>> +    assert line43['count'] == 1
>>>> +    assert line43['calls'] == [{'returned': 0}]
>>>> diff --git a/gcc/testsuite/lib/gcov.exp b/gcc/testsuite/lib/gcov.exp
>>>> index 80e74aeb220..e5e94fa5a76 100644
>>>> --- a/gcc/testsuite/lib/gcov.exp
>>>> +++ b/gcc/testsuite/lib/gcov.exp
>>>> @@ -274,7 +274,7 @@ proc run-gcov-pytest { args } {
>>>>
>>>>      verbose "Running $GCOV $testcase in $srcdir/$subdir" 2
>>>>      set testcase [remote_download host $testcase]
>>>> -    set result [remote_exec host $GCOV "$testcase -i"]
>>>> +    set result [remote_exec host $GCOV "$testcase -i -abc"]
>>>>
>>>>      set pytest_script [lindex $args 1]
>>>>      if { ![check_effective_target_pytest3] } {
>>>> --
>>>> 2.40.0
>>>>
>>

[-- Attachment #2: 0001-gcov-add-info-about-calls-to-JSON-output-format.patch --]
[-- Type: text/x-patch, Size: 8732 bytes --]

From ce372c22bd68ad6bb398d12fb52055045c8e94da Mon Sep 17 00:00:00 2001
From: Martin Liska <mliska@suse.cz>
Date: Thu, 6 Apr 2023 11:54:51 +0200
Subject: [PATCH] gcov: add info about "calls" to JSON output format

gcc/ChangeLog:

	* doc/gcov.texi: Document the new "calls" field and document
	the API bump. Mention also "block_ids" for lines.
	* gcov.cc (output_intermediate_json_line): Output info about
	calls and extend branches as well.
	(generate_results): Bump version to 2.
	(output_line_details): Use block ID instead of a non-sensual
	index.

gcc/testsuite/ChangeLog:

	* g++.dg/gcov/gcov-17.C: Add call to a noreturn function.
	* g++.dg/gcov/test-gcov-17.py: Cover new format.
	* lib/gcov.exp: Add options for gcov that emit the extra info.
---
 gcc/doc/gcov.texi                         | 47 ++++++++++++++++++++++-
 gcc/gcov.cc                               | 31 ++++++++++++---
 gcc/testsuite/g++.dg/gcov/gcov-17.C       |  7 ++++
 gcc/testsuite/g++.dg/gcov/test-gcov-17.py | 20 ++++++++--
 gcc/testsuite/lib/gcov.exp                |  2 +-
 5 files changed, 95 insertions(+), 12 deletions(-)

diff --git a/gcc/doc/gcov.texi b/gcc/doc/gcov.texi
index d39cce3a683..3019efc4674 100644
--- a/gcc/doc/gcov.texi
+++ b/gcc/doc/gcov.texi
@@ -195,7 +195,7 @@ Structure of the JSON is following:
 @{
   "current_working_directory": "foo/bar",
   "data_file": "a.out",
-  "format_version": "1",
+  "format_version": "2",
   "gcc_version": "11.1.1 20210510"
   "files": ["$file"]
 @}
@@ -214,6 +214,12 @@ a compilation unit was compiled
 @item
 @var{format_version}: semantic version of the format
 
+Changes in version @emph{2}:
+@itemize @bullet
+@item
+@var{calls}: information about function calls is added
+@end itemize
+
 @item
 @var{gcc_version}: version of the GCC compiler
 @end itemize
@@ -291,7 +297,9 @@ Each @var{line} has the following form:
 
 @smallexample
 @{
+  "block_ids": ["$block_id"],
   "branches": ["$branch"],
+  "calls": ["$call"],
   "count": 2,
   "line_number": 15,
   "unexecuted_block": false,
@@ -299,10 +307,13 @@ Each @var{line} has the following form:
 @}
 @end smallexample
 
-Branches are present only with @var{-b} option.
+Branches and calls are present only with @var{-b} option.
 Fields of the @var{line} element have following semantics:
 
 @itemize @bullet
+@item
+@var{block_ids}: IDs of basic blocks that belong to the line
+
 @item
 @var{count}: number of executions of the line
 
@@ -323,7 +334,9 @@ Each @var{branch} has the following form:
 @smallexample
 @{
   "count": 11,
+  "destination_block_id": 17,
   "fallthrough": true,
+  "source_block_id": 13,
   "throw": false
 @}
 @end smallexample
@@ -339,6 +352,36 @@ Fields of the @var{branch} element have following semantics:
 
 @item
 @var{throw}: true when the branch is an exceptional branch
+
+@item
+@var{isource_block_id}: ID of the basic block where this branch happens
+
+@item
+@var{destination_block_id}: ID of the basic block this branch jumps to
+@end itemize
+
+Each @var{call} has the following form:
+
+@smallexample
+@{
+  "destination_block_id": 1,
+  "returned": 11,
+  "source_block_id": 13
+@}
+@end smallexample
+
+Fields of the @var{call} element have following semantics:
+
+@itemize @bullet
+@item
+@var{returned}: number of times a function call returned (call count is equal
+to @var{line::count})
+
+@item
+@var{isource_block_id}: ID of the basic block where this call happens
+
+@item
+@var{destination_block_id}: ID of the basic block this calls continues after return
 @end itemize
 
 @item -H
diff --git a/gcc/gcov.cc b/gcc/gcov.cc
index 2ec7248cc0e..d96b4f77e3b 100644
--- a/gcc/gcov.cc
+++ b/gcc/gcov.cc
@@ -1113,9 +1113,17 @@ output_intermediate_json_line (json::array *object,
   lineo->set ("unexecuted_block",
 	      new json::literal (line->has_unexecuted_block));
 
+  json::array *bb_ids = new json::array ();
+  for (const block_info *block : line->blocks)
+    bb_ids->append (new json::integer_number (block->id));
+  lineo->set ("block_ids", bb_ids);
+
   json::array *branches = new json::array ();
   lineo->set ("branches", branches);
 
+  json::array *calls = new json::array ();
+  lineo->set ("calls", calls);
+
   vector<arc_info *>::const_iterator it;
   if (flag_branches)
     for (it = line->branches.begin (); it != line->branches.end ();
@@ -1128,8 +1136,23 @@ output_intermediate_json_line (json::array *object,
 	    branch->set ("throw", new json::literal ((*it)->is_throw));
 	    branch->set ("fallthrough",
 			 new json::literal ((*it)->fall_through));
+	    branch->set ("source_block_id",
+			 new json::integer_number ((*it)->src->id));
+	    branch->set ("destination_block_id",
+			 new json::integer_number ((*it)->dst->id));
 	    branches->append (branch);
 	  }
+	else if ((*it)->is_call_non_return)
+	  {
+	    json::object *call = new json::object ();
+	    gcov_type returns = (*it)->src->count - (*it)->count;
+	    call->set ("source_block_id",
+		       new json::integer_number ((*it)->src->id));
+	    call->set ("destination_block_id",
+		       new json::integer_number ((*it)->dst->id));
+	    call->set ("returned", new json::integer_number (returns));
+	    calls->append (call);
+	  }
       }
 
   object->append (lineo);
@@ -1523,7 +1546,7 @@ generate_results (const char *file_name)
   gcov_intermediate_filename = get_gcov_intermediate_filename (file_name);
 
   json::object *root = new json::object ();
-  root->set ("format_version", new json::string ("1"));
+  root->set ("format_version", new json::string ("2"));
   root->set ("gcc_version", new json::string (version_string));
 
   if (bbg_cwd != NULL)
@@ -3060,9 +3083,7 @@ output_line_details (FILE *f, const line_info *line, unsigned line_num)
   if (flag_all_blocks)
     {
       arc_info *arc;
-      int ix, jx;
-
-      ix = jx = 0;
+      int jx = 0;
       for (vector<block_info *>::const_iterator it = line->blocks.begin ();
 	   it != line->blocks.end (); it++)
 	{
@@ -3072,7 +3093,7 @@ output_line_details (FILE *f, const line_info *line, unsigned line_num)
 				     (*it)->exceptional, false,
 				     (*it)->count, line_num,
 				     "%%%%%", "$$$$$", 0);
-	      fprintf (f, "-block %2d", ix++);
+	      fprintf (f, "-block %d", (*it)->id);
 	      if (flag_verbose)
 		fprintf (f, " (BB %u)", (*it)->id);
 	      fprintf (f, "\n");
diff --git a/gcc/testsuite/g++.dg/gcov/gcov-17.C b/gcc/testsuite/g++.dg/gcov/gcov-17.C
index d11883cfd39..efe019599a5 100644
--- a/gcc/testsuite/g++.dg/gcov/gcov-17.C
+++ b/gcc/testsuite/g++.dg/gcov/gcov-17.C
@@ -15,6 +15,11 @@ private:
 template class Foo<int>;
 template class Foo<char>;
 
+static void noret()
+{
+  __builtin_exit (0);
+}
+
 int
 main (void)
 {
@@ -34,6 +39,8 @@ main (void)
     __builtin_printf ("Failure\n");
   else
     __builtin_printf ("Success\n");
+
+  noret ();
   return 0;
 }
 
diff --git a/gcc/testsuite/g++.dg/gcov/test-gcov-17.py b/gcc/testsuite/g++.dg/gcov/test-gcov-17.py
index ec5df3dec03..ad065b0f443 100644
--- a/gcc/testsuite/g++.dg/gcov/test-gcov-17.py
+++ b/gcc/testsuite/g++.dg/gcov/test-gcov-17.py
@@ -12,7 +12,7 @@ def test_basics(gcov):
     files = gcov['files']
     assert len(files) == 1
     functions = files[0]['functions']
-    assert len(functions) == 5
+    assert len(functions) == 6
 
 
 def test_lines(gcov):
@@ -31,7 +31,19 @@ def test_lines(gcov):
     assert line9[1]['count'] == 2
     assert line9[0]['unexecuted_block']
     assert not line9[1]['unexecuted_block']
-    assert linesdict[31][0]['unexecuted_block']
-    assert linesdict[34][0]['unexecuted_block']
-    assert not linesdict[37][0]['unexecuted_block']
+    assert linesdict[36][0]['unexecuted_block']
+    assert linesdict[39][0]['unexecuted_block']
+    assert not linesdict[41][0]['unexecuted_block']
     assert 32 not in linesdict
+    print(lines)
+
+    line41 = linesdict[41][0]
+    assert line41['count'] == 1
+    assert line41['calls'][0]['returned'] == 1
+    assert line41['calls'][0]['source_block_id'] == 13
+    assert line41['calls'][0]['destination_block_id'] == 1
+    assert len(line41['block_ids']) > 0
+
+    line43 = linesdict[43][0]
+    assert line43['count'] == 1
+    assert line43['calls'][0]['returned'] == 0
diff --git a/gcc/testsuite/lib/gcov.exp b/gcc/testsuite/lib/gcov.exp
index 80e74aeb220..e5e94fa5a76 100644
--- a/gcc/testsuite/lib/gcov.exp
+++ b/gcc/testsuite/lib/gcov.exp
@@ -274,7 +274,7 @@ proc run-gcov-pytest { args } {
 
     verbose "Running $GCOV $testcase in $srcdir/$subdir" 2
     set testcase [remote_download host $testcase]
-    set result [remote_exec host $GCOV "$testcase -i"]
+    set result [remote_exec host $GCOV "$testcase -i -abc"]
 
     set pytest_script [lindex $args 1]
     if { ![check_effective_target_pytest3] } {
-- 
2.40.0


  reply	other threads:[~2023-04-25 11:05 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-04-06 13:58 Martin Liška
2023-04-11  9:23 ` Richard Biener
2023-04-12  7:20   ` Martin Liška
2023-04-14 12:34     ` Jan Hubicka
2023-04-25 11:05       ` Martin Liška [this message]
2023-05-04  8:32         ` Martin Liška

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=f055fae0-c048-307d-d22b-13f56be3834e@suse.cz \
    --to=mliska@suse.cz \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=hubicka@ucw.cz \
    --cc=richard.guenther@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).