Use 'location_hash' for 'seen_locations' in 'gcc/profile.c:branch_prob' (was: [PATCH] Fix GCOV CFG related issues)
Commit Message
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
Comments
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
>
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(-)
@@ -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