Message ID | 87fss5xv08.fsf@euler.schwinge.homeip.net |
---|---|
State | New |
Headers | show |
Series | Use 'location_hash' for 'seen_locations' in 'gcc/profile.c:branch_prob' (was: [PATCH] Fix GCOV CFG related issues) | expand |
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(-) 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