* [PATCH] Fix GCOV CFG related issues.
@ 2018-07-25 13:40 Martin Liška
2021-11-09 14:29 ` Use 'location_hash' for 'seen_locations' in 'gcc/profile.c:branch_prob' (was: [PATCH] Fix GCOV CFG related issues) Thomas Schwinge
0 siblings, 1 reply; 3+ messages in thread
From: Martin Liška @ 2018-07-25 13:40 UTC (permalink / raw)
Cc: gcc-patches, Nathan Sidwell
[-- Attachment #1: Type: text/plain, Size: 1623 bytes --]
Hi.
It fixes couple of very similar issues. Currently we handle GOTO
expression, but it's not easy to find these in GIMPLE middle-end.
The patch fixes that.
Patch can bootstrap on ppc64le-redhat-linux and survives regression tests.
Will install in couple of days if no objection.
Martin
gcc/ChangeLog:
2018-07-25 Martin Liska <mliska@suse.cz>
PR gcov-profile/83813
PR gcov-profile/84758
PR gcov-profile/85217
PR gcov-profile/85332
* profile.c (branch_prob): Do not record GOTO expressions
for GIMPLE statements which locations are already streamed.
gcc/testsuite/ChangeLog:
2018-07-25 Martin Liska <mliska@suse.cz>
PR gcov-profile/83813
PR gcov-profile/84758
PR gcov-profile/85217
PR gcov-profile/85332
* gcc.misc-tests/gcov-pr83813.c: New test.
* gcc.misc-tests/gcov-pr84758.c: New test.
* gcc.misc-tests/gcov-pr85217.c: New test.
* gcc.misc-tests/gcov-pr85332.c: New test.
---
gcc/profile.c | 29 ++++++++++++++-------
gcc/testsuite/gcc.misc-tests/gcov-pr83813.c | 23 ++++++++++++++++
gcc/testsuite/gcc.misc-tests/gcov-pr84758.c | 28 ++++++++++++++++++++
gcc/testsuite/gcc.misc-tests/gcov-pr85217.c | 20 ++++++++++++++
gcc/testsuite/gcc.misc-tests/gcov-pr85332.c | 26 ++++++++++++++++++
5 files changed, 117 insertions(+), 9 deletions(-)
create mode 100644 gcc/testsuite/gcc.misc-tests/gcov-pr83813.c
create mode 100644 gcc/testsuite/gcc.misc-tests/gcov-pr84758.c
create mode 100644 gcc/testsuite/gcc.misc-tests/gcov-pr85217.c
create mode 100644 gcc/testsuite/gcc.misc-tests/gcov-pr85332.c
[-- Attachment #2: 0001-Fix-GCOV-CFG-related-issues.patch --]
[-- Type: text/x-patch, Size: 4837 bytes --]
diff --git a/gcc/profile.c b/gcc/profile.c
index 0cd0270b4fb..00f37b657a4 100644
--- a/gcc/profile.c
+++ b/gcc/profile.c
@@ -1256,6 +1256,8 @@ branch_prob (void)
/* Initialize the output. */
output_location (NULL, 0, NULL, NULL);
+ hash_set<int_hash <location_t, 0, 2> > seen_locations;
+
FOR_EACH_BB_FN (bb, cfun)
{
gimple_stmt_iterator gsi;
@@ -1263,8 +1265,9 @@ branch_prob (void)
if (bb == ENTRY_BLOCK_PTR_FOR_FN (cfun)->next_bb)
{
- expanded_location curr_location =
- expand_location (DECL_SOURCE_LOCATION (current_function_decl));
+ location_t loc = DECL_SOURCE_LOCATION (current_function_decl);
+ seen_locations.add (loc);
+ expanded_location curr_location = expand_location (loc);
output_location (curr_location.file, curr_location.line,
&offset, bb);
}
@@ -1272,17 +1275,25 @@ branch_prob (void)
for (gsi = gsi_start_bb (bb); !gsi_end_p (gsi); gsi_next (&gsi))
{
gimple *stmt = gsi_stmt (gsi);
- if (!RESERVED_LOCATION_P (gimple_location (stmt)))
- output_location (gimple_filename (stmt), gimple_lineno (stmt),
- &offset, bb);
+ location_t loc = gimple_location (stmt);
+ if (!RESERVED_LOCATION_P (loc))
+ {
+ seen_locations.add (loc);
+ output_location (gimple_filename (stmt), gimple_lineno (stmt),
+ &offset, bb);
+ }
}
- /* Notice GOTO expressions eliminated while constructing the CFG. */
+ /* Notice GOTO expressions eliminated while constructing the CFG.
+ It's hard to distinguish such expression, but goto_locus should
+ not be any of already seen location. */
+ location_t loc;
if (single_succ_p (bb)
- && !RESERVED_LOCATION_P (single_succ_edge (bb)->goto_locus))
+ && (loc = single_succ_edge (bb)->goto_locus)
+ && !RESERVED_LOCATION_P (loc)
+ && !seen_locations.contains (loc))
{
- expanded_location curr_location
- = expand_location (single_succ_edge (bb)->goto_locus);
+ expanded_location curr_location = expand_location (loc);
output_location (curr_location.file, curr_location.line,
&offset, bb);
}
diff --git a/gcc/testsuite/gcc.misc-tests/gcov-pr83813.c b/gcc/testsuite/gcc.misc-tests/gcov-pr83813.c
new file mode 100644
index 00000000000..ac935b969f8
--- /dev/null
+++ b/gcc/testsuite/gcc.misc-tests/gcov-pr83813.c
@@ -0,0 +1,23 @@
+/* { dg-options "-fprofile-arcs -ftest-coverage" } */
+/* { dg-do run { target native } } */
+
+union U
+{
+ int f0;
+ unsigned char f1;
+};
+
+int main()
+{
+ int i = 0;
+ union U u = {0}; /* count(1) */
+ for (u.f1 = 0; u.f1 != -2; ++u.f1) {
+ i ^= u.f1; /* count(1) */
+ if (i < 1) /* count(1) */
+ return 0; /* count(1) */
+ }
+
+ return 1;
+}
+
+/* { dg-final { run-gcov gcov-pr83813.c } } */
diff --git a/gcc/testsuite/gcc.misc-tests/gcov-pr84758.c b/gcc/testsuite/gcc.misc-tests/gcov-pr84758.c
new file mode 100644
index 00000000000..2ae6900375f
--- /dev/null
+++ b/gcc/testsuite/gcc.misc-tests/gcov-pr84758.c
@@ -0,0 +1,28 @@
+/* { dg-options "-fprofile-arcs -ftest-coverage" } */
+/* { dg-do run { target native } } */
+
+int x, y;
+
+static void
+foo (int a, int b)
+{
+ {
+ if (a == 1 || a == 2) /* count(1) */
+ {
+ x = 4; /* count(1) */
+ if (b == 3) /* count(1) */
+ x = 6; /* count(1) */
+ }
+ else
+ x = 15; /* count(#####) */
+ }
+}
+
+int
+main (void)
+{
+ foo (2, 3);
+ return 0;
+}
+
+/* { dg-final { run-gcov gcov-pr84758.c } } */
diff --git a/gcc/testsuite/gcc.misc-tests/gcov-pr85217.c b/gcc/testsuite/gcc.misc-tests/gcov-pr85217.c
new file mode 100644
index 00000000000..86a3c4b5a12
--- /dev/null
+++ b/gcc/testsuite/gcc.misc-tests/gcov-pr85217.c
@@ -0,0 +1,20 @@
+/* { dg-options "-fprofile-arcs -ftest-coverage" } */
+/* { dg-do run { target native } } */
+
+int a=0;
+
+int main() {
+ for (;; a++) {
+ int c[1];
+ if (a) {
+ break;
+ a;
+ continue; /* count(1) */
+ }
+ continue; /* count(1) */
+ }
+
+ return 0;
+}
+
+/* { dg-final { run-gcov gcov-pr85217.c } } */
diff --git a/gcc/testsuite/gcc.misc-tests/gcov-pr85332.c b/gcc/testsuite/gcc.misc-tests/gcov-pr85332.c
new file mode 100644
index 00000000000..73e50b19fc7
--- /dev/null
+++ b/gcc/testsuite/gcc.misc-tests/gcov-pr85332.c
@@ -0,0 +1,26 @@
+/* { dg-options "-fprofile-arcs -ftest-coverage" } */
+/* { dg-do run { target native } } */
+
+int doit(int sel, int n, void *p)
+{
+ int * const p0 = p;
+
+ switch (sel)
+ {
+ case 0: /* count(3) */
+ do {*p0 += *p0;} while (--n); /* count(3) */
+ return *p0 == 0; /* count(1) */
+
+ default:
+ __builtin_abort ();
+ }
+}
+
+int main()
+{
+ int v0;
+ v0 = 1; doit(0, 3, &v0);
+ __builtin_exit (0);
+}
+
+/* { dg-final { run-gcov gcov-pr85332.c } } */
^ permalink raw reply [flat|nested] 3+ messages in thread
* Use 'location_hash' for 'seen_locations' in 'gcc/profile.c:branch_prob' (was: [PATCH] Fix GCOV CFG related issues)
2018-07-25 13:40 [PATCH] Fix GCOV CFG related issues Martin Liška
@ 2021-11-09 14:29 ` Thomas Schwinge
2021-11-09 14:38 ` Martin Liška
0 siblings, 1 reply; 3+ messages in thread
From: Thomas Schwinge @ 2021-11-09 14:29 UTC (permalink / raw)
To: Martin Liška, gcc-patches; +Cc: Nathan Sidwell
[-- Attachment #1: Type: text/plain, Size: 3140 bytes --]
Hi!
On 2018-07-25T15:40:24+0200, Martin Liška <mliska@suse.cz> wrote:
> --- a/gcc/profile.c
> +++ b/gcc/profile.c
> @@ -1256,6 +1256,8 @@ branch_prob (void)
> /* Initialize the output. */
> output_location (NULL, 0, NULL, NULL);
>
> + hash_set<int_hash <location_t, 0, 2> > seen_locations;
> +
> FOR_EACH_BB_FN (bb, cfun)
> {
> gimple_stmt_iterator gsi;
Given my recent commit 088199e5d0fc0d54f48af0783a2630a773bbb387
"Generalize 'gcc/input.h:struct location_hash'", OK to push the attached
"Use 'location_hash' for 'seen_locations' in 'gcc/profile.c:branch_prob'"?
Grüße
Thomas
> @@ -1263,8 +1265,9 @@ branch_prob (void)
>
> if (bb == ENTRY_BLOCK_PTR_FOR_FN (cfun)->next_bb)
> {
> - expanded_location curr_location =
> - expand_location (DECL_SOURCE_LOCATION (current_function_decl));
> + location_t loc = DECL_SOURCE_LOCATION (current_function_decl);
> + seen_locations.add (loc);
> + expanded_location curr_location = expand_location (loc);
> output_location (curr_location.file, curr_location.line,
> &offset, bb);
> }
> @@ -1272,17 +1275,25 @@ branch_prob (void)
> for (gsi = gsi_start_bb (bb); !gsi_end_p (gsi); gsi_next (&gsi))
> {
> gimple *stmt = gsi_stmt (gsi);
> - if (!RESERVED_LOCATION_P (gimple_location (stmt)))
> - output_location (gimple_filename (stmt), gimple_lineno (stmt),
> - &offset, bb);
> + location_t loc = gimple_location (stmt);
> + if (!RESERVED_LOCATION_P (loc))
> + {
> + seen_locations.add (loc);
> + output_location (gimple_filename (stmt), gimple_lineno (stmt),
> + &offset, bb);
> + }
> }
>
> - /* Notice GOTO expressions eliminated while constructing the CFG. */
> + /* Notice GOTO expressions eliminated while constructing the CFG.
> + It's hard to distinguish such expression, but goto_locus should
> + not be any of already seen location. */
> + location_t loc;
> if (single_succ_p (bb)
> - && !RESERVED_LOCATION_P (single_succ_edge (bb)->goto_locus))
> + && (loc = single_succ_edge (bb)->goto_locus)
> + && !RESERVED_LOCATION_P (loc)
> + && !seen_locations.contains (loc))
> {
> - expanded_location curr_location
> - = expand_location (single_succ_edge (bb)->goto_locus);
> + expanded_location curr_location = expand_location (loc);
> output_location (curr_location.file, curr_location.line,
> &offset, bb);
> }
-----------------
Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht München, HRB 106955
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Use-location_hash-for-seen_locations-in-gcc-profile..patch --]
[-- Type: text/x-diff, Size: 2566 bytes --]
From 5d78a424466f3fc89f430c8b8282ce5820dffbe3 Mon Sep 17 00:00:00 2001
From: Thomas Schwinge <thomas@codesourcery.com>
Date: Tue, 31 Aug 2021 23:34:23 +0200
Subject: [PATCH] Use 'location_hash' for 'seen_locations' in
'gcc/profile.c:branch_prob'
Follow-up to commit 102fcf94e625a2016a65829c73a42bd6c2420376
"Fix GCOV CFG related issues": considering the current
'int_hash <location_t, 0, 2>', per 'libcpp/include/line-map.h':
Actual | Value | Meaning
-----------+-------------------------------+-------------------------------
0x00000000 | UNKNOWN_LOCATION (gcc/input.h)| Unknown/invalid location.
-----------+-------------------------------+-------------------------------
0x00000001 | BUILTINS_LOCATION | The location for declarations
| (gcc/input.h) | in "<built-in>"
-----------+-------------------------------+-------------------------------
0x00000002 | RESERVED_LOCATION_COUNT | The first location to be
| (also | handed out, and the
| ordmap[0]->start_location) | first line in ordmap 0
... this currently uses value '0' ('UNKNOWN_LOCATION') as spare values for
'Empty', and value '2' ('RESERVED_LOCATION_COUNT') as spare values for
'Deleted', which is questionable?
What actually does get put into 'seen_locations' is (mostly...)
restricted/gated by '!RESERVED_LOCATION_P' (which is true unless
'UNKNOWN_LOCATION' or 'BUILTINS_LOCATION'), thus we may simply use
'location_hash'.
gcc/
* profile.c (branch_prob): Use 'location_hash' for
'seen_locations'.
---
gcc/profile.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/gcc/profile.c b/gcc/profile.c
index c33c833167f..d07002d265e 100644
--- a/gcc/profile.c
+++ b/gcc/profile.c
@@ -1375,7 +1375,7 @@ branch_prob (bool thunk)
/* Initialize the output. */
output_location (&streamed_locations, NULL, 0, NULL, NULL);
- hash_set<int_hash <location_t, 0, 2> > seen_locations;
+ hash_set<location_hash> seen_locations;
FOR_EACH_BB_FN (bb, cfun)
{
@@ -1385,6 +1385,7 @@ branch_prob (bool thunk)
if (bb == ENTRY_BLOCK_PTR_FOR_FN (cfun)->next_bb)
{
location_t loc = DECL_SOURCE_LOCATION (current_function_decl);
+ gcc_checking_assert (!RESERVED_LOCATION_P (loc));
seen_locations.add (loc);
expanded_location curr_location = expand_location (loc);
output_location (&streamed_locations, curr_location.file,
--
2.33.0
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: Use 'location_hash' for 'seen_locations' in 'gcc/profile.c:branch_prob' (was: [PATCH] Fix GCOV CFG related issues)
2021-11-09 14:29 ` Use 'location_hash' for 'seen_locations' in 'gcc/profile.c:branch_prob' (was: [PATCH] Fix GCOV CFG related issues) Thomas Schwinge
@ 2021-11-09 14:38 ` Martin Liška
0 siblings, 0 replies; 3+ messages in thread
From: Martin Liška @ 2021-11-09 14:38 UTC (permalink / raw)
To: Thomas Schwinge, gcc-patches; +Cc: Nathan Sidwell
On 11/9/21 15:29, Thomas Schwinge wrote:
> Hi!
>
> On 2018-07-25T15:40:24+0200, Martin Liška <mliska@suse.cz> wrote:
>> --- a/gcc/profile.c
>> +++ b/gcc/profile.c
>> @@ -1256,6 +1256,8 @@ branch_prob (void)
>> /* Initialize the output. */
>> output_location (NULL, 0, NULL, NULL);
>>
>> + hash_set<int_hash <location_t, 0, 2> > seen_locations;
>> +
>> FOR_EACH_BB_FN (bb, cfun)
>> {
>> gimple_stmt_iterator gsi;
>
> Given my recent commit 088199e5d0fc0d54f48af0783a2630a773bbb387
> "Generalize 'gcc/input.h:struct location_hash'", OK to push the attached
> "Use 'location_hash' for 'seen_locations' in 'gcc/profile.c:branch_prob'"?
Yes, thanks.
Martin
>
>
> Grüße
> Thomas
>
>
>> @@ -1263,8 +1265,9 @@ branch_prob (void)
>>
>> if (bb == ENTRY_BLOCK_PTR_FOR_FN (cfun)->next_bb)
>> {
>> - expanded_location curr_location =
>> - expand_location (DECL_SOURCE_LOCATION (current_function_decl));
>> + location_t loc = DECL_SOURCE_LOCATION (current_function_decl);
>> + seen_locations.add (loc);
>> + expanded_location curr_location = expand_location (loc);
>> output_location (curr_location.file, curr_location.line,
>> &offset, bb);
>> }
>> @@ -1272,17 +1275,25 @@ branch_prob (void)
>> for (gsi = gsi_start_bb (bb); !gsi_end_p (gsi); gsi_next (&gsi))
>> {
>> gimple *stmt = gsi_stmt (gsi);
>> - if (!RESERVED_LOCATION_P (gimple_location (stmt)))
>> - output_location (gimple_filename (stmt), gimple_lineno (stmt),
>> - &offset, bb);
>> + location_t loc = gimple_location (stmt);
>> + if (!RESERVED_LOCATION_P (loc))
>> + {
>> + seen_locations.add (loc);
>> + output_location (gimple_filename (stmt), gimple_lineno (stmt),
>> + &offset, bb);
>> + }
>> }
>>
>> - /* Notice GOTO expressions eliminated while constructing the CFG. */
>> + /* Notice GOTO expressions eliminated while constructing the CFG.
>> + It's hard to distinguish such expression, but goto_locus should
>> + not be any of already seen location. */
>> + location_t loc;
>> if (single_succ_p (bb)
>> - && !RESERVED_LOCATION_P (single_succ_edge (bb)->goto_locus))
>> + && (loc = single_succ_edge (bb)->goto_locus)
>> + && !RESERVED_LOCATION_P (loc)
>> + && !seen_locations.contains (loc))
>> {
>> - expanded_location curr_location
>> - = expand_location (single_succ_edge (bb)->goto_locus);
>> + expanded_location curr_location = expand_location (loc);
>> output_location (curr_location.file, curr_location.line,
>> &offset, bb);
>> }
>
>
> -----------------
> Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht München, HRB 106955
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2021-11-09 14:38 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-07-25 13:40 [PATCH] Fix GCOV CFG related issues Martin Liška
2021-11-09 14:29 ` Use 'location_hash' for 'seen_locations' in 'gcc/profile.c:branch_prob' (was: [PATCH] Fix GCOV CFG related issues) Thomas Schwinge
2021-11-09 14:38 ` Martin Liška
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).