* [PATCH][stage1] Support profile (BB counts and edge probabilities) in GIMPLE FE. @ 2019-04-05 12:32 Martin Liška 2019-04-08 9:11 ` Richard Biener 0 siblings, 1 reply; 22+ messages in thread From: Martin Liška @ 2019-04-05 12:32 UTC (permalink / raw) To: gcc-patches; +Cc: Richard Biener [-- Attachment #1: Type: text/plain, Size: 1462 bytes --] Hi. The patch adds support for profile for GIMPLE FE. That can be useful in the future. Patch can bootstrap on x86_64-linux-gnu and survives regression tests. Ready to be installed after stage1 opens? Thanks, Martin gcc/ChangeLog: 2019-04-05 Martin Liska <mliska@suse.cz> * gimple-pretty-print.c (dump_gimple_bb_header): Dump BB count. (pp_cfg_jump): Dump edge probability. * profile-count.h (get_raw_value): New function. (from_raw_value): Likewise. gcc/c/ChangeLog: 2019-04-05 Martin Liska <mliska@suse.cz> * gimple-parser.c (struct gimple_parser): Add frequency for gimple_parser_edge. (gimple_parser::push_edge): Add new argument frequency. (c_parser_gimple_parse_bb_spec): Parse also frequency if present. (c_parser_parse_gimple_body): Set edge probability. (c_parser_gimple_compound_statement): Consume token before calling c_parser_gimple_goto_stmt. Parse BB counts. (c_parser_gimple_statement): Pass new argument. (c_parser_gimple_goto_stmt): Likewise. (c_parser_gimple_if_stmt): Likewise. gcc/testsuite/ChangeLog: 2019-04-05 Martin Liska <mliska@suse.cz> * gcc.dg/gimplefe-37.c: New test. --- gcc/c/gimple-parser.c | 116 +++++++++++++++++++++++------ gcc/gimple-pretty-print.c | 8 ++ gcc/profile-count.h | 15 ++++ gcc/testsuite/gcc.dg/gimplefe-37.c | 27 +++++++ 4 files changed, 144 insertions(+), 22 deletions(-) create mode 100644 gcc/testsuite/gcc.dg/gimplefe-37.c [-- Attachment #2: 0001-Support-profile-BB-counts-and-edge-probabilities-in-.patch --] [-- Type: text/x-patch, Size: 11367 bytes --] diff --git a/gcc/c/gimple-parser.c b/gcc/c/gimple-parser.c index fff34606dae..082f84de3ac 100644 --- a/gcc/c/gimple-parser.c +++ b/gcc/c/gimple-parser.c @@ -81,20 +81,22 @@ struct gimple_parser int src; int dest; int flags; + int frequency; }; auto_vec<gimple_parser_edge> edges; basic_block current_bb; - void push_edge (int, int, int); + void push_edge (int, int, int, int); }; void -gimple_parser::push_edge (int src, int dest, int flags) +gimple_parser::push_edge (int src, int dest, int flags, int frequency) { gimple_parser_edge e; e.src = src; e.dest = dest; e.flags = flags; + e.frequency = frequency; edges.safe_push (e); } @@ -120,10 +122,12 @@ static void c_parser_gimple_expr_list (gimple_parser &, vec<tree> *); /* See if VAL is an identifier matching __BB<num> and return <num> - in *INDEX. Return true if so. */ + in *INDEX. Return true if so and parse also FREQUENCY of + the edge. */ static bool -c_parser_gimple_parse_bb_spec (tree val, int *index) +c_parser_gimple_parse_bb_spec (tree val, gimple_parser &parser, + int *index, int *frequency) { if (strncmp (IDENTIFIER_POINTER (val), "__BB", 4) != 0) return false; @@ -131,7 +135,33 @@ c_parser_gimple_parse_bb_spec (tree val, int *index) if (!ISDIGIT (*p)) return false; *index = atoi (IDENTIFIER_POINTER (val) + 4); - return *index > 0; + + if (*index > 0) + { + *frequency = -1; + /* Parse frequency if provided. */ + if (c_parser_next_token_is (parser, CPP_OPEN_PAREN)) + { + tree f; + c_parser_consume_token (parser); + if (!c_parser_next_token_is (parser, CPP_NUMBER) + || (TREE_CODE (f = c_parser_peek_token (parser)->value) + != INTEGER_CST)) + { + c_parser_error (parser, "expected frequency value"); + return false; + } + + *frequency = TREE_INT_CST_LOW (f); + c_parser_consume_token (parser); + if (!c_parser_require (parser, CPP_CLOSE_PAREN, "expected %<)%>")) + return false; + } + + return true; + } + + return false; } /* Parse the body of a function declaration marked with "__GIMPLE". */ @@ -209,9 +239,14 @@ c_parser_parse_gimple_body (c_parser *cparser, char *gimple_pass, add_local_decl (cfun, var); /* We have a CFG. Build the edges. */ for (unsigned i = 0; i < parser.edges.length (); ++i) - make_edge (BASIC_BLOCK_FOR_FN (cfun, parser.edges[i].src), - BASIC_BLOCK_FOR_FN (cfun, parser.edges[i].dest), - parser.edges[i].flags); + { + edge e = make_edge (BASIC_BLOCK_FOR_FN (cfun, parser.edges[i].src), + BASIC_BLOCK_FOR_FN (cfun, parser.edges[i].dest), + parser.edges[i].flags); + if (parser.edges[i].frequency != -1) + e->probability + = profile_probability::from_raw_value (parser.edges[i].frequency); + } /* Add edges for case labels. */ basic_block bb; FOR_EACH_BB_FN (bb, cfun) @@ -274,6 +309,8 @@ c_parser_parse_gimple_body (c_parser *cparser, char *gimple_pass, fix_loop_structure (NULL); } + if (cfun->curr_properties & PROP_cfg) + update_max_bb_count (); dump_function (TDI_gimple, current_function_decl); } @@ -337,11 +374,9 @@ c_parser_gimple_compound_statement (gimple_parser &parser, gimple_seq *seq) c_parser_consume_token (parser); if (c_parser_next_token_is (parser, CPP_NAME)) { - c_parser_gimple_goto_stmt (parser, loc, - c_parser_peek_token - (parser)->value, - seq); + tree label = c_parser_peek_token (parser)->value; c_parser_consume_token (parser); + c_parser_gimple_goto_stmt (parser, loc, label, seq); if (! c_parser_require (parser, CPP_SEMICOLON, "expected %<;%>")) return return_p; @@ -355,7 +390,7 @@ c_parser_gimple_compound_statement (gimple_parser &parser, gimple_seq *seq) "expected %<;%>")) return return_p; if (cfun->curr_properties & PROP_cfg) - parser.push_edge (parser.current_bb->index, EXIT_BLOCK, 0); + parser.push_edge (parser.current_bb->index, EXIT_BLOCK, 0, -1); break; default: goto expr_stmt; @@ -397,6 +432,7 @@ c_parser_gimple_compound_statement (gimple_parser &parser, gimple_seq *seq) return return_p; } int is_loop_header_of = -1; + int bb_count = -1; c_parser_consume_token (parser); while (c_parser_next_token_is (parser, CPP_COMMA)) { @@ -430,6 +466,30 @@ c_parser_gimple_compound_statement (gimple_parser &parser, gimple_seq *seq) "expected %<)%>")) return return_p; } + /* count (NUM) */ + else if (!strcmp (IDENTIFIER_POINTER + (c_parser_peek_token (parser)->value), + "count")) + { + c_parser_consume_token (parser); + if (! c_parser_require (parser, CPP_OPEN_PAREN, + "expected %<(%>")) + return return_p; + tree count; + if (! c_parser_next_token_is (parser, CPP_NUMBER) + || TREE_CODE (count + = c_parser_peek_token (parser)->value) + != INTEGER_CST) + { + c_parser_error (parser, "expected count value"); + return return_p; + } + c_parser_consume_token (parser); + bb_count = TREE_INT_CST_LOW (count); + if (! c_parser_require (parser, CPP_CLOSE_PAREN, + "expected %<)%>")) + return return_p; + } else { c_parser_error (parser, "unknown block specifier"); @@ -470,7 +530,7 @@ c_parser_gimple_compound_statement (gimple_parser &parser, gimple_seq *seq) last_basic_block_for_fn (cfun) = index + 1; n_basic_blocks_for_fn (cfun)++; if (!parser.current_bb) - parser.push_edge (ENTRY_BLOCK, bb->index, EDGE_FALLTHRU); + parser.push_edge (ENTRY_BLOCK, bb->index, EDGE_FALLTHRU, -1); /* We leave the proper setting to fixup. */ struct loop *loop_father = loops_for_fn (cfun)->tree_root; @@ -499,6 +559,10 @@ c_parser_gimple_compound_statement (gimple_parser &parser, gimple_seq *seq) } bb->loop_father = loop_father; + if (bb_count != -1) + bb->count + = profile_count::from_gcov_type (bb_count).guessed_local (); + /* Stmts now go to the new block. */ parser.current_bb = bb; break; @@ -688,7 +752,9 @@ c_parser_gimple_statement (gimple_parser &parser, gimple_seq *seq) if (c_parser_next_token_is (parser, CPP_COLON)) c_parser_consume_token (parser); int src_index = -1; - if (!c_parser_gimple_parse_bb_spec (arg, &src_index)) + int frequency; + if (!c_parser_gimple_parse_bb_spec (arg, parser, &src_index, + &frequency)) c_parser_error (parser, "invalid source block specification"); vargs.safe_push (size_int (src_index)); } @@ -1757,10 +1823,12 @@ c_parser_gimple_goto_stmt (gimple_parser &parser, if (cfun->curr_properties & PROP_cfg) { int dest_index; - if (c_parser_gimple_parse_bb_spec (label, &dest_index)) + int frequency; + if (c_parser_gimple_parse_bb_spec (label, parser, &dest_index, + &frequency)) { parser.push_edge (parser.current_bb->index, dest_index, - EDGE_FALLTHRU); + EDGE_FALLTHRU, frequency); return; } } @@ -1811,10 +1879,12 @@ c_parser_gimple_if_stmt (gimple_parser &parser, gimple_seq *seq) label = c_parser_peek_token (parser)->value; c_parser_consume_token (parser); int dest_index; + int frequency; if ((cfun->curr_properties & PROP_cfg) - && c_parser_gimple_parse_bb_spec (label, &dest_index)) + && c_parser_gimple_parse_bb_spec (label, parser, &dest_index, + &frequency)) parser.push_edge (parser.current_bb->index, dest_index, - EDGE_TRUE_VALUE); + EDGE_TRUE_VALUE, frequency); else t_label = lookup_label_for_goto (loc, label); if (! c_parser_require (parser, CPP_SEMICOLON, "expected %<;%>")) @@ -1844,14 +1914,16 @@ c_parser_gimple_if_stmt (gimple_parser &parser, gimple_seq *seq) return; } label = c_parser_peek_token (parser)->value; + c_parser_consume_token (parser); int dest_index; + int frequency; if ((cfun->curr_properties & PROP_cfg) - && c_parser_gimple_parse_bb_spec (label, &dest_index)) + && c_parser_gimple_parse_bb_spec (label, parser, &dest_index, + &frequency)) parser.push_edge (parser.current_bb->index, dest_index, - EDGE_FALSE_VALUE); + EDGE_FALSE_VALUE, frequency); else f_label = lookup_label_for_goto (loc, label); - c_parser_consume_token (parser); if (! c_parser_require (parser, CPP_SEMICOLON, "expected %<;%>")) return; } diff --git a/gcc/gimple-pretty-print.c b/gcc/gimple-pretty-print.c index 69bae0d10d0..b5fca58aa07 100644 --- a/gcc/gimple-pretty-print.c +++ b/gcc/gimple-pretty-print.c @@ -2704,6 +2704,8 @@ dump_gimple_bb_header (FILE *outf, basic_block bb, int indent, fprintf (outf, "%*s__BB(%d", indent, "", bb->index); if (bb->loop_father->header == bb) fprintf (outf, ",loop_header(%d)", bb->loop_father->num); + if (bb->count.initialized_p ()) + fprintf (outf, ",count(%" PRId64 ")", bb->count.to_gcov_type ()); fprintf (outf, "):\n"); } else @@ -2760,6 +2762,12 @@ pp_cfg_jump (pretty_printer *buffer, edge e, dump_flags_t flags) { pp_string (buffer, "goto __BB"); pp_decimal_int (buffer, e->dest->index); + if (e->probability.initialized_p ()) + { + pp_string (buffer, "("); + pp_decimal_int (buffer, e->probability.get_raw_value ()); + pp_string (buffer, ")"); + } pp_semicolon (buffer); } else diff --git a/gcc/profile-count.h b/gcc/profile-count.h index d6de61f0a61..8d978e14e00 100644 --- a/gcc/profile-count.h +++ b/gcc/profile-count.h @@ -564,6 +564,21 @@ public: /* Print THIS to stderr. */ void debug () const; + /* Get m_val for GIMPLE FE. */ + uint32_t get_raw_value () const + { + return m_val; + } + + /* Build guessed profiled based on VALUE (in GIMPLE FE). */ + static profile_probability from_raw_value (uint32_t value) + { + profile_probability ret; + ret.m_val = value; + ret.m_quality = profile_guessed; + return ret; + } + /* Return true if THIS is known to differ significantly from OTHER. */ bool differs_from_p (profile_probability other) const; /* Return if difference is greater than 50%. */ diff --git a/gcc/testsuite/gcc.dg/gimplefe-37.c b/gcc/testsuite/gcc.dg/gimplefe-37.c new file mode 100644 index 00000000000..2872eb03051 --- /dev/null +++ b/gcc/testsuite/gcc.dg/gimplefe-37.c @@ -0,0 +1,27 @@ +/* { dg-do compile } */ +/* { dg-options "-O2 -fgimple -fdump-tree-optimized" } */ + +int __GIMPLE (ssa,startwith("slsr")) +main (int argc) +{ + int _1; + + __BB(2,count(3)): + if (argc_2(D) == 2) + goto __BB3(44739243); + else + goto __BB4(89478485); + + __BB(3,count(1)): + goto __BB4(134217728); + + __BB(4,count(3)): + _1 = __PHI (__BB2: 0, __BB3: 12); + return _1; +} + + +/* { dg-final { scan-tree-dump-times "<bb \[0-9\]> \\\[local count: 3" 2 "optimized" } } */ +/* { dg-final { scan-tree-dump-times "<bb \[0-9\]> \\\[local count: 2" 1 "optimized" } } */ +/* { dg-final { scan-tree-dump-times "goto <bb \[0-9\]>; \\\[33\\\.33%\\\]" 1 "optimized" } } */ +/* { dg-final { scan-tree-dump-times "goto <bb \[0-9\]>; \\\[66\\\.67%\\\]" 1 "optimized" } } */ ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH][stage1] Support profile (BB counts and edge probabilities) in GIMPLE FE. 2019-04-05 12:32 [PATCH][stage1] Support profile (BB counts and edge probabilities) in GIMPLE FE Martin Liška @ 2019-04-08 9:11 ` Richard Biener 2019-04-08 13:35 ` Martin Liška 0 siblings, 1 reply; 22+ messages in thread From: Richard Biener @ 2019-04-08 9:11 UTC (permalink / raw) To: Martin Liška, Jan Hubicka; +Cc: GCC Patches On Fri, Apr 5, 2019 at 2:32 PM Martin Liška <mliska@suse.cz> wrote: > > Hi. > > The patch adds support for profile for GIMPLE FE. That can be useful > in the future. > > Patch can bootstrap on x86_64-linux-gnu and survives regression tests. > > Ready to be installed after stage1 opens? Hmm, I guess to be useful we need the profile quality indicators as well (and think about a proper syntax here since it seems related). Btw, do BB counts and edge probabilities not have a relation? If so why do we need both? In predict.c we also look at ENTRY_BLOCK->count so we need a place to set that as well. We should probably set edge probabilities of fallthru edges properly during our "CFG build". + goto __BB3(44739243); since we eventually want to add other flags this syntactically should maybe be goto __BB3(guessed(44739243)); and similar for the __BB count case (just s/count/quality/). The entry block count could be sticked to __GIMPLE (ssa,guessed(N)) for example. There's also the exit block, not sure if we ever look at its count, so __GIMPLE (ssa,guessed(N[,M])) might be a possibility if we always have the same quality here (probably not...). Otherwise thanks for trying ;) Richard. > Thanks, > Martin > > gcc/ChangeLog: > > 2019-04-05 Martin Liska <mliska@suse.cz> > > * gimple-pretty-print.c (dump_gimple_bb_header): > Dump BB count. > (pp_cfg_jump): Dump edge probability. > * profile-count.h (get_raw_value): New function. > (from_raw_value): Likewise. > > gcc/c/ChangeLog: > > 2019-04-05 Martin Liska <mliska@suse.cz> > > * gimple-parser.c (struct gimple_parser): Add frequency > for gimple_parser_edge. > (gimple_parser::push_edge): Add new argument frequency. > (c_parser_gimple_parse_bb_spec): Parse also frequency > if present. > (c_parser_parse_gimple_body): Set edge probability. > (c_parser_gimple_compound_statement): Consume token > before calling c_parser_gimple_goto_stmt. > Parse BB counts. > (c_parser_gimple_statement): Pass new argument. > (c_parser_gimple_goto_stmt): Likewise. > (c_parser_gimple_if_stmt): Likewise. > > gcc/testsuite/ChangeLog: > > 2019-04-05 Martin Liska <mliska@suse.cz> > > * gcc.dg/gimplefe-37.c: New test. > --- > gcc/c/gimple-parser.c | 116 +++++++++++++++++++++++------ > gcc/gimple-pretty-print.c | 8 ++ > gcc/profile-count.h | 15 ++++ > gcc/testsuite/gcc.dg/gimplefe-37.c | 27 +++++++ > 4 files changed, 144 insertions(+), 22 deletions(-) > create mode 100644 gcc/testsuite/gcc.dg/gimplefe-37.c > > ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH][stage1] Support profile (BB counts and edge probabilities) in GIMPLE FE. 2019-04-08 9:11 ` Richard Biener @ 2019-04-08 13:35 ` Martin Liška 2019-04-09 13:15 ` Martin Liška 0 siblings, 1 reply; 22+ messages in thread From: Martin Liška @ 2019-04-08 13:35 UTC (permalink / raw) To: Richard Biener, Jan Hubicka; +Cc: GCC Patches On 4/8/19 11:11 AM, Richard Biener wrote: > On Fri, Apr 5, 2019 at 2:32 PM Martin LiÅ¡ka <mliska@suse.cz> wrote: >> >> Hi. >> >> The patch adds support for profile for GIMPLE FE. That can be useful >> in the future. >> >> Patch can bootstrap on x86_64-linux-gnu and survives regression tests. >> >> Ready to be installed after stage1 opens? > > Hmm, I guess to be useful we need the profile quality indicators as well > (and think about a proper syntax here since it seems related). I wanted to make it easy, buy yes. We should implement that. > > Btw, do BB counts and edge probabilities not have a relation? Yes, they should match. But you have optimizations that make some discrepancies. So that I would implement support for both of them. > If so why > do we need both? In predict.c we also look at ENTRY_BLOCK->count > so we need a place to set that as well. We should probably set > edge probabilities of fallthru edges properly during our "CFG build". > > + goto __BB3(44739243); > > since we eventually want to add other flags this syntactically > should maybe be goto __BB3(guessed(44739243)); and similar > for the __BB count case (just s/count/quality/). The syntax works for me. > > The entry block count could be sticked to __GIMPLE (ssa,guessed(N)) > for example. There's also the exit block, not sure if we ever look at > its count, so __GIMPLE (ssa,guessed(N[,M])) might be a possibility > if we always have the same quality here (probably not...). I'll check it. > > Otherwise thanks for trying ;) > > Richard. > >> Thanks, >> Martin >> >> gcc/ChangeLog: >> >> 2019-04-05 Martin Liska <mliska@suse.cz> >> >> * gimple-pretty-print.c (dump_gimple_bb_header): >> Dump BB count. >> (pp_cfg_jump): Dump edge probability. >> * profile-count.h (get_raw_value): New function. >> (from_raw_value): Likewise. >> >> gcc/c/ChangeLog: >> >> 2019-04-05 Martin Liska <mliska@suse.cz> >> >> * gimple-parser.c (struct gimple_parser): Add frequency >> for gimple_parser_edge. >> (gimple_parser::push_edge): Add new argument frequency. >> (c_parser_gimple_parse_bb_spec): Parse also frequency >> if present. >> (c_parser_parse_gimple_body): Set edge probability. >> (c_parser_gimple_compound_statement): Consume token >> before calling c_parser_gimple_goto_stmt. >> Parse BB counts. >> (c_parser_gimple_statement): Pass new argument. >> (c_parser_gimple_goto_stmt): Likewise. >> (c_parser_gimple_if_stmt): Likewise. >> >> gcc/testsuite/ChangeLog: >> >> 2019-04-05 Martin Liska <mliska@suse.cz> >> >> * gcc.dg/gimplefe-37.c: New test. >> --- >> gcc/c/gimple-parser.c | 116 +++++++++++++++++++++++------ >> gcc/gimple-pretty-print.c | 8 ++ >> gcc/profile-count.h | 15 ++++ >> gcc/testsuite/gcc.dg/gimplefe-37.c | 27 +++++++ >> 4 files changed, 144 insertions(+), 22 deletions(-) >> create mode 100644 gcc/testsuite/gcc.dg/gimplefe-37.c >> >> ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH][stage1] Support profile (BB counts and edge probabilities) in GIMPLE FE. 2019-04-08 13:35 ` Martin Liška @ 2019-04-09 13:15 ` Martin Liška 2019-04-09 14:02 ` Jan Hubicka 0 siblings, 1 reply; 22+ messages in thread From: Martin Liška @ 2019-04-09 13:15 UTC (permalink / raw) To: Richard Biener, Jan Hubicka; +Cc: GCC Patches [-- Attachment #1: Type: text/plain, Size: 3440 bytes --] On 4/8/19 3:35 PM, Martin LiÅ¡ka wrote: > On 4/8/19 11:11 AM, Richard Biener wrote: >> On Fri, Apr 5, 2019 at 2:32 PM Martin LiÅ¡ka <mliska@suse.cz> wrote: >>> >>> Hi. >>> >>> The patch adds support for profile for GIMPLE FE. That can be useful >>> in the future. >>> >>> Patch can bootstrap on x86_64-linux-gnu and survives regression tests. >>> >>> Ready to be installed after stage1 opens? >> >> Hmm, I guess to be useful we need the profile quality indicators as well >> (and think about a proper syntax here since it seems related). > > I wanted to make it easy, buy yes. We should implement that. > >> >> Btw, do BB counts and edge probabilities not have a relation? > > Yes, they should match. But you have optimizations that make some discrepancies. > So that I would implement support for both of them. > >> If so why >> do we need both? In predict.c we also look at ENTRY_BLOCK->count >> so we need a place to set that as well. We should probably set >> edge probabilities of fallthru edges properly during our "CFG build". >> >> + goto __BB3(44739243); >> >> since we eventually want to add other flags this syntactically >> should maybe be goto __BB3(guessed(44739243)); and similar >> for the __BB count case (just s/count/quality/). > > The syntax works for me. > >> >> The entry block count could be sticked to __GIMPLE (ssa,guessed(N)) >> for example. There's also the exit block, not sure if we ever look at >> its count, so __GIMPLE (ssa,guessed(N[,M])) might be a possibility >> if we always have the same quality here (probably not...). > > I'll check it. > >> >> Otherwise thanks for trying ;) >> >> Richard. >> >>> Thanks, >>> Martin >>> >>> gcc/ChangeLog: >>> >>> 2019-04-05 Martin Liska <mliska@suse.cz> >>> >>> * gimple-pretty-print.c (dump_gimple_bb_header): >>> Dump BB count. >>> (pp_cfg_jump): Dump edge probability. >>> * profile-count.h (get_raw_value): New function. >>> (from_raw_value): Likewise. >>> >>> gcc/c/ChangeLog: >>> >>> 2019-04-05 Martin Liska <mliska@suse.cz> >>> >>> * gimple-parser.c (struct gimple_parser): Add frequency >>> for gimple_parser_edge. >>> (gimple_parser::push_edge): Add new argument frequency. >>> (c_parser_gimple_parse_bb_spec): Parse also frequency >>> if present. >>> (c_parser_parse_gimple_body): Set edge probability. >>> (c_parser_gimple_compound_statement): Consume token >>> before calling c_parser_gimple_goto_stmt. >>> Parse BB counts. >>> (c_parser_gimple_statement): Pass new argument. >>> (c_parser_gimple_goto_stmt): Likewise. >>> (c_parser_gimple_if_stmt): Likewise. >>> >>> gcc/testsuite/ChangeLog: >>> >>> 2019-04-05 Martin Liska <mliska@suse.cz> >>> >>> * gcc.dg/gimplefe-37.c: New test. >>> --- >>> gcc/c/gimple-parser.c | 116 +++++++++++++++++++++++------ >>> gcc/gimple-pretty-print.c | 8 ++ >>> gcc/profile-count.h | 15 ++++ >>> gcc/testsuite/gcc.dg/gimplefe-37.c | 27 +++++++ >>> 4 files changed, 144 insertions(+), 22 deletions(-) >>> create mode 100644 gcc/testsuite/gcc.dg/gimplefe-37.c >>> >>> > Hi. There's updated version that supports profile quality for both counts and probabilities. I'm wondering whether ENTRY and EXIT BBs needs to have set probability. Apparently, I haven't seen any verifier that would complain. Martin [-- Attachment #2: 0001-Support-profile-BB-counts-and-edge-probabilities-in-.patch --] [-- Type: text/x-patch, Size: 23528 bytes --] From 8a2fed046a9a951ded01b3be1a9ce82ed5ab7496 Mon Sep 17 00:00:00 2001 From: marxin <mliska@suse.cz> Date: Thu, 4 Apr 2019 14:46:15 +0200 Subject: [PATCH] Support profile (BB counts and edge probabilities) in GIMPLE FE. gcc/ChangeLog: 2019-04-05 Martin Liska <mliska@suse.cz> * gimple-pretty-print.c (dump_gimple_bb_header): Dump BB count. (pp_cfg_jump): Dump edge probability. * profile-count.c (profile_quality_as_string): Simplify with a static array. (parse_profile_quality): New function. (profile_count::dump): Simplify with a static array. (profile_count::from_gcov_type): Add new argument. * profile-count.h (parse_profile_quality): Likewise. gcc/c/ChangeLog: 2019-04-05 Martin Liska <mliska@suse.cz> * gimple-parser.c (struct gimple_parser): Add probability. for gimple_parser_edge. (gimple_parser::push_edge): Add new argument probability. (c_parser_gimple_parse_bb_spec): Parse also probability if present. (c_parser_parse_gimple_body): Set edge probability. (c_parser_gimple_compound_statement): Consume token before calling c_parser_gimple_goto_stmt. Parse BB counts. (c_parser_gimple_statement): Pass new argument. (c_parser_gimple_goto_stmt): Likewise. (c_parser_gimple_if_stmt): Likewise. (c_parser_gimple_or_rtl_pass_list): Parse hot_bb_threshold. * c-parser.c (c_parser_declaration_or_fndef): Pass hot_bb_threshold argument. * c-tree.h (struct c_declspecs): Add hot_bb_threshold field. gcc/testsuite/ChangeLog: 2019-04-05 Martin Liska <mliska@suse.cz> * gcc.dg/gimplefe-37.c: New test. * gcc.dg/gimplefe-33.c: Likewise. --- gcc/c/c-parser.c | 3 +- gcc/c/c-tree.h | 2 + gcc/c/gimple-parser.c | 170 ++++++++++++++++++++++++----- gcc/c/gimple-parser.h | 3 +- gcc/gimple-pretty-print.c | 13 +++ gcc/profile-count.c | 88 ++++++++------- gcc/profile-count.h | 22 +++- gcc/testsuite/gcc.dg/gimplefe-37.c | 27 +++++ gcc/testsuite/gcc.dg/gimplefe-38.c | 27 +++++ 9 files changed, 286 insertions(+), 69 deletions(-) create mode 100644 gcc/testsuite/gcc.dg/gimplefe-37.c create mode 100644 gcc/testsuite/gcc.dg/gimplefe-38.c diff --git a/gcc/c/c-parser.c b/gcc/c/c-parser.c index 741d172ff30..205ec9b61ce 100644 --- a/gcc/c/c-parser.c +++ b/gcc/c/c-parser.c @@ -2347,7 +2347,8 @@ c_parser_declaration_or_fndef (c_parser *parser, bool fndef_ok, bool saved = in_late_binary_op; in_late_binary_op = true; c_parser_parse_gimple_body (parser, specs->gimple_or_rtl_pass, - specs->declspec_il); + specs->declspec_il, + specs->hot_bb_threshold); in_late_binary_op = saved; } else diff --git a/gcc/c/c-tree.h b/gcc/c/c-tree.h index 9393f6d1545..b3e024267a1 100644 --- a/gcc/c/c-tree.h +++ b/gcc/c/c-tree.h @@ -317,6 +317,8 @@ struct c_declspecs { tree attrs; /* The pass to start compiling a __GIMPLE or __RTL function with. */ char *gimple_or_rtl_pass; + /* Hotness threshold for __GIMPLE FE. */ + gcov_type hot_bb_threshold; /* The base-2 log of the greatest alignment required by an _Alignas specifier, in bytes, or -1 if no such specifiers with nonzero alignment. */ diff --git a/gcc/c/gimple-parser.c b/gcc/c/gimple-parser.c index fff34606dae..351eea30a3d 100644 --- a/gcc/c/gimple-parser.c +++ b/gcc/c/gimple-parser.c @@ -81,20 +81,23 @@ struct gimple_parser int src; int dest; int flags; + profile_probability probability; }; auto_vec<gimple_parser_edge> edges; basic_block current_bb; - void push_edge (int, int, int); + void push_edge (int, int, int, profile_probability); }; void -gimple_parser::push_edge (int src, int dest, int flags) +gimple_parser::push_edge (int src, int dest, int flags, + profile_probability prob) { gimple_parser_edge e; e.src = src; e.dest = dest; e.flags = flags; + e.probability = prob; edges.safe_push (e); } @@ -120,10 +123,12 @@ static void c_parser_gimple_expr_list (gimple_parser &, vec<tree> *); /* See if VAL is an identifier matching __BB<num> and return <num> - in *INDEX. Return true if so. */ + in *INDEX. Return true if so and parse also FREQUENCY of + the edge. */ static bool -c_parser_gimple_parse_bb_spec (tree val, int *index) +c_parser_gimple_parse_bb_spec (tree val, gimple_parser &parser, + int *index, profile_probability *probablity) { if (strncmp (IDENTIFIER_POINTER (val), "__BB", 4) != 0) return false; @@ -131,14 +136,64 @@ c_parser_gimple_parse_bb_spec (tree val, int *index) if (!ISDIGIT (*p)) return false; *index = atoi (IDENTIFIER_POINTER (val) + 4); - return *index > 0; + + if (*index > 0) + { + *probablity = profile_probability::uninitialized (); + /* Parse frequency if provided. */ + if (c_parser_next_token_is (parser, CPP_OPEN_PAREN)) + { + tree f; + c_parser_consume_token (parser); + if (!c_parser_next_token_is (parser, CPP_NAME)) + { + c_parser_error (parser, "expected frequency quality"); + return false; + } + + profile_quality quality; + const char *v + = IDENTIFIER_POINTER (c_parser_peek_token (parser)->value); + if (!parse_profile_quality (v, &quality)) + { + c_parser_error (parser, "unknown profile quality"); + return false; + } + + c_parser_consume_token (parser); + if (!c_parser_require (parser, CPP_OPEN_PAREN, "expected %<(%>")) + return false; + + if (!c_parser_next_token_is (parser, CPP_NUMBER) + || (TREE_CODE (f = c_parser_peek_token (parser)->value) + != INTEGER_CST)) + { + c_parser_error (parser, "expected frequency value"); + return false; + } + + unsigned int value = TREE_INT_CST_LOW (f); + *probablity = profile_probability (value, quality); + + c_parser_consume_token (parser); + if (!c_parser_require (parser, CPP_CLOSE_PAREN, "expected %<)%>")) + return false; + + if (!c_parser_require (parser, CPP_CLOSE_PAREN, "expected %<)%>")) + return false; + } + + return true; + } + + return false; } /* Parse the body of a function declaration marked with "__GIMPLE". */ void c_parser_parse_gimple_body (c_parser *cparser, char *gimple_pass, - enum c_declspec_il cdil) + enum c_declspec_il cdil, gcov_type hot_bb_threshold) { gimple_parser parser (cparser); gimple_seq seq = NULL; @@ -209,9 +264,12 @@ c_parser_parse_gimple_body (c_parser *cparser, char *gimple_pass, add_local_decl (cfun, var); /* We have a CFG. Build the edges. */ for (unsigned i = 0; i < parser.edges.length (); ++i) - make_edge (BASIC_BLOCK_FOR_FN (cfun, parser.edges[i].src), - BASIC_BLOCK_FOR_FN (cfun, parser.edges[i].dest), - parser.edges[i].flags); + { + edge e = make_edge (BASIC_BLOCK_FOR_FN (cfun, parser.edges[i].src), + BASIC_BLOCK_FOR_FN (cfun, parser.edges[i].dest), + parser.edges[i].flags); + e->probability = parser.edges[i].probability; + } /* Add edges for case labels. */ basic_block bb; FOR_EACH_BB_FN (bb, cfun) @@ -274,6 +332,11 @@ c_parser_parse_gimple_body (c_parser *cparser, char *gimple_pass, fix_loop_structure (NULL); } + if (cfun->curr_properties & PROP_cfg) + { + update_max_bb_count (); + set_hot_bb_threshold (hot_bb_threshold); + } dump_function (TDI_gimple, current_function_decl); } @@ -337,11 +400,9 @@ c_parser_gimple_compound_statement (gimple_parser &parser, gimple_seq *seq) c_parser_consume_token (parser); if (c_parser_next_token_is (parser, CPP_NAME)) { - c_parser_gimple_goto_stmt (parser, loc, - c_parser_peek_token - (parser)->value, - seq); + tree label = c_parser_peek_token (parser)->value; c_parser_consume_token (parser); + c_parser_gimple_goto_stmt (parser, loc, label, seq); if (! c_parser_require (parser, CPP_SEMICOLON, "expected %<;%>")) return return_p; @@ -355,7 +416,8 @@ c_parser_gimple_compound_statement (gimple_parser &parser, gimple_seq *seq) "expected %<;%>")) return return_p; if (cfun->curr_properties & PROP_cfg) - parser.push_edge (parser.current_bb->index, EXIT_BLOCK, 0); + parser.push_edge (parser.current_bb->index, EXIT_BLOCK, 0, + profile_probability::uninitialized ()); break; default: goto expr_stmt; @@ -397,6 +459,7 @@ c_parser_gimple_compound_statement (gimple_parser &parser, gimple_seq *seq) return return_p; } int is_loop_header_of = -1; + profile_count bb_count = profile_count::uninitialized (); c_parser_consume_token (parser); while (c_parser_next_token_is (parser, CPP_COMMA)) { @@ -430,10 +493,38 @@ c_parser_gimple_compound_statement (gimple_parser &parser, gimple_seq *seq) "expected %<)%>")) return return_p; } + /* Parse profile: quality(value) */ else { - c_parser_error (parser, "unknown block specifier"); - return return_p; + tree q; + profile_quality quality; + tree v = c_parser_peek_token (parser)->value; + if (!parse_profile_quality (IDENTIFIER_POINTER (v), + &quality)) + { + c_parser_error (parser, "unknown block specifier"); + return false; + } + + c_parser_consume_token (parser); + if (!c_parser_require (parser, CPP_OPEN_PAREN, + "expected %<(%>")) + return false; + + if (!c_parser_next_token_is (parser, CPP_NUMBER) + || (TREE_CODE (q = c_parser_peek_token (parser)->value) + != INTEGER_CST)) + { + c_parser_error (parser, "expected frequency value"); + return false; + } + + bb_count = profile_count::from_gcov_type (TREE_INT_CST_LOW (q), + quality); + c_parser_consume_token (parser); + if (! c_parser_require (parser, CPP_CLOSE_PAREN, + "expected %<)%>")) + return return_p; } } if (! c_parser_require (parser, CPP_CLOSE_PAREN, @@ -470,7 +561,8 @@ c_parser_gimple_compound_statement (gimple_parser &parser, gimple_seq *seq) last_basic_block_for_fn (cfun) = index + 1; n_basic_blocks_for_fn (cfun)++; if (!parser.current_bb) - parser.push_edge (ENTRY_BLOCK, bb->index, EDGE_FALLTHRU); + parser.push_edge (ENTRY_BLOCK, bb->index, EDGE_FALLTHRU, + profile_probability::uninitialized ()); /* We leave the proper setting to fixup. */ struct loop *loop_father = loops_for_fn (cfun)->tree_root; @@ -498,6 +590,7 @@ c_parser_gimple_compound_statement (gimple_parser &parser, gimple_seq *seq) loop_father = get_loop (cfun, is_loop_header_of); } bb->loop_father = loop_father; + bb->count = bb_count; /* Stmts now go to the new block. */ parser.current_bb = bb; @@ -688,7 +781,9 @@ c_parser_gimple_statement (gimple_parser &parser, gimple_seq *seq) if (c_parser_next_token_is (parser, CPP_COLON)) c_parser_consume_token (parser); int src_index = -1; - if (!c_parser_gimple_parse_bb_spec (arg, &src_index)) + profile_probability prob; + if (!c_parser_gimple_parse_bb_spec (arg, parser, &src_index, + &prob)) c_parser_error (parser, "invalid source block specification"); vargs.safe_push (size_int (src_index)); } @@ -1629,6 +1724,25 @@ c_parser_gimple_or_rtl_pass_list (c_parser *parser, c_declspecs *specs) if (! c_parser_require (parser, CPP_CLOSE_PAREN, "expected %<(%>")) return; } + else if (! strcmp (op, "hot_bb_threshold")) + { + if (! c_parser_require (parser, CPP_OPEN_PAREN, "expected %<(%>")) + return; + + tree v; + if (!c_parser_next_token_is (parser, CPP_NUMBER) + || (TREE_CODE (v = c_parser_peek_token (parser)->value) + != INTEGER_CST)) + { + c_parser_error (parser, "expected frequency value"); + return; + } + + specs->hot_bb_threshold = TREE_INT_CST_LOW (v); + c_parser_consume_token (parser); + if (! c_parser_require (parser, CPP_CLOSE_PAREN, "expected %<)%>")) + return; + } else if (specs->declspec_il != cdil_gimple) /* Allow only one IL specifier and none on RTL. */ ; @@ -1757,10 +1871,12 @@ c_parser_gimple_goto_stmt (gimple_parser &parser, if (cfun->curr_properties & PROP_cfg) { int dest_index; - if (c_parser_gimple_parse_bb_spec (label, &dest_index)) + profile_probability prob; + if (c_parser_gimple_parse_bb_spec (label, parser, &dest_index, + &prob)) { parser.push_edge (parser.current_bb->index, dest_index, - EDGE_FALLTHRU); + EDGE_FALLTHRU, prob); return; } } @@ -1811,10 +1927,12 @@ c_parser_gimple_if_stmt (gimple_parser &parser, gimple_seq *seq) label = c_parser_peek_token (parser)->value; c_parser_consume_token (parser); int dest_index; + profile_probability prob; if ((cfun->curr_properties & PROP_cfg) - && c_parser_gimple_parse_bb_spec (label, &dest_index)) + && c_parser_gimple_parse_bb_spec (label, parser, &dest_index, + &prob)) parser.push_edge (parser.current_bb->index, dest_index, - EDGE_TRUE_VALUE); + EDGE_TRUE_VALUE, prob); else t_label = lookup_label_for_goto (loc, label); if (! c_parser_require (parser, CPP_SEMICOLON, "expected %<;%>")) @@ -1844,14 +1962,16 @@ c_parser_gimple_if_stmt (gimple_parser &parser, gimple_seq *seq) return; } label = c_parser_peek_token (parser)->value; + c_parser_consume_token (parser); int dest_index; + profile_probability prob; if ((cfun->curr_properties & PROP_cfg) - && c_parser_gimple_parse_bb_spec (label, &dest_index)) + && c_parser_gimple_parse_bb_spec (label, parser, &dest_index, + &prob)) parser.push_edge (parser.current_bb->index, dest_index, - EDGE_FALSE_VALUE); + EDGE_FALSE_VALUE, prob); else f_label = lookup_label_for_goto (loc, label); - c_parser_consume_token (parser); if (! c_parser_require (parser, CPP_SEMICOLON, "expected %<;%>")) return; } diff --git a/gcc/c/gimple-parser.h b/gcc/c/gimple-parser.h index 383ad768759..22b5b624679 100644 --- a/gcc/c/gimple-parser.h +++ b/gcc/c/gimple-parser.h @@ -22,7 +22,8 @@ along with GCC; see the file COPYING3. If not see /* Gimple parsing functions. */ extern void c_parser_parse_gimple_body (c_parser *, char *, - enum c_declspec_il); + enum c_declspec_il, gcov_type + hot_bb_threshold); extern void c_parser_gimple_or_rtl_pass_list (c_parser *, c_declspecs *); #endif diff --git a/gcc/gimple-pretty-print.c b/gcc/gimple-pretty-print.c index 69bae0d10d0..7e3916bff86 100644 --- a/gcc/gimple-pretty-print.c +++ b/gcc/gimple-pretty-print.c @@ -2704,6 +2704,10 @@ dump_gimple_bb_header (FILE *outf, basic_block bb, int indent, fprintf (outf, "%*s__BB(%d", indent, "", bb->index); if (bb->loop_father->header == bb) fprintf (outf, ",loop_header(%d)", bb->loop_father->num); + if (bb->count.initialized_p ()) + fprintf (outf, ",%s(%d)", + profile_quality_as_string (bb->count.quality ()), + bb->count.value ()); fprintf (outf, "):\n"); } else @@ -2760,6 +2764,15 @@ pp_cfg_jump (pretty_printer *buffer, edge e, dump_flags_t flags) { pp_string (buffer, "goto __BB"); pp_decimal_int (buffer, e->dest->index); + if (e->probability.initialized_p ()) + { + pp_string (buffer, "("); + pp_string (buffer, + profile_quality_as_string (e->probability.quality ())); + pp_string (buffer, "("); + pp_decimal_int (buffer, e->probability.value ()); + pp_string (buffer, "))"); + } pp_semicolon (buffer); } else diff --git a/gcc/profile-count.c b/gcc/profile-count.c index 8c58f8666f0..4f0bac002d7 100644 --- a/gcc/profile-count.c +++ b/gcc/profile-count.c @@ -33,34 +33,57 @@ along with GCC; see the file COPYING3. If not see #include "wide-int.h" #include "sreal.h" +/* Names from profile_quality enum values. */ + +const char *profile_quality_names[] = +{ + "uninitialized", + "guessed_local", + "guessed_global0", + "guessed_global0adjusted", + "guessed", + "afdo", + "adjusted", + "precise" +}; + /* Get a string describing QUALITY. */ const char * profile_quality_as_string (enum profile_quality quality) { - switch (quality) - { - default: - gcc_unreachable (); - case profile_uninitialized: - return "uninitialized"; - case profile_guessed_local: - return "guessed_local"; - case profile_guessed_global0: - return "guessed_global0"; - case profile_guessed_global0adjusted: - return "guessed_global0adjusted"; - case profile_guessed: - return "guessed"; - case profile_afdo: - return "afdo"; - case profile_adjusted: - return "adjusted"; - case profile_precise: - return "precise"; - } + return profile_quality_names[quality]; +} + +/* Parse VALUE as profile quality and return true when a valid QUALITY. */ + +bool +parse_profile_quality (const char *value, profile_quality *quality) +{ + for (unsigned i = 0; i < ARRAY_SIZE (profile_quality_names); i++) + if (strcmp (profile_quality_names[i], value) == 0) + { + *quality = (profile_quality)i; + return true; + } + + return false; } +/* Display names from profile_quality enum values. */ + +const char *profile_quality_display_names[] = +{ + NULL, + "estimated locally", + "estimated locally, globally 0", + "estimated locally, globally 0 adjusted", + "adjusted", + "auto FDO", + "guessed", + "precise" +}; + /* Dump THIS to F. */ void @@ -69,23 +92,8 @@ profile_count::dump (FILE *f) const if (!initialized_p ()) fprintf (f, "uninitialized"); else - { - fprintf (f, "%" PRId64, m_val); - if (m_quality == profile_guessed_local) - fprintf (f, " (estimated locally)"); - else if (m_quality == profile_guessed_global0) - fprintf (f, " (estimated locally, globally 0)"); - else if (m_quality == profile_guessed_global0adjusted) - fprintf (f, " (estimated locally, globally 0 adjusted)"); - else if (m_quality == profile_adjusted) - fprintf (f, " (adjusted)"); - else if (m_quality == profile_afdo) - fprintf (f, " (auto FDO)"); - else if (m_quality == profile_guessed) - fprintf (f, " (guessed)"); - else if (m_quality == profile_precise) - fprintf (f, " (precise)"); - } + fprintf (f, "%" PRId64 " (%s)", m_val, + profile_quality_display_names[m_quality]); } /* Dump THIS to stderr. */ @@ -362,7 +370,7 @@ profile_count::combine_with_ipa_count (profile_count ipa) Conversions back and forth are used to read the coverage and get it into internal representation. */ profile_count -profile_count::from_gcov_type (gcov_type v) +profile_count::from_gcov_type (gcov_type v, profile_quality quality) { profile_count ret; gcc_checking_assert (v >= 0); @@ -371,7 +379,7 @@ profile_count::from_gcov_type (gcov_type v) "Capping gcov count %" PRId64 " to max_count %" PRId64 "\n", (int64_t) v, (int64_t) max_count); ret.m_val = MIN (v, (gcov_type)max_count); - ret.m_quality = profile_precise; + ret.m_quality = quality; return ret; } diff --git a/gcc/profile-count.h b/gcc/profile-count.h index d6de61f0a61..2669f0d5fff 100644 --- a/gcc/profile-count.h +++ b/gcc/profile-count.h @@ -60,6 +60,8 @@ enum profile_quality { }; extern const char *profile_quality_as_string (enum profile_quality); +extern bool parse_profile_quality (const char *value, + profile_quality *quality); /* The base value for branch probability notes and edge probabilities. */ #define REG_BR_PROB_BASE 10000 @@ -149,6 +151,13 @@ class GTY((user)) profile_probability friend class profile_count; public: + profile_probability (): m_val (uninitialized_probability), + m_quality (profile_guessed) + {} + + profile_probability (uint32_t val, profile_quality quality): + m_val (val), m_quality (quality) + {} /* Named probabilities. */ static profile_probability never () @@ -558,6 +567,12 @@ public: return initialized_p () && other.initialized_p () && m_val >= other.m_val; } + /* Get the value of the count. */ + uint32_t value () const { return m_val; } + + /* Get the quality of the count. */ + enum profile_quality quality () const { return m_quality; } + /* Output THIS to F. */ void dump (FILE *f) const; @@ -675,7 +690,6 @@ private: return ipa_p () == other.ipa_p (); } public: - /* Used for counters which are expected to be never executed. */ static profile_count zero () { @@ -737,6 +751,9 @@ public: return m_quality == profile_precise; } + /* Get the value of the count. */ + uint32_t value () const { return m_val; } + /* Get the quality of the count. */ enum profile_quality quality () const { return m_quality; } @@ -1136,7 +1153,8 @@ public: /* The profiling runtime uses gcov_type, which is usually 64bit integer. Conversions back and forth are used to read the coverage and get it into internal representation. */ - static profile_count from_gcov_type (gcov_type v); + static profile_count from_gcov_type (gcov_type v, + profile_quality quality = profile_precise); /* LTO streaming support. */ static profile_count stream_in (struct lto_input_block *); diff --git a/gcc/testsuite/gcc.dg/gimplefe-37.c b/gcc/testsuite/gcc.dg/gimplefe-37.c new file mode 100644 index 00000000000..7554e0f2212 --- /dev/null +++ b/gcc/testsuite/gcc.dg/gimplefe-37.c @@ -0,0 +1,27 @@ +/* { dg-do compile } */ +/* { dg-options "-O2 -fgimple -fdump-tree-optimized" } */ + +int __GIMPLE (ssa,startwith("slsr"),hot_bb_threshold(10)) +main (int argc) +{ + int _1; + + __BB(2,precise(3)): + if (argc_2(D) == 2) + goto __BB3(precise(44739243)); + else + goto __BB4(precise(89478485)); + + __BB(3,precise(1)): + goto __BB4(precise(134217728)); + + __BB(4,precise(3)): + _1 = __PHI (__BB2: 0, __BB3: 12); + return _1; +} + + +/* { dg-final { scan-tree-dump-times "<bb \[0-9\]> \\\[count: 3" 2 "optimized" } } */ +/* { dg-final { scan-tree-dump-times "<bb \[0-9\]> \\\[count: 2" 1 "optimized" } } */ +/* { dg-final { scan-tree-dump-times "goto <bb \[0-9\]>; \\\[33\\\.33%\\\]" 1 "optimized" } } */ +/* { dg-final { scan-tree-dump-times "goto <bb \[0-9\]>; \\\[66\\\.67%\\\]" 1 "optimized" } } */ diff --git a/gcc/testsuite/gcc.dg/gimplefe-38.c b/gcc/testsuite/gcc.dg/gimplefe-38.c new file mode 100644 index 00000000000..9398db9b0ef --- /dev/null +++ b/gcc/testsuite/gcc.dg/gimplefe-38.c @@ -0,0 +1,27 @@ +/* { dg-do compile } */ +/* { dg-options "-O2 -fgimple -fdump-tree-slsr" } */ + +int __GIMPLE (ssa,startwith("slsr")) +main (int argc) +{ + int _1; + + __BB(2,guessed_local(1073741824)): + if (argc_2(D) == 2) + goto __BB3(guessed(16777216)); + else + goto __BB4(guessed(117440512)); + + __BB(3,guessed_local(134217728)): + goto __BB4(precise(134217728)); + + __BB(4,guessed_local(1073741824)): + _1 = __PHI (__BB2: 0, __BB3: 12); + return _1; +} + + +/* { dg-final { scan-tree-dump-times "<bb \[0-9\]> \\\[local count: 1073741824" 2 "slsr" } } */ +/* { dg-final { scan-tree-dump-times "<bb \[0-9\]> \\\[local count: 134217728" 1 "slsr" } } */ +/* { dg-final { scan-tree-dump-times "goto <bb \[0-9\]>; \\\[12\\\.50%\\\]" 1 "slsr" } } */ +/* { dg-final { scan-tree-dump-times "goto <bb \[0-9\]>; \\\[87\\\.50%\\\]" 1 "slsr" } } */ -- 2.21.0 ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH][stage1] Support profile (BB counts and edge probabilities) in GIMPLE FE. 2019-04-09 13:15 ` Martin Liška @ 2019-04-09 14:02 ` Jan Hubicka 2019-04-10 9:57 ` Martin Liška 0 siblings, 1 reply; 22+ messages in thread From: Jan Hubicka @ 2019-04-09 14:02 UTC (permalink / raw) To: Martin Liška; +Cc: Richard Biener, GCC Patches > Hi. > > There's updated version that supports profile quality for both counts > and probabilities. I'm wondering whether ENTRY and EXIT BBs needs to > have set probability. Apparently, I haven't seen any verifier that > would complain. Well, you do not need to define it but then several cases will degenerate. In particular BB frequencies (for callgraph profile or hot/cold decisions) are calculated as ratios of entry BB and given BB count. If entry BB is undefined you will get those undefined and heuristics will resort to conservative answers. I do not think we use exit block count. Honza ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH][stage1] Support profile (BB counts and edge probabilities) in GIMPLE FE. 2019-04-09 14:02 ` Jan Hubicka @ 2019-04-10 9:57 ` Martin Liška 2019-04-26 13:29 ` Richard Biener 0 siblings, 1 reply; 22+ messages in thread From: Martin Liška @ 2019-04-10 9:57 UTC (permalink / raw) To: Jan Hubicka; +Cc: Richard Biener, GCC Patches [-- Attachment #1: Type: text/plain, Size: 812 bytes --] On 4/9/19 3:19 PM, Jan Hubicka wrote: >> Hi. >> >> There's updated version that supports profile quality for both counts >> and probabilities. I'm wondering whether ENTRY and EXIT BBs needs to >> have set probability. Apparently, I haven't seen any verifier that >> would complain. > > Well, you do not need to define it but then several cases will > degenerate. In particular BB frequencies (for callgraph profile or > hot/cold decisions) are calculated as ratios of entry BB and given BB > count. If entry BB is undefined you will get those undefined and > heuristics will resort to conservative answers. > > I do not think we use exit block count. > > Honza > Thank you Honza for explanation. I'm sending version of the patch that supports entry BB count. I've been testing the patch right now. Martin [-- Attachment #2: 0001-Support-profile-BB-counts-and-edge-probabilities-in-.patch --] [-- Type: text/x-patch, Size: 25846 bytes --] From cc041144e55b0d002f43a1a2a61364b0e085c4a0 Mon Sep 17 00:00:00 2001 From: marxin <mliska@suse.cz> Date: Thu, 4 Apr 2019 14:46:15 +0200 Subject: [PATCH] Support profile (BB counts and edge probabilities) in GIMPLE FE. gcc/ChangeLog: 2019-04-05 Martin Liska <mliska@suse.cz> * tree-cfg.c (dump_function_to_file): Dump entry BB count. * gimple-pretty-print.c (dump_gimple_bb_header): Dump BB count. (pp_cfg_jump): Dump edge probability. * profile-count.c (profile_quality_as_string): Simplify with a static array. (parse_profile_quality): New function. (profile_count::dump): Simplify with a static array. (profile_count::from_gcov_type): Add new argument. * profile-count.h (parse_profile_quality): Likewise. gcc/c/ChangeLog: 2019-04-05 Martin Liska <mliska@suse.cz> * gimple-parser.c (struct gimple_parser): Add probability. for gimple_parser_edge. (gimple_parser::push_edge): Add new argument probability. (c_parser_gimple_parse_bb_spec): Parse also probability if present. (c_parser_parse_gimple_body): Set edge probability. (c_parser_gimple_compound_statement): Consume token before calling c_parser_gimple_goto_stmt. Parse BB counts. (c_parser_gimple_statement): Pass new argument. (c_parser_gimple_goto_stmt): Likewise. (c_parser_gimple_if_stmt): Likewise. (c_parser_gimple_or_rtl_pass_list): Parse hot_bb_threshold. * c-parser.c (c_parser_declaration_or_fndef): Pass hot_bb_threshold argument. * c-tree.h (struct c_declspecs): Add hot_bb_threshold field. gcc/testsuite/ChangeLog: 2019-04-05 Martin Liska <mliska@suse.cz> * gcc.dg/gimplefe-37.c: New test. * gcc.dg/gimplefe-33.c: Likewise. --- gcc/c/c-parser.c | 4 +- gcc/c/c-tree.h | 4 + gcc/c/gimple-parser.c | 195 +++++++++++++++++++++++++---- gcc/c/gimple-parser.h | 3 +- gcc/gimple-pretty-print.c | 13 ++ gcc/profile-count.c | 88 +++++++------ gcc/profile-count.h | 22 +++- gcc/testsuite/gcc.dg/gimplefe-37.c | 27 ++++ gcc/testsuite/gcc.dg/gimplefe-38.c | 27 ++++ gcc/tree-cfg.c | 11 +- 10 files changed, 322 insertions(+), 72 deletions(-) create mode 100644 gcc/testsuite/gcc.dg/gimplefe-37.c create mode 100644 gcc/testsuite/gcc.dg/gimplefe-38.c diff --git a/gcc/c/c-parser.c b/gcc/c/c-parser.c index 741d172ff30..8a6dea4ba06 100644 --- a/gcc/c/c-parser.c +++ b/gcc/c/c-parser.c @@ -2347,7 +2347,9 @@ c_parser_declaration_or_fndef (c_parser *parser, bool fndef_ok, bool saved = in_late_binary_op; in_late_binary_op = true; c_parser_parse_gimple_body (parser, specs->gimple_or_rtl_pass, - specs->declspec_il); + specs->declspec_il, + specs->hot_bb_threshold, + specs->entry_bb_count); in_late_binary_op = saved; } else diff --git a/gcc/c/c-tree.h b/gcc/c/c-tree.h index 9393f6d1545..9294fd4106e 100644 --- a/gcc/c/c-tree.h +++ b/gcc/c/c-tree.h @@ -317,6 +317,10 @@ struct c_declspecs { tree attrs; /* The pass to start compiling a __GIMPLE or __RTL function with. */ char *gimple_or_rtl_pass; + /* Hotness threshold for __GIMPLE FE. */ + gcov_type hot_bb_threshold; + /* ENTRY BB count. */ + profile_count entry_bb_count; /* The base-2 log of the greatest alignment required by an _Alignas specifier, in bytes, or -1 if no such specifiers with nonzero alignment. */ diff --git a/gcc/c/gimple-parser.c b/gcc/c/gimple-parser.c index fff34606dae..8a4262d6642 100644 --- a/gcc/c/gimple-parser.c +++ b/gcc/c/gimple-parser.c @@ -81,20 +81,23 @@ struct gimple_parser int src; int dest; int flags; + profile_probability probability; }; auto_vec<gimple_parser_edge> edges; basic_block current_bb; - void push_edge (int, int, int); + void push_edge (int, int, int, profile_probability); }; void -gimple_parser::push_edge (int src, int dest, int flags) +gimple_parser::push_edge (int src, int dest, int flags, + profile_probability prob) { gimple_parser_edge e; e.src = src; e.dest = dest; e.flags = flags; + e.probability = prob; edges.safe_push (e); } @@ -120,10 +123,12 @@ static void c_parser_gimple_expr_list (gimple_parser &, vec<tree> *); /* See if VAL is an identifier matching __BB<num> and return <num> - in *INDEX. Return true if so. */ + in *INDEX. Return true if so and parse also FREQUENCY of + the edge. */ static bool -c_parser_gimple_parse_bb_spec (tree val, int *index) +c_parser_gimple_parse_bb_spec (tree val, gimple_parser &parser, + int *index, profile_probability *probablity) { if (strncmp (IDENTIFIER_POINTER (val), "__BB", 4) != 0) return false; @@ -131,14 +136,65 @@ c_parser_gimple_parse_bb_spec (tree val, int *index) if (!ISDIGIT (*p)) return false; *index = atoi (IDENTIFIER_POINTER (val) + 4); - return *index > 0; + + if (*index > 0) + { + *probablity = profile_probability::uninitialized (); + /* Parse frequency if provided. */ + if (c_parser_next_token_is (parser, CPP_OPEN_PAREN)) + { + tree f; + c_parser_consume_token (parser); + if (!c_parser_next_token_is (parser, CPP_NAME)) + { + c_parser_error (parser, "expected frequency quality"); + return false; + } + + profile_quality quality; + const char *v + = IDENTIFIER_POINTER (c_parser_peek_token (parser)->value); + if (!parse_profile_quality (v, &quality)) + { + c_parser_error (parser, "unknown profile quality"); + return false; + } + + c_parser_consume_token (parser); + if (!c_parser_require (parser, CPP_OPEN_PAREN, "expected %<(%>")) + return false; + + if (!c_parser_next_token_is (parser, CPP_NUMBER) + || (TREE_CODE (f = c_parser_peek_token (parser)->value) + != INTEGER_CST)) + { + c_parser_error (parser, "expected frequency value"); + return false; + } + + unsigned int value = TREE_INT_CST_LOW (f); + *probablity = profile_probability (value, quality); + + c_parser_consume_token (parser); + if (!c_parser_require (parser, CPP_CLOSE_PAREN, "expected %<)%>")) + return false; + + if (!c_parser_require (parser, CPP_CLOSE_PAREN, "expected %<)%>")) + return false; + } + + return true; + } + + return false; } /* Parse the body of a function declaration marked with "__GIMPLE". */ void c_parser_parse_gimple_body (c_parser *cparser, char *gimple_pass, - enum c_declspec_il cdil) + enum c_declspec_il cdil, gcov_type hot_bb_threshold, + profile_count entry_bb_count) { gimple_parser parser (cparser); gimple_seq seq = NULL; @@ -209,9 +265,12 @@ c_parser_parse_gimple_body (c_parser *cparser, char *gimple_pass, add_local_decl (cfun, var); /* We have a CFG. Build the edges. */ for (unsigned i = 0; i < parser.edges.length (); ++i) - make_edge (BASIC_BLOCK_FOR_FN (cfun, parser.edges[i].src), - BASIC_BLOCK_FOR_FN (cfun, parser.edges[i].dest), - parser.edges[i].flags); + { + edge e = make_edge (BASIC_BLOCK_FOR_FN (cfun, parser.edges[i].src), + BASIC_BLOCK_FOR_FN (cfun, parser.edges[i].dest), + parser.edges[i].flags); + e->probability = parser.edges[i].probability; + } /* Add edges for case labels. */ basic_block bb; FOR_EACH_BB_FN (bb, cfun) @@ -274,6 +333,12 @@ c_parser_parse_gimple_body (c_parser *cparser, char *gimple_pass, fix_loop_structure (NULL); } + if (cfun->curr_properties & PROP_cfg) + { + update_max_bb_count (); + set_hot_bb_threshold (hot_bb_threshold); + ENTRY_BLOCK_PTR_FOR_FN (cfun)->count = entry_bb_count; + } dump_function (TDI_gimple, current_function_decl); } @@ -337,11 +402,9 @@ c_parser_gimple_compound_statement (gimple_parser &parser, gimple_seq *seq) c_parser_consume_token (parser); if (c_parser_next_token_is (parser, CPP_NAME)) { - c_parser_gimple_goto_stmt (parser, loc, - c_parser_peek_token - (parser)->value, - seq); + tree label = c_parser_peek_token (parser)->value; c_parser_consume_token (parser); + c_parser_gimple_goto_stmt (parser, loc, label, seq); if (! c_parser_require (parser, CPP_SEMICOLON, "expected %<;%>")) return return_p; @@ -355,7 +418,8 @@ c_parser_gimple_compound_statement (gimple_parser &parser, gimple_seq *seq) "expected %<;%>")) return return_p; if (cfun->curr_properties & PROP_cfg) - parser.push_edge (parser.current_bb->index, EXIT_BLOCK, 0); + parser.push_edge (parser.current_bb->index, EXIT_BLOCK, 0, + profile_probability::uninitialized ()); break; default: goto expr_stmt; @@ -397,6 +461,7 @@ c_parser_gimple_compound_statement (gimple_parser &parser, gimple_seq *seq) return return_p; } int is_loop_header_of = -1; + profile_count bb_count = profile_count::uninitialized (); c_parser_consume_token (parser); while (c_parser_next_token_is (parser, CPP_COMMA)) { @@ -430,10 +495,39 @@ c_parser_gimple_compound_statement (gimple_parser &parser, gimple_seq *seq) "expected %<)%>")) return return_p; } + /* Parse profile: quality(value) */ else { - c_parser_error (parser, "unknown block specifier"); - return return_p; + tree q; + profile_quality quality; + tree v = c_parser_peek_token (parser)->value; + if (!parse_profile_quality (IDENTIFIER_POINTER (v), + &quality)) + { + c_parser_error (parser, "unknown block specifier"); + return false; + } + + c_parser_consume_token (parser); + if (!c_parser_require (parser, CPP_OPEN_PAREN, + "expected %<(%>")) + return false; + + if (!c_parser_next_token_is (parser, CPP_NUMBER) + || (TREE_CODE (q = c_parser_peek_token (parser)->value) + != INTEGER_CST)) + { + c_parser_error (parser, "expected count value"); + return false; + } + + bb_count + = profile_count::from_gcov_type (TREE_INT_CST_LOW (q), + quality); + c_parser_consume_token (parser); + if (! c_parser_require (parser, CPP_CLOSE_PAREN, + "expected %<)%>")) + return return_p; } } if (! c_parser_require (parser, CPP_CLOSE_PAREN, @@ -470,7 +564,8 @@ c_parser_gimple_compound_statement (gimple_parser &parser, gimple_seq *seq) last_basic_block_for_fn (cfun) = index + 1; n_basic_blocks_for_fn (cfun)++; if (!parser.current_bb) - parser.push_edge (ENTRY_BLOCK, bb->index, EDGE_FALLTHRU); + parser.push_edge (ENTRY_BLOCK, bb->index, EDGE_FALLTHRU, + profile_probability::uninitialized ()); /* We leave the proper setting to fixup. */ struct loop *loop_father = loops_for_fn (cfun)->tree_root; @@ -498,6 +593,7 @@ c_parser_gimple_compound_statement (gimple_parser &parser, gimple_seq *seq) loop_father = get_loop (cfun, is_loop_header_of); } bb->loop_father = loop_father; + bb->count = bb_count; /* Stmts now go to the new block. */ parser.current_bb = bb; @@ -688,7 +784,9 @@ c_parser_gimple_statement (gimple_parser &parser, gimple_seq *seq) if (c_parser_next_token_is (parser, CPP_COLON)) c_parser_consume_token (parser); int src_index = -1; - if (!c_parser_gimple_parse_bb_spec (arg, &src_index)) + profile_probability prob; + if (!c_parser_gimple_parse_bb_spec (arg, parser, &src_index, + &prob)) c_parser_error (parser, "invalid source block specification"); vargs.safe_push (size_int (src_index)); } @@ -1609,8 +1707,10 @@ c_parser_gimple_or_rtl_pass_list (c_parser *parser, c_declspecs *specs) return; c_parser_consume_token (parser); + specs->entry_bb_count = profile_count::uninitialized (); while (c_parser_next_token_is (parser, CPP_NAME)) { + profile_quality quality; const char *op = IDENTIFIER_POINTER (c_parser_peek_token (parser)->value); c_parser_consume_token (parser); if (! strcmp (op, "startwith")) @@ -1629,6 +1729,45 @@ c_parser_gimple_or_rtl_pass_list (c_parser *parser, c_declspecs *specs) if (! c_parser_require (parser, CPP_CLOSE_PAREN, "expected %<(%>")) return; } + else if (!strcmp (op, "hot_bb_threshold")) + { + if (!c_parser_require (parser, CPP_OPEN_PAREN, "expected %<(%>")) + return; + + tree v; + if (!c_parser_next_token_is (parser, CPP_NUMBER) + || (TREE_CODE (v = c_parser_peek_token (parser)->value) + != INTEGER_CST)) + { + c_parser_error (parser, "expected frequency value"); + return; + } + + specs->hot_bb_threshold = TREE_INT_CST_LOW (v); + c_parser_consume_token (parser); + if (!c_parser_require (parser, CPP_CLOSE_PAREN, "expected %<)%>")) + return; + } + else if (parse_profile_quality (op, &quality)) + { + tree q; + if (!c_parser_require (parser, CPP_OPEN_PAREN, "expected %<(%>")) + return; + + if (!c_parser_next_token_is (parser, CPP_NUMBER) + || (TREE_CODE (q = c_parser_peek_token (parser)->value) + != INTEGER_CST)) + { + c_parser_error (parser, "expected count value"); + return; + } + + specs->entry_bb_count + = profile_count::from_gcov_type (TREE_INT_CST_LOW (q), quality); + c_parser_consume_token (parser); + if (!c_parser_require (parser, CPP_CLOSE_PAREN, "expected %<)%>")) + return; + } else if (specs->declspec_il != cdil_gimple) /* Allow only one IL specifier and none on RTL. */ ; @@ -1757,10 +1896,12 @@ c_parser_gimple_goto_stmt (gimple_parser &parser, if (cfun->curr_properties & PROP_cfg) { int dest_index; - if (c_parser_gimple_parse_bb_spec (label, &dest_index)) + profile_probability prob; + if (c_parser_gimple_parse_bb_spec (label, parser, &dest_index, + &prob)) { parser.push_edge (parser.current_bb->index, dest_index, - EDGE_FALLTHRU); + EDGE_FALLTHRU, prob); return; } } @@ -1811,10 +1952,12 @@ c_parser_gimple_if_stmt (gimple_parser &parser, gimple_seq *seq) label = c_parser_peek_token (parser)->value; c_parser_consume_token (parser); int dest_index; + profile_probability prob; if ((cfun->curr_properties & PROP_cfg) - && c_parser_gimple_parse_bb_spec (label, &dest_index)) + && c_parser_gimple_parse_bb_spec (label, parser, &dest_index, + &prob)) parser.push_edge (parser.current_bb->index, dest_index, - EDGE_TRUE_VALUE); + EDGE_TRUE_VALUE, prob); else t_label = lookup_label_for_goto (loc, label); if (! c_parser_require (parser, CPP_SEMICOLON, "expected %<;%>")) @@ -1844,14 +1987,16 @@ c_parser_gimple_if_stmt (gimple_parser &parser, gimple_seq *seq) return; } label = c_parser_peek_token (parser)->value; + c_parser_consume_token (parser); int dest_index; + profile_probability prob; if ((cfun->curr_properties & PROP_cfg) - && c_parser_gimple_parse_bb_spec (label, &dest_index)) + && c_parser_gimple_parse_bb_spec (label, parser, &dest_index, + &prob)) parser.push_edge (parser.current_bb->index, dest_index, - EDGE_FALSE_VALUE); + EDGE_FALSE_VALUE, prob); else f_label = lookup_label_for_goto (loc, label); - c_parser_consume_token (parser); if (! c_parser_require (parser, CPP_SEMICOLON, "expected %<;%>")) return; } diff --git a/gcc/c/gimple-parser.h b/gcc/c/gimple-parser.h index 383ad768759..1c7a155c06b 100644 --- a/gcc/c/gimple-parser.h +++ b/gcc/c/gimple-parser.h @@ -22,7 +22,8 @@ along with GCC; see the file COPYING3. If not see /* Gimple parsing functions. */ extern void c_parser_parse_gimple_body (c_parser *, char *, - enum c_declspec_il); + enum c_declspec_il, gcov_type, + profile_count); extern void c_parser_gimple_or_rtl_pass_list (c_parser *, c_declspecs *); #endif diff --git a/gcc/gimple-pretty-print.c b/gcc/gimple-pretty-print.c index 69bae0d10d0..7e3916bff86 100644 --- a/gcc/gimple-pretty-print.c +++ b/gcc/gimple-pretty-print.c @@ -2704,6 +2704,10 @@ dump_gimple_bb_header (FILE *outf, basic_block bb, int indent, fprintf (outf, "%*s__BB(%d", indent, "", bb->index); if (bb->loop_father->header == bb) fprintf (outf, ",loop_header(%d)", bb->loop_father->num); + if (bb->count.initialized_p ()) + fprintf (outf, ",%s(%d)", + profile_quality_as_string (bb->count.quality ()), + bb->count.value ()); fprintf (outf, "):\n"); } else @@ -2760,6 +2764,15 @@ pp_cfg_jump (pretty_printer *buffer, edge e, dump_flags_t flags) { pp_string (buffer, "goto __BB"); pp_decimal_int (buffer, e->dest->index); + if (e->probability.initialized_p ()) + { + pp_string (buffer, "("); + pp_string (buffer, + profile_quality_as_string (e->probability.quality ())); + pp_string (buffer, "("); + pp_decimal_int (buffer, e->probability.value ()); + pp_string (buffer, "))"); + } pp_semicolon (buffer); } else diff --git a/gcc/profile-count.c b/gcc/profile-count.c index 8c58f8666f0..4f0bac002d7 100644 --- a/gcc/profile-count.c +++ b/gcc/profile-count.c @@ -33,34 +33,57 @@ along with GCC; see the file COPYING3. If not see #include "wide-int.h" #include "sreal.h" +/* Names from profile_quality enum values. */ + +const char *profile_quality_names[] = +{ + "uninitialized", + "guessed_local", + "guessed_global0", + "guessed_global0adjusted", + "guessed", + "afdo", + "adjusted", + "precise" +}; + /* Get a string describing QUALITY. */ const char * profile_quality_as_string (enum profile_quality quality) { - switch (quality) - { - default: - gcc_unreachable (); - case profile_uninitialized: - return "uninitialized"; - case profile_guessed_local: - return "guessed_local"; - case profile_guessed_global0: - return "guessed_global0"; - case profile_guessed_global0adjusted: - return "guessed_global0adjusted"; - case profile_guessed: - return "guessed"; - case profile_afdo: - return "afdo"; - case profile_adjusted: - return "adjusted"; - case profile_precise: - return "precise"; - } + return profile_quality_names[quality]; +} + +/* Parse VALUE as profile quality and return true when a valid QUALITY. */ + +bool +parse_profile_quality (const char *value, profile_quality *quality) +{ + for (unsigned i = 0; i < ARRAY_SIZE (profile_quality_names); i++) + if (strcmp (profile_quality_names[i], value) == 0) + { + *quality = (profile_quality)i; + return true; + } + + return false; } +/* Display names from profile_quality enum values. */ + +const char *profile_quality_display_names[] = +{ + NULL, + "estimated locally", + "estimated locally, globally 0", + "estimated locally, globally 0 adjusted", + "adjusted", + "auto FDO", + "guessed", + "precise" +}; + /* Dump THIS to F. */ void @@ -69,23 +92,8 @@ profile_count::dump (FILE *f) const if (!initialized_p ()) fprintf (f, "uninitialized"); else - { - fprintf (f, "%" PRId64, m_val); - if (m_quality == profile_guessed_local) - fprintf (f, " (estimated locally)"); - else if (m_quality == profile_guessed_global0) - fprintf (f, " (estimated locally, globally 0)"); - else if (m_quality == profile_guessed_global0adjusted) - fprintf (f, " (estimated locally, globally 0 adjusted)"); - else if (m_quality == profile_adjusted) - fprintf (f, " (adjusted)"); - else if (m_quality == profile_afdo) - fprintf (f, " (auto FDO)"); - else if (m_quality == profile_guessed) - fprintf (f, " (guessed)"); - else if (m_quality == profile_precise) - fprintf (f, " (precise)"); - } + fprintf (f, "%" PRId64 " (%s)", m_val, + profile_quality_display_names[m_quality]); } /* Dump THIS to stderr. */ @@ -362,7 +370,7 @@ profile_count::combine_with_ipa_count (profile_count ipa) Conversions back and forth are used to read the coverage and get it into internal representation. */ profile_count -profile_count::from_gcov_type (gcov_type v) +profile_count::from_gcov_type (gcov_type v, profile_quality quality) { profile_count ret; gcc_checking_assert (v >= 0); @@ -371,7 +379,7 @@ profile_count::from_gcov_type (gcov_type v) "Capping gcov count %" PRId64 " to max_count %" PRId64 "\n", (int64_t) v, (int64_t) max_count); ret.m_val = MIN (v, (gcov_type)max_count); - ret.m_quality = profile_precise; + ret.m_quality = quality; return ret; } diff --git a/gcc/profile-count.h b/gcc/profile-count.h index d6de61f0a61..2669f0d5fff 100644 --- a/gcc/profile-count.h +++ b/gcc/profile-count.h @@ -60,6 +60,8 @@ enum profile_quality { }; extern const char *profile_quality_as_string (enum profile_quality); +extern bool parse_profile_quality (const char *value, + profile_quality *quality); /* The base value for branch probability notes and edge probabilities. */ #define REG_BR_PROB_BASE 10000 @@ -149,6 +151,13 @@ class GTY((user)) profile_probability friend class profile_count; public: + profile_probability (): m_val (uninitialized_probability), + m_quality (profile_guessed) + {} + + profile_probability (uint32_t val, profile_quality quality): + m_val (val), m_quality (quality) + {} /* Named probabilities. */ static profile_probability never () @@ -558,6 +567,12 @@ public: return initialized_p () && other.initialized_p () && m_val >= other.m_val; } + /* Get the value of the count. */ + uint32_t value () const { return m_val; } + + /* Get the quality of the count. */ + enum profile_quality quality () const { return m_quality; } + /* Output THIS to F. */ void dump (FILE *f) const; @@ -675,7 +690,6 @@ private: return ipa_p () == other.ipa_p (); } public: - /* Used for counters which are expected to be never executed. */ static profile_count zero () { @@ -737,6 +751,9 @@ public: return m_quality == profile_precise; } + /* Get the value of the count. */ + uint32_t value () const { return m_val; } + /* Get the quality of the count. */ enum profile_quality quality () const { return m_quality; } @@ -1136,7 +1153,8 @@ public: /* The profiling runtime uses gcov_type, which is usually 64bit integer. Conversions back and forth are used to read the coverage and get it into internal representation. */ - static profile_count from_gcov_type (gcov_type v); + static profile_count from_gcov_type (gcov_type v, + profile_quality quality = profile_precise); /* LTO streaming support. */ static profile_count stream_in (struct lto_input_block *); diff --git a/gcc/testsuite/gcc.dg/gimplefe-37.c b/gcc/testsuite/gcc.dg/gimplefe-37.c new file mode 100644 index 00000000000..855bc306fea --- /dev/null +++ b/gcc/testsuite/gcc.dg/gimplefe-37.c @@ -0,0 +1,27 @@ +/* { dg-do compile } */ +/* { dg-options "-O2 -fgimple -fdump-tree-optimized" } */ + +int __GIMPLE (ssa,startwith("slsr"),hot_bb_threshold(10),precise(3)) +main (int argc) +{ + int _1; + + __BB(2,precise(3)): + if (argc_2(D) == 2) + goto __BB3(precise(44739243)); + else + goto __BB4(precise(89478485)); + + __BB(3,precise(1)): + goto __BB4(precise(134217728)); + + __BB(4,precise(3)): + _1 = __PHI (__BB2: 0, __BB3: 12); + return _1; +} + + +/* { dg-final { scan-tree-dump-times "<bb \[0-9\]> \\\[count: 3" 2 "optimized" } } */ +/* { dg-final { scan-tree-dump-times "<bb \[0-9\]> \\\[count: 2" 1 "optimized" } } */ +/* { dg-final { scan-tree-dump-times "goto <bb \[0-9\]>; \\\[33\\\.33%\\\]" 1 "optimized" } } */ +/* { dg-final { scan-tree-dump-times "goto <bb \[0-9\]>; \\\[66\\\.67%\\\]" 1 "optimized" } } */ diff --git a/gcc/testsuite/gcc.dg/gimplefe-38.c b/gcc/testsuite/gcc.dg/gimplefe-38.c new file mode 100644 index 00000000000..64532d87da5 --- /dev/null +++ b/gcc/testsuite/gcc.dg/gimplefe-38.c @@ -0,0 +1,27 @@ +/* { dg-do compile } */ +/* { dg-options "-O2 -fgimple -fdump-tree-slsr" } */ + +int __GIMPLE (ssa,startwith("slsr"),guessed_local(1073741824)) +main (int argc) +{ + int _1; + + __BB(2,guessed_local(1073741824)): + if (argc_2(D) == 2) + goto __BB3(guessed(16777216)); + else + goto __BB4(guessed(117440512)); + + __BB(3,guessed_local(134217728)): + goto __BB4(precise(134217728)); + + __BB(4,guessed_local(1073741824)): + _1 = __PHI (__BB2: 0, __BB3: 12); + return _1; +} + + +/* { dg-final { scan-tree-dump-times "<bb \[0-9\]> \\\[local count: 1073741824" 2 "slsr" } } */ +/* { dg-final { scan-tree-dump-times "<bb \[0-9\]> \\\[local count: 134217728" 1 "slsr" } } */ +/* { dg-final { scan-tree-dump-times "goto <bb \[0-9\]>; \\\[12\\\.50%\\\]" 1 "slsr" } } */ +/* { dg-final { scan-tree-dump-times "goto <bb \[0-9\]>; \\\[87\\\.50%\\\]" 1 "slsr" } } */ diff --git a/gcc/tree-cfg.c b/gcc/tree-cfg.c index 0dc94ea41d4..bf50de5da5e 100644 --- a/gcc/tree-cfg.c +++ b/gcc/tree-cfg.c @@ -7874,11 +7874,16 @@ dump_function_to_file (tree fndecl, FILE *file, dump_flags_t flags) { print_generic_expr (file, TREE_TYPE (TREE_TYPE (fndecl)), dump_flags | TDF_SLIM); - fprintf (file, " __GIMPLE (%s)\n%s (", + fprintf (file, " __GIMPLE (%s", (fun->curr_properties & PROP_ssa) ? "ssa" : (fun->curr_properties & PROP_cfg) ? "cfg" - : "", - function_name (fun)); + : ""); + basic_block bb = ENTRY_BLOCK_PTR_FOR_FN (cfun); + if (bb->count.initialized_p ()) + fprintf (file, ",%s(%d)", + profile_quality_as_string (bb->count.quality ()), + bb->count.value ()); + fprintf (file, ")\n%s (", function_name (fun)); } else fprintf (file, "%s %s(", function_name (fun), tmclone ? "[tm-clone] " : ""); -- 2.21.0 ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH][stage1] Support profile (BB counts and edge probabilities) in GIMPLE FE. 2019-04-10 9:57 ` Martin Liška @ 2019-04-26 13:29 ` Richard Biener 2019-04-29 12:54 ` Martin Liška 2019-04-30 2:05 ` Joseph Myers 0 siblings, 2 replies; 22+ messages in thread From: Richard Biener @ 2019-04-26 13:29 UTC (permalink / raw) To: Martin Liška, Joseph S. Myers; +Cc: Jan Hubicka, GCC Patches On Wed, Apr 10, 2019 at 10:12 AM Martin Liška <mliska@suse.cz> wrote: > > On 4/9/19 3:19 PM, Jan Hubicka wrote: > >> Hi. > >> > >> There's updated version that supports profile quality for both counts > >> and probabilities. I'm wondering whether ENTRY and EXIT BBs needs to > >> have set probability. Apparently, I haven't seen any verifier that > >> would complain. > > > > Well, you do not need to define it but then several cases will > > degenerate. In particular BB frequencies (for callgraph profile or > > hot/cold decisions) are calculated as ratios of entry BB and given BB > > count. If entry BB is undefined you will get those undefined and > > heuristics will resort to conservative answers. > > > > I do not think we use exit block count. > > > > Honza > > > > Thank you Honza for explanation. I'm sending version of the patch > that supports entry BB count. > > I've been testing the patch right now. Can you move the GIMPLE/RTL FE specific data in c_declspecs to a substructure accessed via indirection? I guess enlarging that isn't really what we should do. You'd move gimple_or_rtl_pass there and make that pointer one to a struct aux_fe_data (lifetime managed by the respective RTL/GIMPLE FE, thus to be freed there)? Joseph, do you agree or is adding more stuff to c_declspecs OK (I would guess it could be a few more elements in the future). -c_parser_gimple_parse_bb_spec (tree val, int *index) +c_parser_gimple_parse_bb_spec (tree val, gimple_parser &parser, + int *index, profile_probability *probablity) { so this will allow specifying probability in PHI node arguments. I think we want to split this into the already existing part and a c_parser_gimple_parse_bb_spec_with_edge_probability to be used at edge sources. + if (cfun->curr_properties & PROP_cfg) + { + update_max_bb_count (); + set_hot_bb_threshold (hot_bb_threshold); + ENTRY_BLOCK_PTR_FOR_FN (cfun)->count = entry_bb_count; I guess the last one should be before update_max_bb_count ()? + } + /* Parse profile: quality(value) */ else { - c_parser_error (parser, "unknown block specifier"); - return return_p; + tree q; + profile_quality quality; + tree v = c_parser_peek_token (parser)->value; peek next token before the if/else and do else if (!strcmp (...)) as in the loop_header case. I expected we can somehow share parsing of profile quality and BB/edge count/frequency? How's the expected syntax btw, comments in the code should tell us. I'm guessing it's quality-id '(' number ')' and thus it should be really shareable between edge and BB count and also __GIMPLE header parsing? So parse_profile_quality should be parse_profile () instead, resulting in a combined value (we do use the same for edge/bb?). + else if (!strcmp (op, "hot_bb_threshold")) + { I'm not sure about this - it doesn't make sense to specify this on a per-function base since it seems to control a global variable (booo!)? Isn't this instead computed on-demand based on profile_info->sum_max? If not then I think we need an alternate way of funneling in global state into the GIMPLE FE. Thanks, Richard. > > Martin ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH][stage1] Support profile (BB counts and edge probabilities) in GIMPLE FE. 2019-04-26 13:29 ` Richard Biener @ 2019-04-29 12:54 ` Martin Liška 2019-05-02 12:31 ` Richard Biener 2019-04-30 2:05 ` Joseph Myers 1 sibling, 1 reply; 22+ messages in thread From: Martin Liška @ 2019-04-29 12:54 UTC (permalink / raw) To: Richard Biener, Joseph S. Myers; +Cc: Jan Hubicka, GCC Patches On 4/26/19 3:18 PM, Richard Biener wrote: > On Wed, Apr 10, 2019 at 10:12 AM Martin LiÅ¡ka <mliska@suse.cz> wrote: >> >> On 4/9/19 3:19 PM, Jan Hubicka wrote: >>>> Hi. >>>> >>>> There's updated version that supports profile quality for both counts >>>> and probabilities. I'm wondering whether ENTRY and EXIT BBs needs to >>>> have set probability. Apparently, I haven't seen any verifier that >>>> would complain. >>> >>> Well, you do not need to define it but then several cases will >>> degenerate. In particular BB frequencies (for callgraph profile or >>> hot/cold decisions) are calculated as ratios of entry BB and given BB >>> count. If entry BB is undefined you will get those undefined and >>> heuristics will resort to conservative answers. >>> >>> I do not think we use exit block count. >>> >>> Honza >>> >> >> Thank you Honza for explanation. I'm sending version of the patch >> that supports entry BB count. >> >> I've been testing the patch right now. > > Can you move the GIMPLE/RTL FE specific data in c_declspecs to > a substructure accessed via indirection? I guess enlarging that > isn't really what we should do. You'd move gimple_or_rtl_pass > there and make that pointer one to a struct aux_fe_data > (lifetime managed by the respective RTL/GIMPLE FE, thus > to be freed there)? Joseph, do you agree or is adding more > stuff to c_declspecs OK (I would guess it could be a few more > elements in the future). Let's wait here for Joseph. > > -c_parser_gimple_parse_bb_spec (tree val, int *index) > +c_parser_gimple_parse_bb_spec (tree val, gimple_parser &parser, > + int *index, profile_probability *probablity) > { > > so this will allow specifying probability in PHI node arguments. > I think we want to split this into the already existing part > and a c_parser_gimple_parse_bb_spec_with_edge_probability > to be used at edge sources. Yes, that's a good idea! > > + if (cfun->curr_properties & PROP_cfg) > + { > + update_max_bb_count (); > + set_hot_bb_threshold (hot_bb_threshold); > + ENTRY_BLOCK_PTR_FOR_FN (cfun)->count = entry_bb_count; > > I guess the last one should be before update_max_bb_count ()? Same here. > > + } > > + /* Parse profile: quality(value) */ > else > { > - c_parser_error (parser, "unknown block specifier"); > - return return_p; > + tree q; > + profile_quality quality; > + tree v = c_parser_peek_token (parser)->value; > > peek next token before the if/else and do > > else if (!strcmp (...)) > > as in the loop_header case. I expected we can somehow share > parsing of profile quality and BB/edge count/frequency? How's > the expected syntax btw, comments in the code should tell us. > I'm guessing it's quality-id '(' number ')' and thus it should be > really shareable between edge and BB count and also __GIMPLE > header parsing? So parse_profile_quality should be > parse_profile () instead, resulting in a combined value > (we do use the same for edge/bb?). It's problematic, there are different error messages for count/frequency. Moreover call to parse_profile_quality in c_parser_gimple_or_rtl_pass_list is a way how to test that next 'token' is a profile count. > > + else if (!strcmp (op, "hot_bb_threshold")) > + { > > I'm not sure about this - it doesn't make sense to specify this > on a per-function base since it seems to control a global > variable (booo!)? Yep, shame on me! > Isn't this instead computed on-demand > based on profile_info->sum_max? No it's a global value shared among functions. > If not then I think > we need an alternate way of funneling in global state into > the GIMPLE FE. What about --param gimple-fe-hot-bb-threshold ? Thanks, Martin > > Thanks, > Richard. > > >> >> Martin ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH][stage1] Support profile (BB counts and edge probabilities) in GIMPLE FE. 2019-04-29 12:54 ` Martin Liška @ 2019-05-02 12:31 ` Richard Biener 2019-05-06 7:59 ` Martin Liška 0 siblings, 1 reply; 22+ messages in thread From: Richard Biener @ 2019-05-02 12:31 UTC (permalink / raw) To: Martin Liška; +Cc: Joseph S. Myers, Jan Hubicka, GCC Patches On Mon, Apr 29, 2019 at 2:51 PM Martin Liška <mliska@suse.cz> wrote: > > On 4/26/19 3:18 PM, Richard Biener wrote: > > On Wed, Apr 10, 2019 at 10:12 AM Martin Liška <mliska@suse.cz> wrote: > >> > >> On 4/9/19 3:19 PM, Jan Hubicka wrote: > >>>> Hi. > >>>> > >>>> There's updated version that supports profile quality for both counts > >>>> and probabilities. I'm wondering whether ENTRY and EXIT BBs needs to > >>>> have set probability. Apparently, I haven't seen any verifier that > >>>> would complain. > >>> > >>> Well, you do not need to define it but then several cases will > >>> degenerate. In particular BB frequencies (for callgraph profile or > >>> hot/cold decisions) are calculated as ratios of entry BB and given BB > >>> count. If entry BB is undefined you will get those undefined and > >>> heuristics will resort to conservative answers. > >>> > >>> I do not think we use exit block count. > >>> > >>> Honza > >>> > >> > >> Thank you Honza for explanation. I'm sending version of the patch > >> that supports entry BB count. > >> > >> I've been testing the patch right now. > > > > Can you move the GIMPLE/RTL FE specific data in c_declspecs to > > a substructure accessed via indirection? I guess enlarging that > > isn't really what we should do. You'd move gimple_or_rtl_pass > > there and make that pointer one to a struct aux_fe_data > > (lifetime managed by the respective RTL/GIMPLE FE, thus > > to be freed there)? Joseph, do you agree or is adding more > > stuff to c_declspecs OK (I would guess it could be a few more > > elements in the future). > > Let's wait here for Joseph. So looks like it won't matter so let's go with the current approach for the moment. > > > > -c_parser_gimple_parse_bb_spec (tree val, int *index) > > +c_parser_gimple_parse_bb_spec (tree val, gimple_parser &parser, > > + int *index, profile_probability *probablity) > > { > > > > so this will allow specifying probability in PHI node arguments. > > I think we want to split this into the already existing part > > and a c_parser_gimple_parse_bb_spec_with_edge_probability > > to be used at edge sources. > > Yes, that's a good idea! > > > > > + if (cfun->curr_properties & PROP_cfg) > > + { > > + update_max_bb_count (); > > + set_hot_bb_threshold (hot_bb_threshold); > > + ENTRY_BLOCK_PTR_FOR_FN (cfun)->count = entry_bb_count; > > > > I guess the last one should be before update_max_bb_count ()? > > Same here. > > > > > + } > > > > + /* Parse profile: quality(value) */ > > else > > { > > - c_parser_error (parser, "unknown block specifier"); > > - return return_p; > > + tree q; > > + profile_quality quality; > > + tree v = c_parser_peek_token (parser)->value; > > > > peek next token before the if/else and do > > > > else if (!strcmp (...)) > > > > as in the loop_header case. I expected we can somehow share > > parsing of profile quality and BB/edge count/frequency? How's > > the expected syntax btw, comments in the code should tell us. > > I'm guessing it's quality-id '(' number ')' and thus it should be > > really shareable between edge and BB count and also __GIMPLE > > header parsing? So parse_profile_quality should be > > parse_profile () instead, resulting in a combined value > > (we do use the same for edge/bb?). > > It's problematic, there are different error messages for count/frequency. > Moreover call to parse_profile_quality in c_parser_gimple_or_rtl_pass_list > is a way how to test that next 'token' is a profile count. Who cares about error messages... But sure, I'm just proposing to merge testing for next token and actual parsing. > > > > + else if (!strcmp (op, "hot_bb_threshold")) > > + { > > > > I'm not sure about this - it doesn't make sense to specify this > > on a per-function base since it seems to control a global > > variable (booo!)? > > Yep, shame on me! > > > Isn't this instead computed on-demand > > based on profile_info->sum_max? > > No it's a global value shared among functions. > > > If not then I think > > we need an alternate way of funneling in global state into > > the GIMPLE FE. > > What about --param gimple-fe-hot-bb-threshold ? I thought about that, yes ... in absence can it actually be "computed"? Richard. > Thanks, > Martin > > > > > Thanks, > > Richard. > > > > > >> > >> Martin > ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH][stage1] Support profile (BB counts and edge probabilities) in GIMPLE FE. 2019-05-02 12:31 ` Richard Biener @ 2019-05-06 7:59 ` Martin Liška 2019-05-06 8:00 ` [PATCH 2/2] Support {MIN,MAX}_EXPR " Martin Liška 2019-05-06 14:02 ` [PATCH][stage1] Support profile (BB counts and edge probabilities) " Richard Biener 0 siblings, 2 replies; 22+ messages in thread From: Martin Liška @ 2019-05-06 7:59 UTC (permalink / raw) To: Richard Biener; +Cc: Joseph S. Myers, Jan Hubicka, GCC Patches [-- Attachment #1: Type: text/plain, Size: 4894 bytes --] On 5/2/19 2:31 PM, Richard Biener wrote: > On Mon, Apr 29, 2019 at 2:51 PM Martin LiÅ¡ka <mliska@suse.cz> wrote: >> >> On 4/26/19 3:18 PM, Richard Biener wrote: >>> On Wed, Apr 10, 2019 at 10:12 AM Martin LiÅ¡ka <mliska@suse.cz> wrote: >>>> >>>> On 4/9/19 3:19 PM, Jan Hubicka wrote: >>>>>> Hi. >>>>>> >>>>>> There's updated version that supports profile quality for both counts >>>>>> and probabilities. I'm wondering whether ENTRY and EXIT BBs needs to >>>>>> have set probability. Apparently, I haven't seen any verifier that >>>>>> would complain. >>>>> >>>>> Well, you do not need to define it but then several cases will >>>>> degenerate. In particular BB frequencies (for callgraph profile or >>>>> hot/cold decisions) are calculated as ratios of entry BB and given BB >>>>> count. If entry BB is undefined you will get those undefined and >>>>> heuristics will resort to conservative answers. >>>>> >>>>> I do not think we use exit block count. >>>>> >>>>> Honza >>>>> >>>> >>>> Thank you Honza for explanation. I'm sending version of the patch >>>> that supports entry BB count. >>>> >>>> I've been testing the patch right now. >>> >>> Can you move the GIMPLE/RTL FE specific data in c_declspecs to >>> a substructure accessed via indirection? I guess enlarging that >>> isn't really what we should do. You'd move gimple_or_rtl_pass >>> there and make that pointer one to a struct aux_fe_data >>> (lifetime managed by the respective RTL/GIMPLE FE, thus >>> to be freed there)? Joseph, do you agree or is adding more >>> stuff to c_declspecs OK (I would guess it could be a few more >>> elements in the future). >> >> Let's wait here for Joseph. > > So looks like it won't matter so let's go with the current approach > for the moment. > >>> >>> -c_parser_gimple_parse_bb_spec (tree val, int *index) >>> +c_parser_gimple_parse_bb_spec (tree val, gimple_parser &parser, >>> + int *index, profile_probability *probablity) >>> { >>> >>> so this will allow specifying probability in PHI node arguments. >>> I think we want to split this into the already existing part >>> and a c_parser_gimple_parse_bb_spec_with_edge_probability >>> to be used at edge sources. >> >> Yes, that's a good idea! >> >>> >>> + if (cfun->curr_properties & PROP_cfg) >>> + { >>> + update_max_bb_count (); >>> + set_hot_bb_threshold (hot_bb_threshold); >>> + ENTRY_BLOCK_PTR_FOR_FN (cfun)->count = entry_bb_count; >>> >>> I guess the last one should be before update_max_bb_count ()? >> >> Same here. >> >>> >>> + } >>> >>> + /* Parse profile: quality(value) */ >>> else >>> { >>> - c_parser_error (parser, "unknown block specifier"); >>> - return return_p; >>> + tree q; >>> + profile_quality quality; >>> + tree v = c_parser_peek_token (parser)->value; >>> >>> peek next token before the if/else and do >>> >>> else if (!strcmp (...)) >>> >>> as in the loop_header case. I expected we can somehow share >>> parsing of profile quality and BB/edge count/frequency? How's >>> the expected syntax btw, comments in the code should tell us. >>> I'm guessing it's quality-id '(' number ')' and thus it should be >>> really shareable between edge and BB count and also __GIMPLE >>> header parsing? So parse_profile_quality should be >>> parse_profile () instead, resulting in a combined value >>> (we do use the same for edge/bb?). >> >> It's problematic, there are different error messages for count/frequency. >> Moreover call to parse_profile_quality in c_parser_gimple_or_rtl_pass_list >> is a way how to test that next 'token' is a profile count. > > Who cares about error messages... But sure, I'm just proposing to > merge testing for next token and actual parsing. After I've done removal of hot_bb_threshold parsing, there are just 2 usages of parse_profile_quality. I would like to leave it as it, not introducing a wrappers. > >>> >>> + else if (!strcmp (op, "hot_bb_threshold")) >>> + { >>> >>> I'm not sure about this - it doesn't make sense to specify this >>> on a per-function base since it seems to control a global >>> variable (booo!)? >> >> Yep, shame on me! >> >>> Isn't this instead computed on-demand >>> based on profile_info->sum_max? >> >> No it's a global value shared among functions. >> >>> If not then I think >>> we need an alternate way of funneling in global state into >>> the GIMPLE FE. >> >> What about --param gimple-fe-hot-bb-threshold ? > > I thought about that, yes ... in absence can it actually be > "computed"? Renamed to it. Patch can bootstrap on x86_64-linux-gnu and survives regression tests. Ready to be installed? Thanks, Martin > > Richard. > >> Thanks, >> Martin >> >>> >>> Thanks, >>> Richard. >>> >>> >>>> >>>> Martin >> [-- Attachment #2: 0001-Support-profile-BB-counts-and-edge-probabilities-in-.patch --] [-- Type: text/x-patch, Size: 27338 bytes --] From e61a8cbf3c9c8b7aef250e1a5d31cf654fad76bb Mon Sep 17 00:00:00 2001 From: marxin <mliska@suse.cz> Date: Thu, 4 Apr 2019 14:46:15 +0200 Subject: [PATCH 1/2] Support profile (BB counts and edge probabilities) in GIMPLE FE. gcc/ChangeLog: 2019-04-05 Martin Liska <mliska@suse.cz> * tree-cfg.c (dump_function_to_file): Dump entry BB count. * gimple-pretty-print.c (dump_gimple_bb_header): Dump BB count. (pp_cfg_jump): Dump edge probability. * profile-count.c (profile_quality_as_string): Simplify with a static array. (parse_profile_quality): New function. (profile_count::dump): Simplify with a static array. (profile_count::from_gcov_type): Add new argument. * profile-count.h (parse_profile_quality): Likewise. * predict.h (get_current_hot_bb_threshold): New. * params.def (PARAM_GIMPLE_FE_COMPUTED_HOT_BB_THRESHOLD): New param. * predict.c (get_hot_bb_threshold): Set from the new param. (get_current_hot_bb_threshold): New. gcc/c/ChangeLog: 2019-04-05 Martin Liska <mliska@suse.cz> * gimple-parser.c (struct gimple_parser): Add probability. for gimple_parser_edge. (gimple_parser::push_edge): Add new argument probability. (c_parser_gimple_parse_bb_spec): Parse also probability if present. (c_parser_parse_gimple_body): Set edge probability. (c_parser_gimple_compound_statement): Consume token before calling c_parser_gimple_goto_stmt. Parse BB counts. (c_parser_gimple_statement): Pass new argument. (c_parser_gimple_goto_stmt): Likewise. (c_parser_gimple_if_stmt): Likewise. (c_parser_gimple_or_rtl_pass_list): Parse hot_bb_threshold. * c-parser.c (c_parser_declaration_or_fndef): Pass hot_bb_threshold argument. * c-tree.h (struct c_declspecs): Add hot_bb_threshold field. (c_parser_gimple_parse_bb_spec_edge_probability): New. gcc/testsuite/ChangeLog: 2019-04-05 Martin Liska <mliska@suse.cz> * gcc.dg/gimplefe-37.c: New test. * gcc.dg/gimplefe-33.c: Likewise. --- gcc/c/c-parser.c | 3 +- gcc/c/c-tree.h | 2 + gcc/c/gimple-parser.c | 180 +++++++++++++++++++++++++---- gcc/c/gimple-parser.h | 3 +- gcc/gimple-pretty-print.c | 13 +++ gcc/params.def | 6 + gcc/predict.c | 15 ++- gcc/predict.h | 1 + gcc/profile-count.c | 88 +++++++------- gcc/profile-count.h | 22 +++- gcc/testsuite/gcc.dg/gimplefe-37.c | 27 +++++ gcc/testsuite/gcc.dg/gimplefe-38.c | 27 +++++ gcc/tree-cfg.c | 25 +++- 13 files changed, 341 insertions(+), 71 deletions(-) create mode 100644 gcc/testsuite/gcc.dg/gimplefe-37.c create mode 100644 gcc/testsuite/gcc.dg/gimplefe-38.c diff --git a/gcc/c/c-parser.c b/gcc/c/c-parser.c index 854cd6ce8c6..3aa85125cf1 100644 --- a/gcc/c/c-parser.c +++ b/gcc/c/c-parser.c @@ -2347,7 +2347,8 @@ c_parser_declaration_or_fndef (c_parser *parser, bool fndef_ok, bool saved = in_late_binary_op; in_late_binary_op = true; c_parser_parse_gimple_body (parser, specs->gimple_or_rtl_pass, - specs->declspec_il); + specs->declspec_il, + specs->entry_bb_count); in_late_binary_op = saved; } else diff --git a/gcc/c/c-tree.h b/gcc/c/c-tree.h index 7e35ab1f0bc..346f46a5207 100644 --- a/gcc/c/c-tree.h +++ b/gcc/c/c-tree.h @@ -317,6 +317,8 @@ struct c_declspecs { tree attrs; /* The pass to start compiling a __GIMPLE or __RTL function with. */ char *gimple_or_rtl_pass; + /* ENTRY BB count. */ + profile_count entry_bb_count; /* The base-2 log of the greatest alignment required by an _Alignas specifier, in bytes, or -1 if no such specifiers with nonzero alignment. */ diff --git a/gcc/c/gimple-parser.c b/gcc/c/gimple-parser.c index fff34606dae..6558770873f 100644 --- a/gcc/c/gimple-parser.c +++ b/gcc/c/gimple-parser.c @@ -81,20 +81,23 @@ struct gimple_parser int src; int dest; int flags; + profile_probability probability; }; auto_vec<gimple_parser_edge> edges; basic_block current_bb; - void push_edge (int, int, int); + void push_edge (int, int, int, profile_probability); }; void -gimple_parser::push_edge (int src, int dest, int flags) +gimple_parser::push_edge (int src, int dest, int flags, + profile_probability prob) { gimple_parser_edge e; e.src = src; e.dest = dest; e.flags = flags; + e.probability = prob; edges.safe_push (e); } @@ -120,7 +123,7 @@ static void c_parser_gimple_expr_list (gimple_parser &, vec<tree> *); /* See if VAL is an identifier matching __BB<num> and return <num> - in *INDEX. Return true if so. */ + in *INDEX. */ static bool c_parser_gimple_parse_bb_spec (tree val, int *index) @@ -134,11 +137,77 @@ c_parser_gimple_parse_bb_spec (tree val, int *index) return *index > 0; } +/* See if VAL is an identifier matching __BB<num> and return <num> + in *INDEX. Return true if so and parse also FREQUENCY of + the edge. */ + + +static bool +c_parser_gimple_parse_bb_spec_edge_probability (tree val, + gimple_parser &parser, + int *index, + profile_probability *probablity) +{ + bool return_p = c_parser_gimple_parse_bb_spec (val, index); + if (return_p) + { + *probablity = profile_probability::uninitialized (); + /* Parse frequency if provided. */ + if (c_parser_next_token_is (parser, CPP_OPEN_PAREN)) + { + tree f; + c_parser_consume_token (parser); + if (!c_parser_next_token_is (parser, CPP_NAME)) + { + c_parser_error (parser, "expected frequency quality"); + return false; + } + + profile_quality quality; + const char *v + = IDENTIFIER_POINTER (c_parser_peek_token (parser)->value); + if (!parse_profile_quality (v, &quality)) + { + c_parser_error (parser, "unknown profile quality"); + return false; + } + + c_parser_consume_token (parser); + if (!c_parser_require (parser, CPP_OPEN_PAREN, "expected %<(%>")) + return false; + + if (!c_parser_next_token_is (parser, CPP_NUMBER) + || (TREE_CODE (f = c_parser_peek_token (parser)->value) + != INTEGER_CST)) + { + c_parser_error (parser, "expected frequency value"); + return false; + } + + unsigned int value = TREE_INT_CST_LOW (f); + *probablity = profile_probability (value, quality); + + c_parser_consume_token (parser); + if (!c_parser_require (parser, CPP_CLOSE_PAREN, "expected %<)%>")) + return false; + + if (!c_parser_require (parser, CPP_CLOSE_PAREN, "expected %<)%>")) + return false; + } + + return true; + } + + return false; + +} + /* Parse the body of a function declaration marked with "__GIMPLE". */ void c_parser_parse_gimple_body (c_parser *cparser, char *gimple_pass, - enum c_declspec_il cdil) + enum c_declspec_il cdil, + profile_count entry_bb_count) { gimple_parser parser (cparser); gimple_seq seq = NULL; @@ -209,9 +278,12 @@ c_parser_parse_gimple_body (c_parser *cparser, char *gimple_pass, add_local_decl (cfun, var); /* We have a CFG. Build the edges. */ for (unsigned i = 0; i < parser.edges.length (); ++i) - make_edge (BASIC_BLOCK_FOR_FN (cfun, parser.edges[i].src), - BASIC_BLOCK_FOR_FN (cfun, parser.edges[i].dest), - parser.edges[i].flags); + { + edge e = make_edge (BASIC_BLOCK_FOR_FN (cfun, parser.edges[i].src), + BASIC_BLOCK_FOR_FN (cfun, parser.edges[i].dest), + parser.edges[i].flags); + e->probability = parser.edges[i].probability; + } /* Add edges for case labels. */ basic_block bb; FOR_EACH_BB_FN (bb, cfun) @@ -274,6 +346,11 @@ c_parser_parse_gimple_body (c_parser *cparser, char *gimple_pass, fix_loop_structure (NULL); } + if (cfun->curr_properties & PROP_cfg) + { + ENTRY_BLOCK_PTR_FOR_FN (cfun)->count = entry_bb_count; + update_max_bb_count (); + } dump_function (TDI_gimple, current_function_decl); } @@ -337,11 +414,9 @@ c_parser_gimple_compound_statement (gimple_parser &parser, gimple_seq *seq) c_parser_consume_token (parser); if (c_parser_next_token_is (parser, CPP_NAME)) { - c_parser_gimple_goto_stmt (parser, loc, - c_parser_peek_token - (parser)->value, - seq); + tree label = c_parser_peek_token (parser)->value; c_parser_consume_token (parser); + c_parser_gimple_goto_stmt (parser, loc, label, seq); if (! c_parser_require (parser, CPP_SEMICOLON, "expected %<;%>")) return return_p; @@ -355,7 +430,8 @@ c_parser_gimple_compound_statement (gimple_parser &parser, gimple_seq *seq) "expected %<;%>")) return return_p; if (cfun->curr_properties & PROP_cfg) - parser.push_edge (parser.current_bb->index, EXIT_BLOCK, 0); + parser.push_edge (parser.current_bb->index, EXIT_BLOCK, 0, + profile_probability::uninitialized ()); break; default: goto expr_stmt; @@ -397,6 +473,7 @@ c_parser_gimple_compound_statement (gimple_parser &parser, gimple_seq *seq) return return_p; } int is_loop_header_of = -1; + profile_count bb_count = profile_count::uninitialized (); c_parser_consume_token (parser); while (c_parser_next_token_is (parser, CPP_COMMA)) { @@ -430,10 +507,39 @@ c_parser_gimple_compound_statement (gimple_parser &parser, gimple_seq *seq) "expected %<)%>")) return return_p; } + /* Parse profile: quality(value) */ else { - c_parser_error (parser, "unknown block specifier"); - return return_p; + tree q; + profile_quality quality; + tree v = c_parser_peek_token (parser)->value; + if (!parse_profile_quality (IDENTIFIER_POINTER (v), + &quality)) + { + c_parser_error (parser, "unknown block specifier"); + return false; + } + + c_parser_consume_token (parser); + if (!c_parser_require (parser, CPP_OPEN_PAREN, + "expected %<(%>")) + return false; + + if (!c_parser_next_token_is (parser, CPP_NUMBER) + || (TREE_CODE (q = c_parser_peek_token (parser)->value) + != INTEGER_CST)) + { + c_parser_error (parser, "expected count value"); + return false; + } + + bb_count + = profile_count::from_gcov_type (TREE_INT_CST_LOW (q), + quality); + c_parser_consume_token (parser); + if (! c_parser_require (parser, CPP_CLOSE_PAREN, + "expected %<)%>")) + return return_p; } } if (! c_parser_require (parser, CPP_CLOSE_PAREN, @@ -470,7 +576,8 @@ c_parser_gimple_compound_statement (gimple_parser &parser, gimple_seq *seq) last_basic_block_for_fn (cfun) = index + 1; n_basic_blocks_for_fn (cfun)++; if (!parser.current_bb) - parser.push_edge (ENTRY_BLOCK, bb->index, EDGE_FALLTHRU); + parser.push_edge (ENTRY_BLOCK, bb->index, EDGE_FALLTHRU, + profile_probability::uninitialized ()); /* We leave the proper setting to fixup. */ struct loop *loop_father = loops_for_fn (cfun)->tree_root; @@ -498,6 +605,7 @@ c_parser_gimple_compound_statement (gimple_parser &parser, gimple_seq *seq) loop_father = get_loop (cfun, is_loop_header_of); } bb->loop_father = loop_father; + bb->count = bb_count; /* Stmts now go to the new block. */ parser.current_bb = bb; @@ -1609,8 +1717,10 @@ c_parser_gimple_or_rtl_pass_list (c_parser *parser, c_declspecs *specs) return; c_parser_consume_token (parser); + specs->entry_bb_count = profile_count::uninitialized (); while (c_parser_next_token_is (parser, CPP_NAME)) { + profile_quality quality; const char *op = IDENTIFIER_POINTER (c_parser_peek_token (parser)->value); c_parser_consume_token (parser); if (! strcmp (op, "startwith")) @@ -1629,6 +1739,26 @@ c_parser_gimple_or_rtl_pass_list (c_parser *parser, c_declspecs *specs) if (! c_parser_require (parser, CPP_CLOSE_PAREN, "expected %<(%>")) return; } + else if (parse_profile_quality (op, &quality)) + { + tree q; + if (!c_parser_require (parser, CPP_OPEN_PAREN, "expected %<(%>")) + return; + + if (!c_parser_next_token_is (parser, CPP_NUMBER) + || (TREE_CODE (q = c_parser_peek_token (parser)->value) + != INTEGER_CST)) + { + c_parser_error (parser, "expected count value"); + return; + } + + specs->entry_bb_count + = profile_count::from_gcov_type (TREE_INT_CST_LOW (q), quality); + c_parser_consume_token (parser); + if (!c_parser_require (parser, CPP_CLOSE_PAREN, "expected %<)%>")) + return; + } else if (specs->declspec_il != cdil_gimple) /* Allow only one IL specifier and none on RTL. */ ; @@ -1757,10 +1887,12 @@ c_parser_gimple_goto_stmt (gimple_parser &parser, if (cfun->curr_properties & PROP_cfg) { int dest_index; - if (c_parser_gimple_parse_bb_spec (label, &dest_index)) + profile_probability prob; + if (c_parser_gimple_parse_bb_spec_edge_probability (label, parser, + &dest_index, &prob)) { parser.push_edge (parser.current_bb->index, dest_index, - EDGE_FALLTHRU); + EDGE_FALLTHRU, prob); return; } } @@ -1811,10 +1943,12 @@ c_parser_gimple_if_stmt (gimple_parser &parser, gimple_seq *seq) label = c_parser_peek_token (parser)->value; c_parser_consume_token (parser); int dest_index; + profile_probability prob; if ((cfun->curr_properties & PROP_cfg) - && c_parser_gimple_parse_bb_spec (label, &dest_index)) + && c_parser_gimple_parse_bb_spec_edge_probability (label, parser, + &dest_index, &prob)) parser.push_edge (parser.current_bb->index, dest_index, - EDGE_TRUE_VALUE); + EDGE_TRUE_VALUE, prob); else t_label = lookup_label_for_goto (loc, label); if (! c_parser_require (parser, CPP_SEMICOLON, "expected %<;%>")) @@ -1844,14 +1978,16 @@ c_parser_gimple_if_stmt (gimple_parser &parser, gimple_seq *seq) return; } label = c_parser_peek_token (parser)->value; + c_parser_consume_token (parser); int dest_index; + profile_probability prob; if ((cfun->curr_properties & PROP_cfg) - && c_parser_gimple_parse_bb_spec (label, &dest_index)) + && c_parser_gimple_parse_bb_spec_edge_probability (label, parser, + &dest_index, &prob)) parser.push_edge (parser.current_bb->index, dest_index, - EDGE_FALSE_VALUE); + EDGE_FALSE_VALUE, prob); else f_label = lookup_label_for_goto (loc, label); - c_parser_consume_token (parser); if (! c_parser_require (parser, CPP_SEMICOLON, "expected %<;%>")) return; } diff --git a/gcc/c/gimple-parser.h b/gcc/c/gimple-parser.h index 383ad768759..cc28c0f7bf9 100644 --- a/gcc/c/gimple-parser.h +++ b/gcc/c/gimple-parser.h @@ -22,7 +22,8 @@ along with GCC; see the file COPYING3. If not see /* Gimple parsing functions. */ extern void c_parser_parse_gimple_body (c_parser *, char *, - enum c_declspec_il); + enum c_declspec_il, + profile_count); extern void c_parser_gimple_or_rtl_pass_list (c_parser *, c_declspecs *); #endif diff --git a/gcc/gimple-pretty-print.c b/gcc/gimple-pretty-print.c index 69bae0d10d0..7e3916bff86 100644 --- a/gcc/gimple-pretty-print.c +++ b/gcc/gimple-pretty-print.c @@ -2704,6 +2704,10 @@ dump_gimple_bb_header (FILE *outf, basic_block bb, int indent, fprintf (outf, "%*s__BB(%d", indent, "", bb->index); if (bb->loop_father->header == bb) fprintf (outf, ",loop_header(%d)", bb->loop_father->num); + if (bb->count.initialized_p ()) + fprintf (outf, ",%s(%d)", + profile_quality_as_string (bb->count.quality ()), + bb->count.value ()); fprintf (outf, "):\n"); } else @@ -2760,6 +2764,15 @@ pp_cfg_jump (pretty_printer *buffer, edge e, dump_flags_t flags) { pp_string (buffer, "goto __BB"); pp_decimal_int (buffer, e->dest->index); + if (e->probability.initialized_p ()) + { + pp_string (buffer, "("); + pp_string (buffer, + profile_quality_as_string (e->probability.quality ())); + pp_string (buffer, "("); + pp_decimal_int (buffer, e->probability.value ()); + pp_string (buffer, "))"); + } pp_semicolon (buffer); } else diff --git a/gcc/params.def b/gcc/params.def index 3c9c5fc0f13..840b81f43cc 100644 --- a/gcc/params.def +++ b/gcc/params.def @@ -1414,6 +1414,12 @@ DEFPARAM(PARAM_LOOP_VERSIONING_MAX_OUTER_INSNS, " loops.", 100, 0, 0) +DEFPARAM(PARAM_GIMPLE_FE_COMPUTED_HOT_BB_THRESHOLD, + "gimple-fe-computed-hot-bb-threshold", + "The number of executions of a basic block which is considered hot." + " The parameters is used only in GIMPLE FE.", + 0, 0, 0) + /* Local variables: diff --git a/gcc/predict.c b/gcc/predict.c index b010c20ff7d..12a484a799a 100644 --- a/gcc/predict.c +++ b/gcc/predict.c @@ -132,8 +132,11 @@ get_hot_bb_threshold () { if (min_count == -1) { - min_count - = profile_info->sum_max / PARAM_VALUE (HOT_BB_COUNT_FRACTION); + if (flag_gimple) + min_count = PARAM_VALUE (PARAM_GIMPLE_FE_COMPUTED_HOT_BB_THRESHOLD); + else + min_count + = profile_info->sum_max / PARAM_VALUE (HOT_BB_COUNT_FRACTION); if (dump_file) fprintf (dump_file, "Setting hotness threshold to %" PRId64 ".\n", min_count); @@ -141,6 +144,14 @@ get_hot_bb_threshold () return min_count; } +/* Determine the threshold for hot BB counts and do not set the value. */ + +gcov_type +get_current_hot_bb_threshold () +{ + return min_count; +} + /* Set the threshold for hot BB counts. */ void diff --git a/gcc/predict.h b/gcc/predict.h index c1f2f0307dd..fe87dd6fc92 100644 --- a/gcc/predict.h +++ b/gcc/predict.h @@ -52,6 +52,7 @@ enum prediction extern profile_probability split_branch_probability; extern gcov_type get_hot_bb_threshold (void); +extern gcov_type get_current_hot_bb_threshold (void); extern void set_hot_bb_threshold (gcov_type); extern bool maybe_hot_count_p (struct function *, profile_count); extern bool maybe_hot_bb_p (struct function *, const_basic_block); diff --git a/gcc/profile-count.c b/gcc/profile-count.c index 8c58f8666f0..4f0bac002d7 100644 --- a/gcc/profile-count.c +++ b/gcc/profile-count.c @@ -33,34 +33,57 @@ along with GCC; see the file COPYING3. If not see #include "wide-int.h" #include "sreal.h" +/* Names from profile_quality enum values. */ + +const char *profile_quality_names[] = +{ + "uninitialized", + "guessed_local", + "guessed_global0", + "guessed_global0adjusted", + "guessed", + "afdo", + "adjusted", + "precise" +}; + /* Get a string describing QUALITY. */ const char * profile_quality_as_string (enum profile_quality quality) { - switch (quality) - { - default: - gcc_unreachable (); - case profile_uninitialized: - return "uninitialized"; - case profile_guessed_local: - return "guessed_local"; - case profile_guessed_global0: - return "guessed_global0"; - case profile_guessed_global0adjusted: - return "guessed_global0adjusted"; - case profile_guessed: - return "guessed"; - case profile_afdo: - return "afdo"; - case profile_adjusted: - return "adjusted"; - case profile_precise: - return "precise"; - } + return profile_quality_names[quality]; +} + +/* Parse VALUE as profile quality and return true when a valid QUALITY. */ + +bool +parse_profile_quality (const char *value, profile_quality *quality) +{ + for (unsigned i = 0; i < ARRAY_SIZE (profile_quality_names); i++) + if (strcmp (profile_quality_names[i], value) == 0) + { + *quality = (profile_quality)i; + return true; + } + + return false; } +/* Display names from profile_quality enum values. */ + +const char *profile_quality_display_names[] = +{ + NULL, + "estimated locally", + "estimated locally, globally 0", + "estimated locally, globally 0 adjusted", + "adjusted", + "auto FDO", + "guessed", + "precise" +}; + /* Dump THIS to F. */ void @@ -69,23 +92,8 @@ profile_count::dump (FILE *f) const if (!initialized_p ()) fprintf (f, "uninitialized"); else - { - fprintf (f, "%" PRId64, m_val); - if (m_quality == profile_guessed_local) - fprintf (f, " (estimated locally)"); - else if (m_quality == profile_guessed_global0) - fprintf (f, " (estimated locally, globally 0)"); - else if (m_quality == profile_guessed_global0adjusted) - fprintf (f, " (estimated locally, globally 0 adjusted)"); - else if (m_quality == profile_adjusted) - fprintf (f, " (adjusted)"); - else if (m_quality == profile_afdo) - fprintf (f, " (auto FDO)"); - else if (m_quality == profile_guessed) - fprintf (f, " (guessed)"); - else if (m_quality == profile_precise) - fprintf (f, " (precise)"); - } + fprintf (f, "%" PRId64 " (%s)", m_val, + profile_quality_display_names[m_quality]); } /* Dump THIS to stderr. */ @@ -362,7 +370,7 @@ profile_count::combine_with_ipa_count (profile_count ipa) Conversions back and forth are used to read the coverage and get it into internal representation. */ profile_count -profile_count::from_gcov_type (gcov_type v) +profile_count::from_gcov_type (gcov_type v, profile_quality quality) { profile_count ret; gcc_checking_assert (v >= 0); @@ -371,7 +379,7 @@ profile_count::from_gcov_type (gcov_type v) "Capping gcov count %" PRId64 " to max_count %" PRId64 "\n", (int64_t) v, (int64_t) max_count); ret.m_val = MIN (v, (gcov_type)max_count); - ret.m_quality = profile_precise; + ret.m_quality = quality; return ret; } diff --git a/gcc/profile-count.h b/gcc/profile-count.h index d6de61f0a61..2669f0d5fff 100644 --- a/gcc/profile-count.h +++ b/gcc/profile-count.h @@ -60,6 +60,8 @@ enum profile_quality { }; extern const char *profile_quality_as_string (enum profile_quality); +extern bool parse_profile_quality (const char *value, + profile_quality *quality); /* The base value for branch probability notes and edge probabilities. */ #define REG_BR_PROB_BASE 10000 @@ -149,6 +151,13 @@ class GTY((user)) profile_probability friend class profile_count; public: + profile_probability (): m_val (uninitialized_probability), + m_quality (profile_guessed) + {} + + profile_probability (uint32_t val, profile_quality quality): + m_val (val), m_quality (quality) + {} /* Named probabilities. */ static profile_probability never () @@ -558,6 +567,12 @@ public: return initialized_p () && other.initialized_p () && m_val >= other.m_val; } + /* Get the value of the count. */ + uint32_t value () const { return m_val; } + + /* Get the quality of the count. */ + enum profile_quality quality () const { return m_quality; } + /* Output THIS to F. */ void dump (FILE *f) const; @@ -675,7 +690,6 @@ private: return ipa_p () == other.ipa_p (); } public: - /* Used for counters which are expected to be never executed. */ static profile_count zero () { @@ -737,6 +751,9 @@ public: return m_quality == profile_precise; } + /* Get the value of the count. */ + uint32_t value () const { return m_val; } + /* Get the quality of the count. */ enum profile_quality quality () const { return m_quality; } @@ -1136,7 +1153,8 @@ public: /* The profiling runtime uses gcov_type, which is usually 64bit integer. Conversions back and forth are used to read the coverage and get it into internal representation. */ - static profile_count from_gcov_type (gcov_type v); + static profile_count from_gcov_type (gcov_type v, + profile_quality quality = profile_precise); /* LTO streaming support. */ static profile_count stream_in (struct lto_input_block *); diff --git a/gcc/testsuite/gcc.dg/gimplefe-37.c b/gcc/testsuite/gcc.dg/gimplefe-37.c new file mode 100644 index 00000000000..d3ea186d7f9 --- /dev/null +++ b/gcc/testsuite/gcc.dg/gimplefe-37.c @@ -0,0 +1,27 @@ +/* { dg-do compile } */ +/* { dg-options "-O2 -fgimple -fdump-tree-optimized --param=gimple-fe-computed-hot-bb-threshold=10 " } */ + +int __GIMPLE (ssa,startwith("slsr"),precise(3)) +main (int argc) +{ + int _1; + + __BB(2,precise(3)): + if (argc_2(D) == 2) + goto __BB3(precise(44739243)); + else + goto __BB4(precise(89478485)); + + __BB(3,precise(1)): + goto __BB4(precise(134217728)); + + __BB(4,precise(3)): + _1 = __PHI (__BB2: 0, __BB3: 12); + return _1; +} + + +/* { dg-final { scan-tree-dump-times "<bb \[0-9\]> \\\[count: 3" 2 "optimized" } } */ +/* { dg-final { scan-tree-dump-times "<bb \[0-9\]> \\\[count: 2" 1 "optimized" } } */ +/* { dg-final { scan-tree-dump-times "goto <bb \[0-9\]>; \\\[33\\\.33%\\\]" 1 "optimized" } } */ +/* { dg-final { scan-tree-dump-times "goto <bb \[0-9\]>; \\\[66\\\.67%\\\]" 1 "optimized" } } */ diff --git a/gcc/testsuite/gcc.dg/gimplefe-38.c b/gcc/testsuite/gcc.dg/gimplefe-38.c new file mode 100644 index 00000000000..64532d87da5 --- /dev/null +++ b/gcc/testsuite/gcc.dg/gimplefe-38.c @@ -0,0 +1,27 @@ +/* { dg-do compile } */ +/* { dg-options "-O2 -fgimple -fdump-tree-slsr" } */ + +int __GIMPLE (ssa,startwith("slsr"),guessed_local(1073741824)) +main (int argc) +{ + int _1; + + __BB(2,guessed_local(1073741824)): + if (argc_2(D) == 2) + goto __BB3(guessed(16777216)); + else + goto __BB4(guessed(117440512)); + + __BB(3,guessed_local(134217728)): + goto __BB4(precise(134217728)); + + __BB(4,guessed_local(1073741824)): + _1 = __PHI (__BB2: 0, __BB3: 12); + return _1; +} + + +/* { dg-final { scan-tree-dump-times "<bb \[0-9\]> \\\[local count: 1073741824" 2 "slsr" } } */ +/* { dg-final { scan-tree-dump-times "<bb \[0-9\]> \\\[local count: 134217728" 1 "slsr" } } */ +/* { dg-final { scan-tree-dump-times "goto <bb \[0-9\]>; \\\[12\\\.50%\\\]" 1 "slsr" } } */ +/* { dg-final { scan-tree-dump-times "goto <bb \[0-9\]>; \\\[87\\\.50%\\\]" 1 "slsr" } } */ diff --git a/gcc/tree-cfg.c b/gcc/tree-cfg.c index 587408150fb..f7ebac674db 100644 --- a/gcc/tree-cfg.c +++ b/gcc/tree-cfg.c @@ -7872,13 +7872,32 @@ dump_function_to_file (tree fndecl, FILE *file, dump_flags_t flags) current_function_decl = fndecl; if (flags & TDF_GIMPLE) { + static bool hotness_bb_param_printed = false; + if (get_current_hot_bb_threshold () != -1 + && !hotness_bb_param_printed) + { + hotness_bb_param_printed = true; + fprintf (file, + "/* --param=gimple-fe-computed-hot-bb-threshold=%" PRId64 + " */\n", get_current_hot_bb_threshold ()); + } + print_generic_expr (file, TREE_TYPE (TREE_TYPE (fndecl)), dump_flags | TDF_SLIM); - fprintf (file, " __GIMPLE (%s)\n%s (", + fprintf (file, " __GIMPLE (%s", (fun->curr_properties & PROP_ssa) ? "ssa" : (fun->curr_properties & PROP_cfg) ? "cfg" - : "", - function_name (fun)); + : ""); + + if (cfun->cfg) + { + basic_block bb = ENTRY_BLOCK_PTR_FOR_FN (cfun); + if (bb->count.initialized_p ()) + fprintf (file, ",%s(%d)", + profile_quality_as_string (bb->count.quality ()), + bb->count.value ()); + fprintf (file, ")\n%s (", function_name (fun)); + } } else fprintf (file, "%s %s(", function_name (fun), tmclone ? "[tm-clone] " : ""); -- 2.21.0 ^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH 2/2] Support {MIN,MAX}_EXPR in GIMPLE FE. 2019-05-06 7:59 ` Martin Liška @ 2019-05-06 8:00 ` Martin Liška 2019-05-06 11:35 ` Richard Biener 2019-05-06 14:02 ` [PATCH][stage1] Support profile (BB counts and edge probabilities) " Richard Biener 1 sibling, 1 reply; 22+ messages in thread From: Martin Liška @ 2019-05-06 8:00 UTC (permalink / raw) To: Richard Biener; +Cc: Joseph S. Myers, Jan Hubicka, GCC Patches [-- Attachment #1: Type: text/plain, Size: 165 bytes --] Hi. The patch is about support of a new GIMPLE expr. Patch can bootstrap on x86_64-linux-gnu and survives regression tests. Ready to be installed? Thanks, Martin [-- Attachment #2: 0002-Support-MIN-MAX-_EXPR-in-GIMPLE-FE.patch --] [-- Type: text/x-patch, Size: 4938 bytes --] From a4f23fe8d876f2d8a0628feb57127e276348aab0 Mon Sep 17 00:00:00 2001 From: marxin <mliska@suse.cz> Date: Fri, 3 May 2019 13:54:40 +0200 Subject: [PATCH 2/2] Support {MIN,MAX}_EXPR in GIMPLE FE. gcc/ChangeLog: 2019-05-03 Martin Liska <mliska@suse.cz> * gimple-pretty-print.c (dump_binary_rhs): Dump MIN_EXPR and MAX_EXPR in GIMPLE FE format. gcc/c/ChangeLog: 2019-05-03 Martin Liska <mliska@suse.cz> * c-typeck.c (build_binary_op): Handle MIN_EXPR and MAX_EXPR with flag_gimple. * gimple-parser.c (c_parser_gimple_statement): Support __MIN and __MAX. (c_parser_gimple_unary_expression): Parse also binary expression __MIN and __MAX. gcc/testsuite/ChangeLog: 2019-05-03 Martin Liska <mliska@suse.cz> * gcc.dg/gimplefe-39.c: New test. --- gcc/c/c-typeck.c | 10 ++++++++++ gcc/c/gimple-parser.c | 21 ++++++++++++++++++++- gcc/gimple-pretty-print.c | 15 ++++++++++++++- gcc/testsuite/gcc.dg/gimplefe-39.c | 21 +++++++++++++++++++++ 4 files changed, 65 insertions(+), 2 deletions(-) create mode 100644 gcc/testsuite/gcc.dg/gimplefe-39.c diff --git a/gcc/c/c-typeck.c b/gcc/c/c-typeck.c index 4e443754002..d0ae9c51f6b 100644 --- a/gcc/c/c-typeck.c +++ b/gcc/c/c-typeck.c @@ -12193,6 +12193,16 @@ build_binary_op (location_t location, enum tree_code code, maybe_warn_bool_compare (location, code, orig_op0, orig_op1); break; + case MIN_EXPR: + case MAX_EXPR: + if (flag_gimple) + { + result_type = c_common_type (type0, type1); + break; + } + else + gcc_unreachable (); + default: gcc_unreachable (); } diff --git a/gcc/c/gimple-parser.c b/gcc/c/gimple-parser.c index 6558770873f..f3e0d2f94ad 100644 --- a/gcc/c/gimple-parser.c +++ b/gcc/c/gimple-parser.c @@ -747,7 +747,9 @@ c_parser_gimple_statement (gimple_parser &parser, gimple_seq *seq) { tree id = c_parser_peek_token (parser)->value; if (strcmp (IDENTIFIER_POINTER (id), "__ABS") == 0 - || strcmp (IDENTIFIER_POINTER (id), "__ABSU") == 0) + || strcmp (IDENTIFIER_POINTER (id), "__ABSU") == 0 + || strcmp (IDENTIFIER_POINTER (id), "__MIN") == 0 + || strcmp (IDENTIFIER_POINTER (id), "__MAX") == 0) goto build_unary_expr; break; } @@ -1062,6 +1064,7 @@ c_parser_gimple_unary_expression (gimple_parser &parser) } case CPP_NAME: { + bool is_min; tree id = c_parser_peek_token (parser)->value; if (strcmp (IDENTIFIER_POINTER (id), "__ABS") == 0) { @@ -1075,6 +1078,22 @@ c_parser_gimple_unary_expression (gimple_parser &parser) op = c_parser_gimple_postfix_expression (parser); return parser_build_unary_op (op_loc, ABSU_EXPR, op); } + else if ((is_min = strcmp (IDENTIFIER_POINTER (id), "__MIN") == 0) + || strcmp (IDENTIFIER_POINTER (id), "__MAX") == 0) + { + c_parser_consume_token (parser); + if (!c_parser_require (parser, CPP_OPEN_PAREN, "expected %<(%>")) + return ret; + c_expr op1 = c_parser_gimple_postfix_expression (parser); + if (!c_parser_require (parser, CPP_COMMA, "expected %<,%>")) + return ret; + c_expr op2 = c_parser_gimple_postfix_expression (parser); + if (!c_parser_require (parser, CPP_CLOSE_PAREN, "expected %<)%>")) + return ret; + return parser_build_binary_op (op_loc, + is_min ? MIN_EXPR : MAX_EXPR, + op1, op2); + } else return c_parser_gimple_postfix_expression (parser); } diff --git a/gcc/gimple-pretty-print.c b/gcc/gimple-pretty-print.c index 7e3916bff86..58212c4dcc1 100644 --- a/gcc/gimple-pretty-print.c +++ b/gcc/gimple-pretty-print.c @@ -423,9 +423,22 @@ dump_binary_rhs (pretty_printer *buffer, gassign *gs, int spc, enum tree_code code = gimple_assign_rhs_code (gs); switch (code) { - case COMPLEX_EXPR: case MIN_EXPR: case MAX_EXPR: + if (flags & TDF_GIMPLE) + { + pp_string (buffer, code == MIN_EXPR ? "__MIN (" : "__MAX ("); + dump_generic_node (buffer, gimple_assign_rhs1 (gs), spc, flags, + false); + pp_string (buffer, ", "); + dump_generic_node (buffer, gimple_assign_rhs2 (gs), spc, flags, + false); + pp_string (buffer, ")"); + break; + } + else + gcc_fallthrough (); + case COMPLEX_EXPR: case VEC_WIDEN_MULT_HI_EXPR: case VEC_WIDEN_MULT_LO_EXPR: case VEC_WIDEN_MULT_EVEN_EXPR: diff --git a/gcc/testsuite/gcc.dg/gimplefe-39.c b/gcc/testsuite/gcc.dg/gimplefe-39.c new file mode 100644 index 00000000000..30677356d5b --- /dev/null +++ b/gcc/testsuite/gcc.dg/gimplefe-39.c @@ -0,0 +1,21 @@ +/* { dg-do compile } */ +/* { dg-options "-O2 -fgimple -fdump-tree-optimized" } */ + +int a, b; + +int __GIMPLE (ssa,guessed_local(1073741824)) +main (int argc) +{ + int _1; + int _2; + int _4; + + __BB(2,guessed_local(1073741824)): + _1 = a; + _2 = b; + _4 = __MAX (_1, _2); + return _4; + +} + +/* { dg-final { scan-tree-dump "MAX_EXPR" "optimized" } } */ -- 2.21.0 ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 2/2] Support {MIN,MAX}_EXPR in GIMPLE FE. 2019-05-06 8:00 ` [PATCH 2/2] Support {MIN,MAX}_EXPR " Martin Liška @ 2019-05-06 11:35 ` Richard Biener 2019-05-07 12:01 ` Martin Liška 0 siblings, 1 reply; 22+ messages in thread From: Richard Biener @ 2019-05-06 11:35 UTC (permalink / raw) To: Martin Liška; +Cc: Joseph S. Myers, Jan Hubicka, GCC Patches On Mon, May 6, 2019 at 10:00 AM Martin Liška <mliska@suse.cz> wrote: > > Hi. > > The patch is about support of a new GIMPLE expr. > > Patch can bootstrap on x86_64-linux-gnu and survives regression tests. > > Ready to be installed? Can you please avoid using/changing parser_build_binary_op? The other binary expression handling just does if (lhs.value != error_mark_node && rhs.value != error_mark_node) ret.value = build2_loc (ret_loc, code, ret_type, lhs.value, rhs.value); which should work equally well here. I think for future expansion splitting out the ( op, op ) parsing and expression building into a function might be nicer so that c_parser_gimple_unary_expression reads if (strcmp (INDENTIFIER_POINTER (id), "__MIN") == 0) return c_parser_gimple_parentized_binary_expression (op_loc, MIN_EXPR); else if (...) OK with such change/factoring. Thanks, Richard. > Thanks, > Martin ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 2/2] Support {MIN,MAX}_EXPR in GIMPLE FE. 2019-05-06 11:35 ` Richard Biener @ 2019-05-07 12:01 ` Martin Liška 2019-05-07 12:58 ` Richard Biener 0 siblings, 1 reply; 22+ messages in thread From: Martin Liška @ 2019-05-07 12:01 UTC (permalink / raw) To: Richard Biener; +Cc: Joseph S. Myers, Jan Hubicka, GCC Patches [-- Attachment #1: Type: text/plain, Size: 1056 bytes --] On 5/6/19 1:35 PM, Richard Biener wrote: > On Mon, May 6, 2019 at 10:00 AM Martin LiÅ¡ka <mliska@suse.cz> wrote: >> >> Hi. >> >> The patch is about support of a new GIMPLE expr. >> >> Patch can bootstrap on x86_64-linux-gnu and survives regression tests. >> >> Ready to be installed? > > Can you please avoid using/changing parser_build_binary_op? The other > binary expression handling just does > > if (lhs.value != error_mark_node && rhs.value != error_mark_node) > ret.value = build2_loc (ret_loc, code, ret_type, lhs.value, rhs.value); > > which should work equally well here. I think for future expansion > splitting out the ( op, op ) parsing and expression building into > a function might be nicer so that c_parser_gimple_unary_expression > reads > > if (strcmp (INDENTIFIER_POINTER (id), "__MIN") == 0) > return c_parser_gimple_parentized_binary_expression (op_loc, MIN_EXPR); > else if (...) > > OK with such change/factoring. I've done all what you pointed out. Martin > > Thanks, > Richard. > >> Thanks, >> Martin [-- Attachment #2: 0002-Support-MIN-MAX-_EXPR-in-GIMPLE-FE.patch --] [-- Type: text/x-patch, Size: 4775 bytes --] From fe7fc3a153e404c485fa1d8dcd428c4a8ebc8f67 Mon Sep 17 00:00:00 2001 From: marxin <mliska@suse.cz> Date: Fri, 3 May 2019 13:54:40 +0200 Subject: [PATCH 2/2] Support {MIN,MAX}_EXPR in GIMPLE FE. gcc/ChangeLog: 2019-05-03 Martin Liska <mliska@suse.cz> * gimple-pretty-print.c (dump_binary_rhs): Dump MIN_EXPR and MAX_EXPR in GIMPLE FE format. gcc/c/ChangeLog: 2019-05-03 Martin Liska <mliska@suse.cz> * gimple-parser.c (c_parser_gimple_statement): Support __MIN and __MAX. (c_parser_gimple_unary_expression): Parse also binary expression __MIN and __MAX. (c_parser_gimple_parentized_binary_expression): New function. gcc/testsuite/ChangeLog: 2019-05-03 Martin Liska <mliska@suse.cz> * gcc.dg/gimplefe-39.c: New test. --- gcc/c/gimple-parser.c | 38 +++++++++++++++++++++++++++++- gcc/gimple-pretty-print.c | 15 +++++++++++- gcc/testsuite/gcc.dg/gimplefe-39.c | 21 +++++++++++++++++ 3 files changed, 72 insertions(+), 2 deletions(-) create mode 100644 gcc/testsuite/gcc.dg/gimplefe-39.c diff --git a/gcc/c/gimple-parser.c b/gcc/c/gimple-parser.c index ede5a927c3d..99f764710b2 100644 --- a/gcc/c/gimple-parser.c +++ b/gcc/c/gimple-parser.c @@ -750,7 +750,9 @@ c_parser_gimple_statement (gimple_parser &parser, gimple_seq *seq) { tree id = c_parser_peek_token (parser)->value; if (strcmp (IDENTIFIER_POINTER (id), "__ABS") == 0 - || strcmp (IDENTIFIER_POINTER (id), "__ABSU") == 0) + || strcmp (IDENTIFIER_POINTER (id), "__ABSU") == 0 + || strcmp (IDENTIFIER_POINTER (id), "__MIN") == 0 + || strcmp (IDENTIFIER_POINTER (id), "__MAX") == 0) goto build_unary_expr; break; } @@ -989,6 +991,32 @@ c_parser_gimple_binary_expression (gimple_parser &parser) return ret; } +/* Parse a gimple parentized binary expression. */ + +static c_expr +c_parser_gimple_parentized_binary_expression (gimple_parser &parser, + location_t op_loc, + tree_code code) +{ + struct c_expr ret; + ret.set_error (); + + c_parser_consume_token (parser); + if (!c_parser_require (parser, CPP_OPEN_PAREN, "expected %<(%>")) + return ret; + c_expr op1 = c_parser_gimple_postfix_expression (parser); + if (!c_parser_require (parser, CPP_COMMA, "expected %<,%>")) + return ret; + c_expr op2 = c_parser_gimple_postfix_expression (parser); + if (!c_parser_require (parser, CPP_CLOSE_PAREN, "expected %<)%>")) + return ret; + + if (op1.value != error_mark_node && op2.value != error_mark_node) + ret.value = build2_loc (op_loc, + code, TREE_TYPE (op1.value), op1.value, op2.value); + return ret; +} + /* Parse gimple unary expression. gimple-unary-expression: @@ -1078,6 +1106,14 @@ c_parser_gimple_unary_expression (gimple_parser &parser) op = c_parser_gimple_postfix_expression (parser); return parser_build_unary_op (op_loc, ABSU_EXPR, op); } + else if (strcmp (IDENTIFIER_POINTER (id), "__MIN") == 0) + return c_parser_gimple_parentized_binary_expression (parser, + op_loc, + MIN_EXPR); + else if (strcmp (IDENTIFIER_POINTER (id), "__MAX") == 0) + return c_parser_gimple_parentized_binary_expression (parser, + op_loc, + MAX_EXPR); else return c_parser_gimple_postfix_expression (parser); } diff --git a/gcc/gimple-pretty-print.c b/gcc/gimple-pretty-print.c index 7e3916bff86..58212c4dcc1 100644 --- a/gcc/gimple-pretty-print.c +++ b/gcc/gimple-pretty-print.c @@ -423,9 +423,22 @@ dump_binary_rhs (pretty_printer *buffer, gassign *gs, int spc, enum tree_code code = gimple_assign_rhs_code (gs); switch (code) { - case COMPLEX_EXPR: case MIN_EXPR: case MAX_EXPR: + if (flags & TDF_GIMPLE) + { + pp_string (buffer, code == MIN_EXPR ? "__MIN (" : "__MAX ("); + dump_generic_node (buffer, gimple_assign_rhs1 (gs), spc, flags, + false); + pp_string (buffer, ", "); + dump_generic_node (buffer, gimple_assign_rhs2 (gs), spc, flags, + false); + pp_string (buffer, ")"); + break; + } + else + gcc_fallthrough (); + case COMPLEX_EXPR: case VEC_WIDEN_MULT_HI_EXPR: case VEC_WIDEN_MULT_LO_EXPR: case VEC_WIDEN_MULT_EVEN_EXPR: diff --git a/gcc/testsuite/gcc.dg/gimplefe-39.c b/gcc/testsuite/gcc.dg/gimplefe-39.c new file mode 100644 index 00000000000..30677356d5b --- /dev/null +++ b/gcc/testsuite/gcc.dg/gimplefe-39.c @@ -0,0 +1,21 @@ +/* { dg-do compile } */ +/* { dg-options "-O2 -fgimple -fdump-tree-optimized" } */ + +int a, b; + +int __GIMPLE (ssa,guessed_local(1073741824)) +main (int argc) +{ + int _1; + int _2; + int _4; + + __BB(2,guessed_local(1073741824)): + _1 = a; + _2 = b; + _4 = __MAX (_1, _2); + return _4; + +} + +/* { dg-final { scan-tree-dump "MAX_EXPR" "optimized" } } */ -- 2.21.0 ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 2/2] Support {MIN,MAX}_EXPR in GIMPLE FE. 2019-05-07 12:01 ` Martin Liška @ 2019-05-07 12:58 ` Richard Biener 0 siblings, 0 replies; 22+ messages in thread From: Richard Biener @ 2019-05-07 12:58 UTC (permalink / raw) To: Martin Liška; +Cc: Joseph S. Myers, Jan Hubicka, GCC Patches On Tue, May 7, 2019 at 2:01 PM Martin Liška <mliska@suse.cz> wrote: > > On 5/6/19 1:35 PM, Richard Biener wrote: > > On Mon, May 6, 2019 at 10:00 AM Martin Liška <mliska@suse.cz> wrote: > >> > >> Hi. > >> > >> The patch is about support of a new GIMPLE expr. > >> > >> Patch can bootstrap on x86_64-linux-gnu and survives regression tests. > >> > >> Ready to be installed? > > > > Can you please avoid using/changing parser_build_binary_op? The other > > binary expression handling just does > > > > if (lhs.value != error_mark_node && rhs.value != error_mark_node) > > ret.value = build2_loc (ret_loc, code, ret_type, lhs.value, rhs.value); > > > > which should work equally well here. I think for future expansion > > splitting out the ( op, op ) parsing and expression building into > > a function might be nicer so that c_parser_gimple_unary_expression > > reads > > > > if (strcmp (INDENTIFIER_POINTER (id), "__MIN") == 0) > > return c_parser_gimple_parentized_binary_expression (op_loc, MIN_EXPR); > > else if (...) > > > > OK with such change/factoring. > > I've done all what you pointed out. OK. Thanks, Richard. > Martin > > > > > Thanks, > > Richard. > > > >> Thanks, > >> Martin > ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH][stage1] Support profile (BB counts and edge probabilities) in GIMPLE FE. 2019-05-06 7:59 ` Martin Liška 2019-05-06 8:00 ` [PATCH 2/2] Support {MIN,MAX}_EXPR " Martin Liška @ 2019-05-06 14:02 ` Richard Biener 2019-05-07 12:00 ` Martin Liška 1 sibling, 1 reply; 22+ messages in thread From: Richard Biener @ 2019-05-06 14:02 UTC (permalink / raw) To: Martin Liška; +Cc: Joseph S. Myers, Jan Hubicka, GCC Patches On Mon, May 6, 2019 at 9:59 AM Martin Liška <mliska@suse.cz> wrote: > > On 5/2/19 2:31 PM, Richard Biener wrote: > > On Mon, Apr 29, 2019 at 2:51 PM Martin Liška <mliska@suse.cz> wrote: > >> > >> On 4/26/19 3:18 PM, Richard Biener wrote: > >>> On Wed, Apr 10, 2019 at 10:12 AM Martin Liška <mliska@suse.cz> wrote: > >>>> > >>>> On 4/9/19 3:19 PM, Jan Hubicka wrote: > >>>>>> Hi. > >>>>>> > >>>>>> There's updated version that supports profile quality for both counts > >>>>>> and probabilities. I'm wondering whether ENTRY and EXIT BBs needs to > >>>>>> have set probability. Apparently, I haven't seen any verifier that > >>>>>> would complain. > >>>>> > >>>>> Well, you do not need to define it but then several cases will > >>>>> degenerate. In particular BB frequencies (for callgraph profile or > >>>>> hot/cold decisions) are calculated as ratios of entry BB and given BB > >>>>> count. If entry BB is undefined you will get those undefined and > >>>>> heuristics will resort to conservative answers. > >>>>> > >>>>> I do not think we use exit block count. > >>>>> > >>>>> Honza > >>>>> > >>>> > >>>> Thank you Honza for explanation. I'm sending version of the patch > >>>> that supports entry BB count. > >>>> > >>>> I've been testing the patch right now. > >>> > >>> Can you move the GIMPLE/RTL FE specific data in c_declspecs to > >>> a substructure accessed via indirection? I guess enlarging that > >>> isn't really what we should do. You'd move gimple_or_rtl_pass > >>> there and make that pointer one to a struct aux_fe_data > >>> (lifetime managed by the respective RTL/GIMPLE FE, thus > >>> to be freed there)? Joseph, do you agree or is adding more > >>> stuff to c_declspecs OK (I would guess it could be a few more > >>> elements in the future). > >> > >> Let's wait here for Joseph. > > > > So looks like it won't matter so let's go with the current approach > > for the moment. > > > >>> > >>> -c_parser_gimple_parse_bb_spec (tree val, int *index) > >>> +c_parser_gimple_parse_bb_spec (tree val, gimple_parser &parser, > >>> + int *index, profile_probability *probablity) > >>> { > >>> > >>> so this will allow specifying probability in PHI node arguments. > >>> I think we want to split this into the already existing part > >>> and a c_parser_gimple_parse_bb_spec_with_edge_probability > >>> to be used at edge sources. > >> > >> Yes, that's a good idea! > >> > >>> > >>> + if (cfun->curr_properties & PROP_cfg) > >>> + { > >>> + update_max_bb_count (); > >>> + set_hot_bb_threshold (hot_bb_threshold); > >>> + ENTRY_BLOCK_PTR_FOR_FN (cfun)->count = entry_bb_count; > >>> > >>> I guess the last one should be before update_max_bb_count ()? > >> > >> Same here. > >> > >>> > >>> + } > >>> > >>> + /* Parse profile: quality(value) */ > >>> else > >>> { > >>> - c_parser_error (parser, "unknown block specifier"); > >>> - return return_p; > >>> + tree q; > >>> + profile_quality quality; > >>> + tree v = c_parser_peek_token (parser)->value; > >>> > >>> peek next token before the if/else and do > >>> > >>> else if (!strcmp (...)) > >>> > >>> as in the loop_header case. I expected we can somehow share > >>> parsing of profile quality and BB/edge count/frequency? How's > >>> the expected syntax btw, comments in the code should tell us. > >>> I'm guessing it's quality-id '(' number ')' and thus it should be > >>> really shareable between edge and BB count and also __GIMPLE > >>> header parsing? So parse_profile_quality should be > >>> parse_profile () instead, resulting in a combined value > >>> (we do use the same for edge/bb?). > >> > >> It's problematic, there are different error messages for count/frequency. > >> Moreover call to parse_profile_quality in c_parser_gimple_or_rtl_pass_list > >> is a way how to test that next 'token' is a profile count. > > > > Who cares about error messages... But sure, I'm just proposing to > > merge testing for next token and actual parsing. > > After I've done removal of hot_bb_threshold parsing, there are just 2 usages > of parse_profile_quality. I would like to leave it as it, not introducing a wrappers. > > > > >>> > >>> + else if (!strcmp (op, "hot_bb_threshold")) > >>> + { > >>> > >>> I'm not sure about this - it doesn't make sense to specify this > >>> on a per-function base since it seems to control a global > >>> variable (booo!)? > >> > >> Yep, shame on me! > >> > >>> Isn't this instead computed on-demand > >>> based on profile_info->sum_max? > >> > >> No it's a global value shared among functions. > >> > >>> If not then I think > >>> we need an alternate way of funneling in global state into > >>> the GIMPLE FE. > >> > >> What about --param gimple-fe-hot-bb-threshold ? > > > > I thought about that, yes ... in absence can it actually be > > "computed"? > > Renamed to it. > > Patch can bootstrap on x86_64-linux-gnu and survives regression tests. > > Ready to be installed? @@ -470,7 +576,8 @@ c_parser_gimple_compound_statement (gimple_parser &parser, gimple_seq *seq) last_basic_block_for_fn (cfun) = index + 1; n_basic_blocks_for_fn (cfun)++; if (!parser.current_bb) - parser.push_edge (ENTRY_BLOCK, bb->index, EDGE_FALLTHRU); + parser.push_edge (ENTRY_BLOCK, bb->index, EDGE_FALLTHRU, + profile_probability::uninitialized ()); /* We leave the proper setting to fixup. */ struct loop *loop_father = loops_for_fn (cfun)->tree_root; the fallthru edge from ENTRY to the single-succ of ENTRY should have 100% probability and be "known", no? Or do we actually use uninitialized for fallthru edges? diff --git a/gcc/params.def b/gcc/params.def index 3c9c5fc0f13..840b81f43cc 100644 --- a/gcc/params.def +++ b/gcc/params.def @@ -1414,6 +1414,12 @@ DEFPARAM(PARAM_LOOP_VERSIONING_MAX_OUTER_INSNS, " loops.", 100, 0, 0) +DEFPARAM(PARAM_GIMPLE_FE_COMPUTED_HOT_BB_THRESHOLD, + "gimple-fe-computed-hot-bb-threshold", + "The number of executions of a basic block which is considered hot." + " The parameters is used only in GIMPLE FE.", + 0, 0, 0) + /* Local variables: diff --git a/gcc/predict.c b/gcc/predict.c index b010c20ff7d..12a484a799a 100644 --- a/gcc/predict.c +++ b/gcc/predict.c @@ -132,8 +132,11 @@ get_hot_bb_threshold () { if (min_count == -1) { - min_count - = profile_info->sum_max / PARAM_VALUE (HOT_BB_COUNT_FRACTION); + if (flag_gimple) + min_count = PARAM_VALUE (PARAM_GIMPLE_FE_COMPUTED_HOT_BB_THRESHOLD); + else + min_count + = profile_info->sum_max / PARAM_VALUE (HOT_BB_COUNT_FRACTION); if (dump_file) fprintf (dump_file, "Setting hotness threshold to %" PRId64 ".\n", min_count); Ick. I'd rather do this somewhere in the frontend where PARAM_VALUE is initialized via set_hot_bb_threshold. Even redundantly for each parsed GIMPLE function would be OK and better IMHO. That way you don't need get_current_hot_bb_threshold(?). OK with that changes. We need to think about switch stmt parsing where we currently have switch (argc_2(D)) {default: L2; case 0: L0; case 1: L0; case 2: L1; } __BB(3): L0: a_4 = 0; goto __BB6; ... extending this to switch (argc_2(D)) { default: L2 (guessed(10)); ... might be possible so we have profile on the edges. After the patch all -gimple dumps have profile - probably good but also visually disturbing. Ah well. Hope to do loop flags as followup soon. Thanks, Richard. > Thanks, > Martin > > > > > Richard. > > > >> Thanks, > >> Martin > >> > >>> > >>> Thanks, > >>> Richard. > >>> > >>> > >>>> > >>>> Martin > >> > ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH][stage1] Support profile (BB counts and edge probabilities) in GIMPLE FE. 2019-05-06 14:02 ` [PATCH][stage1] Support profile (BB counts and edge probabilities) " Richard Biener @ 2019-05-07 12:00 ` Martin Liška 2019-05-07 12:56 ` Richard Biener 2019-05-10 6:39 ` Bernhard Reutner-Fischer 0 siblings, 2 replies; 22+ messages in thread From: Martin Liška @ 2019-05-07 12:00 UTC (permalink / raw) To: Richard Biener; +Cc: Joseph S. Myers, Jan Hubicka, GCC Patches [-- Attachment #1: Type: text/plain, Size: 8339 bytes --] On 5/6/19 4:02 PM, Richard Biener wrote: > On Mon, May 6, 2019 at 9:59 AM Martin LiÅ¡ka <mliska@suse.cz> wrote: >> >> On 5/2/19 2:31 PM, Richard Biener wrote: >>> On Mon, Apr 29, 2019 at 2:51 PM Martin LiÅ¡ka <mliska@suse.cz> wrote: >>>> >>>> On 4/26/19 3:18 PM, Richard Biener wrote: >>>>> On Wed, Apr 10, 2019 at 10:12 AM Martin LiÅ¡ka <mliska@suse.cz> wrote: >>>>>> >>>>>> On 4/9/19 3:19 PM, Jan Hubicka wrote: >>>>>>>> Hi. >>>>>>>> >>>>>>>> There's updated version that supports profile quality for both counts >>>>>>>> and probabilities. I'm wondering whether ENTRY and EXIT BBs needs to >>>>>>>> have set probability. Apparently, I haven't seen any verifier that >>>>>>>> would complain. >>>>>>> >>>>>>> Well, you do not need to define it but then several cases will >>>>>>> degenerate. In particular BB frequencies (for callgraph profile or >>>>>>> hot/cold decisions) are calculated as ratios of entry BB and given BB >>>>>>> count. If entry BB is undefined you will get those undefined and >>>>>>> heuristics will resort to conservative answers. >>>>>>> >>>>>>> I do not think we use exit block count. >>>>>>> >>>>>>> Honza >>>>>>> >>>>>> >>>>>> Thank you Honza for explanation. I'm sending version of the patch >>>>>> that supports entry BB count. >>>>>> >>>>>> I've been testing the patch right now. >>>>> >>>>> Can you move the GIMPLE/RTL FE specific data in c_declspecs to >>>>> a substructure accessed via indirection? I guess enlarging that >>>>> isn't really what we should do. You'd move gimple_or_rtl_pass >>>>> there and make that pointer one to a struct aux_fe_data >>>>> (lifetime managed by the respective RTL/GIMPLE FE, thus >>>>> to be freed there)? Joseph, do you agree or is adding more >>>>> stuff to c_declspecs OK (I would guess it could be a few more >>>>> elements in the future). >>>> >>>> Let's wait here for Joseph. >>> >>> So looks like it won't matter so let's go with the current approach >>> for the moment. >>> >>>>> >>>>> -c_parser_gimple_parse_bb_spec (tree val, int *index) >>>>> +c_parser_gimple_parse_bb_spec (tree val, gimple_parser &parser, >>>>> + int *index, profile_probability *probablity) >>>>> { >>>>> >>>>> so this will allow specifying probability in PHI node arguments. >>>>> I think we want to split this into the already existing part >>>>> and a c_parser_gimple_parse_bb_spec_with_edge_probability >>>>> to be used at edge sources. >>>> >>>> Yes, that's a good idea! >>>> >>>>> >>>>> + if (cfun->curr_properties & PROP_cfg) >>>>> + { >>>>> + update_max_bb_count (); >>>>> + set_hot_bb_threshold (hot_bb_threshold); >>>>> + ENTRY_BLOCK_PTR_FOR_FN (cfun)->count = entry_bb_count; >>>>> >>>>> I guess the last one should be before update_max_bb_count ()? >>>> >>>> Same here. >>>> >>>>> >>>>> + } >>>>> >>>>> + /* Parse profile: quality(value) */ >>>>> else >>>>> { >>>>> - c_parser_error (parser, "unknown block specifier"); >>>>> - return return_p; >>>>> + tree q; >>>>> + profile_quality quality; >>>>> + tree v = c_parser_peek_token (parser)->value; >>>>> >>>>> peek next token before the if/else and do >>>>> >>>>> else if (!strcmp (...)) >>>>> >>>>> as in the loop_header case. I expected we can somehow share >>>>> parsing of profile quality and BB/edge count/frequency? How's >>>>> the expected syntax btw, comments in the code should tell us. >>>>> I'm guessing it's quality-id '(' number ')' and thus it should be >>>>> really shareable between edge and BB count and also __GIMPLE >>>>> header parsing? So parse_profile_quality should be >>>>> parse_profile () instead, resulting in a combined value >>>>> (we do use the same for edge/bb?). >>>> >>>> It's problematic, there are different error messages for count/frequency. >>>> Moreover call to parse_profile_quality in c_parser_gimple_or_rtl_pass_list >>>> is a way how to test that next 'token' is a profile count. >>> >>> Who cares about error messages... But sure, I'm just proposing to >>> merge testing for next token and actual parsing. >> >> After I've done removal of hot_bb_threshold parsing, there are just 2 usages >> of parse_profile_quality. I would like to leave it as it, not introducing a wrappers. >> >>> >>>>> >>>>> + else if (!strcmp (op, "hot_bb_threshold")) >>>>> + { >>>>> >>>>> I'm not sure about this - it doesn't make sense to specify this >>>>> on a per-function base since it seems to control a global >>>>> variable (booo!)? >>>> >>>> Yep, shame on me! >>>> >>>>> Isn't this instead computed on-demand >>>>> based on profile_info->sum_max? >>>> >>>> No it's a global value shared among functions. >>>> >>>>> If not then I think >>>>> we need an alternate way of funneling in global state into >>>>> the GIMPLE FE. >>>> >>>> What about --param gimple-fe-hot-bb-threshold ? >>> >>> I thought about that, yes ... in absence can it actually be >>> "computed"? >> >> Renamed to it. >> >> Patch can bootstrap on x86_64-linux-gnu and survives regression tests. >> >> Ready to be installed? > > @@ -470,7 +576,8 @@ c_parser_gimple_compound_statement (gimple_parser > &parser, gimple_seq *seq) > last_basic_block_for_fn (cfun) = index + 1; > n_basic_blocks_for_fn (cfun)++; > if (!parser.current_bb) > - parser.push_edge (ENTRY_BLOCK, bb->index, EDGE_FALLTHRU); > + parser.push_edge (ENTRY_BLOCK, bb->index, EDGE_FALLTHRU, > + profile_probability::uninitialized ()); > > /* We leave the proper setting to fixup. */ > struct loop *loop_father = loops_for_fn (cfun)->tree_root; > > the fallthru edge from ENTRY to the single-succ of ENTRY should have > 100% probability and be "known", no? Or do we actually use uninitialized > for fallthru edges? Fixed by setting to ::always (). > > diff --git a/gcc/params.def b/gcc/params.def > index 3c9c5fc0f13..840b81f43cc 100644 > --- a/gcc/params.def > +++ b/gcc/params.def > @@ -1414,6 +1414,12 @@ DEFPARAM(PARAM_LOOP_VERSIONING_MAX_OUTER_INSNS, > " loops.", > 100, 0, 0) > > +DEFPARAM(PARAM_GIMPLE_FE_COMPUTED_HOT_BB_THRESHOLD, > + "gimple-fe-computed-hot-bb-threshold", > + "The number of executions of a basic block which is considered hot." > + " The parameters is used only in GIMPLE FE.", > + 0, 0, 0) > + > /* > > Local variables: > diff --git a/gcc/predict.c b/gcc/predict.c > index b010c20ff7d..12a484a799a 100644 > --- a/gcc/predict.c > +++ b/gcc/predict.c > @@ -132,8 +132,11 @@ get_hot_bb_threshold () > { > if (min_count == -1) > { > - min_count > - = profile_info->sum_max / PARAM_VALUE (HOT_BB_COUNT_FRACTION); > + if (flag_gimple) > + min_count = PARAM_VALUE (PARAM_GIMPLE_FE_COMPUTED_HOT_BB_THRESHOLD); > + else > + min_count > + = profile_info->sum_max / PARAM_VALUE (HOT_BB_COUNT_FRACTION); > if (dump_file) > fprintf (dump_file, "Setting hotness threshold to %" PRId64 ".\n", > min_count); > > Ick. I'd rather do this somewhere in the frontend where PARAM_VALUE > is initialized > via set_hot_bb_threshold. Even redundantly for each parsed GIMPLE > function would > be OK and better IMHO. I've done that in c_parser_parse_gimple_body. > That way you don't need get_current_hot_bb_threshold(?). We still want it for dump_function_to_file that prints the value in a comment. > > OK with that changes. > > We need to think about switch stmt parsing where we currently have > > switch (argc_2(D)) {default: L2; case 0: L0; case 1: L0; case 2: L1; } > > __BB(3): > L0: > a_4 = 0; > goto __BB6; > ... > > extending this to > > switch (argc_2(D)) { default: L2 (guessed(10)); ... > > might be possible so we have profile on the edges. I can work on that during this stage1. Martin > > After the patch all -gimple dumps have profile - probably good > but also visually disturbing. Ah well. > > Hope to do loop flags as followup soon. > > Thanks, > Richard. > >> Thanks, >> Martin >> >>> >>> Richard. >>> >>>> Thanks, >>>> Martin >>>> >>>>> >>>>> Thanks, >>>>> Richard. >>>>> >>>>> >>>>>> >>>>>> Martin >>>> >> [-- Attachment #2: 0001-Support-profile-BB-counts-and-edge-probabilities-in-.patch --] [-- Type: text/x-patch, Size: 27568 bytes --] From cf363e50fec0810bf167c01916c6b758d04e966c Mon Sep 17 00:00:00 2001 From: marxin <mliska@suse.cz> Date: Thu, 4 Apr 2019 14:46:15 +0200 Subject: [PATCH 1/2] Support profile (BB counts and edge probabilities) in GIMPLE FE. gcc/ChangeLog: 2019-04-05 Martin Liska <mliska@suse.cz> * tree-cfg.c (dump_function_to_file): Dump entry BB count. * gimple-pretty-print.c (dump_gimple_bb_header): Dump BB count. (pp_cfg_jump): Dump edge probability. * profile-count.c (profile_quality_as_string): Simplify with a static array. (parse_profile_quality): New function. (profile_count::dump): Simplify with a static array. (profile_count::from_gcov_type): Add new argument. * profile-count.h (parse_profile_quality): Likewise. * predict.h (get_current_hot_bb_threshold): New. * params.def (PARAM_GIMPLE_FE_COMPUTED_HOT_BB_THRESHOLD): New param. * predict.c (get_hot_bb_threshold): Set from the new param. (get_current_hot_bb_threshold): New. gcc/c/ChangeLog: 2019-04-05 Martin Liska <mliska@suse.cz> * gimple-parser.c (struct gimple_parser): Add probability. for gimple_parser_edge. (gimple_parser::push_edge): Add new argument probability. (c_parser_gimple_parse_bb_spec): Parse also probability if present. (c_parser_parse_gimple_body): Set edge probability. (c_parser_gimple_compound_statement): Consume token before calling c_parser_gimple_goto_stmt. Parse BB counts. (c_parser_gimple_statement): Pass new argument. (c_parser_gimple_goto_stmt): Likewise. (c_parser_gimple_if_stmt): Likewise. (c_parser_gimple_or_rtl_pass_list): Parse hot_bb_threshold. * c-parser.c (c_parser_declaration_or_fndef): Pass hot_bb_threshold argument. * c-tree.h (struct c_declspecs): Add hot_bb_threshold field. (c_parser_gimple_parse_bb_spec_edge_probability): New. gcc/testsuite/ChangeLog: 2019-04-05 Martin Liska <mliska@suse.cz> * gcc.dg/gimplefe-37.c: New test. * gcc.dg/gimplefe-33.c: Likewise. --- gcc/c/c-parser.c | 3 +- gcc/c/c-tree.h | 2 + gcc/c/gimple-parser.c | 183 +++++++++++++++++++++++++---- gcc/c/gimple-parser.h | 3 +- gcc/gimple-pretty-print.c | 13 ++ gcc/params.def | 6 + gcc/predict.c | 12 +- gcc/predict.h | 1 + gcc/profile-count.c | 88 +++++++------- gcc/profile-count.h | 22 +++- gcc/testsuite/gcc.dg/gimplefe-37.c | 27 +++++ gcc/testsuite/gcc.dg/gimplefe-38.c | 27 +++++ gcc/tree-cfg.c | 25 +++- 13 files changed, 341 insertions(+), 71 deletions(-) create mode 100644 gcc/testsuite/gcc.dg/gimplefe-37.c create mode 100644 gcc/testsuite/gcc.dg/gimplefe-38.c diff --git a/gcc/c/c-parser.c b/gcc/c/c-parser.c index 854cd6ce8c6..3aa85125cf1 100644 --- a/gcc/c/c-parser.c +++ b/gcc/c/c-parser.c @@ -2347,7 +2347,8 @@ c_parser_declaration_or_fndef (c_parser *parser, bool fndef_ok, bool saved = in_late_binary_op; in_late_binary_op = true; c_parser_parse_gimple_body (parser, specs->gimple_or_rtl_pass, - specs->declspec_il); + specs->declspec_il, + specs->entry_bb_count); in_late_binary_op = saved; } else diff --git a/gcc/c/c-tree.h b/gcc/c/c-tree.h index 7e35ab1f0bc..346f46a5207 100644 --- a/gcc/c/c-tree.h +++ b/gcc/c/c-tree.h @@ -317,6 +317,8 @@ struct c_declspecs { tree attrs; /* The pass to start compiling a __GIMPLE or __RTL function with. */ char *gimple_or_rtl_pass; + /* ENTRY BB count. */ + profile_count entry_bb_count; /* The base-2 log of the greatest alignment required by an _Alignas specifier, in bytes, or -1 if no such specifiers with nonzero alignment. */ diff --git a/gcc/c/gimple-parser.c b/gcc/c/gimple-parser.c index fff34606dae..ede5a927c3d 100644 --- a/gcc/c/gimple-parser.c +++ b/gcc/c/gimple-parser.c @@ -63,6 +63,7 @@ along with GCC; see the file COPYING3. If not see #include "tree-phinodes.h" #include "tree-into-ssa.h" #include "bitmap.h" +#include "params.h" /* GIMPLE parser state. */ @@ -81,20 +82,23 @@ struct gimple_parser int src; int dest; int flags; + profile_probability probability; }; auto_vec<gimple_parser_edge> edges; basic_block current_bb; - void push_edge (int, int, int); + void push_edge (int, int, int, profile_probability); }; void -gimple_parser::push_edge (int src, int dest, int flags) +gimple_parser::push_edge (int src, int dest, int flags, + profile_probability prob) { gimple_parser_edge e; e.src = src; e.dest = dest; e.flags = flags; + e.probability = prob; edges.safe_push (e); } @@ -120,7 +124,7 @@ static void c_parser_gimple_expr_list (gimple_parser &, vec<tree> *); /* See if VAL is an identifier matching __BB<num> and return <num> - in *INDEX. Return true if so. */ + in *INDEX. */ static bool c_parser_gimple_parse_bb_spec (tree val, int *index) @@ -134,11 +138,77 @@ c_parser_gimple_parse_bb_spec (tree val, int *index) return *index > 0; } +/* See if VAL is an identifier matching __BB<num> and return <num> + in *INDEX. Return true if so and parse also FREQUENCY of + the edge. */ + + +static bool +c_parser_gimple_parse_bb_spec_edge_probability (tree val, + gimple_parser &parser, + int *index, + profile_probability *probablity) +{ + bool return_p = c_parser_gimple_parse_bb_spec (val, index); + if (return_p) + { + *probablity = profile_probability::uninitialized (); + /* Parse frequency if provided. */ + if (c_parser_next_token_is (parser, CPP_OPEN_PAREN)) + { + tree f; + c_parser_consume_token (parser); + if (!c_parser_next_token_is (parser, CPP_NAME)) + { + c_parser_error (parser, "expected frequency quality"); + return false; + } + + profile_quality quality; + const char *v + = IDENTIFIER_POINTER (c_parser_peek_token (parser)->value); + if (!parse_profile_quality (v, &quality)) + { + c_parser_error (parser, "unknown profile quality"); + return false; + } + + c_parser_consume_token (parser); + if (!c_parser_require (parser, CPP_OPEN_PAREN, "expected %<(%>")) + return false; + + if (!c_parser_next_token_is (parser, CPP_NUMBER) + || (TREE_CODE (f = c_parser_peek_token (parser)->value) + != INTEGER_CST)) + { + c_parser_error (parser, "expected frequency value"); + return false; + } + + unsigned int value = TREE_INT_CST_LOW (f); + *probablity = profile_probability (value, quality); + + c_parser_consume_token (parser); + if (!c_parser_require (parser, CPP_CLOSE_PAREN, "expected %<)%>")) + return false; + + if (!c_parser_require (parser, CPP_CLOSE_PAREN, "expected %<)%>")) + return false; + } + + return true; + } + + return false; + +} + /* Parse the body of a function declaration marked with "__GIMPLE". */ void c_parser_parse_gimple_body (c_parser *cparser, char *gimple_pass, - enum c_declspec_il cdil) + enum c_declspec_il cdil, + profile_count entry_bb_count) { gimple_parser parser (cparser); gimple_seq seq = NULL; @@ -209,9 +279,12 @@ c_parser_parse_gimple_body (c_parser *cparser, char *gimple_pass, add_local_decl (cfun, var); /* We have a CFG. Build the edges. */ for (unsigned i = 0; i < parser.edges.length (); ++i) - make_edge (BASIC_BLOCK_FOR_FN (cfun, parser.edges[i].src), - BASIC_BLOCK_FOR_FN (cfun, parser.edges[i].dest), - parser.edges[i].flags); + { + edge e = make_edge (BASIC_BLOCK_FOR_FN (cfun, parser.edges[i].src), + BASIC_BLOCK_FOR_FN (cfun, parser.edges[i].dest), + parser.edges[i].flags); + e->probability = parser.edges[i].probability; + } /* Add edges for case labels. */ basic_block bb; FOR_EACH_BB_FN (bb, cfun) @@ -274,6 +347,13 @@ c_parser_parse_gimple_body (c_parser *cparser, char *gimple_pass, fix_loop_structure (NULL); } + if (cfun->curr_properties & PROP_cfg) + { + ENTRY_BLOCK_PTR_FOR_FN (cfun)->count = entry_bb_count; + gcov_type t = PARAM_VALUE (PARAM_GIMPLE_FE_COMPUTED_HOT_BB_THRESHOLD); + set_hot_bb_threshold (t); + update_max_bb_count (); + } dump_function (TDI_gimple, current_function_decl); } @@ -337,11 +417,9 @@ c_parser_gimple_compound_statement (gimple_parser &parser, gimple_seq *seq) c_parser_consume_token (parser); if (c_parser_next_token_is (parser, CPP_NAME)) { - c_parser_gimple_goto_stmt (parser, loc, - c_parser_peek_token - (parser)->value, - seq); + tree label = c_parser_peek_token (parser)->value; c_parser_consume_token (parser); + c_parser_gimple_goto_stmt (parser, loc, label, seq); if (! c_parser_require (parser, CPP_SEMICOLON, "expected %<;%>")) return return_p; @@ -355,7 +433,8 @@ c_parser_gimple_compound_statement (gimple_parser &parser, gimple_seq *seq) "expected %<;%>")) return return_p; if (cfun->curr_properties & PROP_cfg) - parser.push_edge (parser.current_bb->index, EXIT_BLOCK, 0); + parser.push_edge (parser.current_bb->index, EXIT_BLOCK, 0, + profile_probability::uninitialized ()); break; default: goto expr_stmt; @@ -397,6 +476,7 @@ c_parser_gimple_compound_statement (gimple_parser &parser, gimple_seq *seq) return return_p; } int is_loop_header_of = -1; + profile_count bb_count = profile_count::uninitialized (); c_parser_consume_token (parser); while (c_parser_next_token_is (parser, CPP_COMMA)) { @@ -430,10 +510,39 @@ c_parser_gimple_compound_statement (gimple_parser &parser, gimple_seq *seq) "expected %<)%>")) return return_p; } + /* Parse profile: quality(value) */ else { - c_parser_error (parser, "unknown block specifier"); - return return_p; + tree q; + profile_quality quality; + tree v = c_parser_peek_token (parser)->value; + if (!parse_profile_quality (IDENTIFIER_POINTER (v), + &quality)) + { + c_parser_error (parser, "unknown block specifier"); + return false; + } + + c_parser_consume_token (parser); + if (!c_parser_require (parser, CPP_OPEN_PAREN, + "expected %<(%>")) + return false; + + if (!c_parser_next_token_is (parser, CPP_NUMBER) + || (TREE_CODE (q = c_parser_peek_token (parser)->value) + != INTEGER_CST)) + { + c_parser_error (parser, "expected count value"); + return false; + } + + bb_count + = profile_count::from_gcov_type (TREE_INT_CST_LOW (q), + quality); + c_parser_consume_token (parser); + if (! c_parser_require (parser, CPP_CLOSE_PAREN, + "expected %<)%>")) + return return_p; } } if (! c_parser_require (parser, CPP_CLOSE_PAREN, @@ -470,7 +579,8 @@ c_parser_gimple_compound_statement (gimple_parser &parser, gimple_seq *seq) last_basic_block_for_fn (cfun) = index + 1; n_basic_blocks_for_fn (cfun)++; if (!parser.current_bb) - parser.push_edge (ENTRY_BLOCK, bb->index, EDGE_FALLTHRU); + parser.push_edge (ENTRY_BLOCK, bb->index, EDGE_FALLTHRU, + profile_probability::always ()); /* We leave the proper setting to fixup. */ struct loop *loop_father = loops_for_fn (cfun)->tree_root; @@ -498,6 +608,7 @@ c_parser_gimple_compound_statement (gimple_parser &parser, gimple_seq *seq) loop_father = get_loop (cfun, is_loop_header_of); } bb->loop_father = loop_father; + bb->count = bb_count; /* Stmts now go to the new block. */ parser.current_bb = bb; @@ -1609,8 +1720,10 @@ c_parser_gimple_or_rtl_pass_list (c_parser *parser, c_declspecs *specs) return; c_parser_consume_token (parser); + specs->entry_bb_count = profile_count::uninitialized (); while (c_parser_next_token_is (parser, CPP_NAME)) { + profile_quality quality; const char *op = IDENTIFIER_POINTER (c_parser_peek_token (parser)->value); c_parser_consume_token (parser); if (! strcmp (op, "startwith")) @@ -1629,6 +1742,26 @@ c_parser_gimple_or_rtl_pass_list (c_parser *parser, c_declspecs *specs) if (! c_parser_require (parser, CPP_CLOSE_PAREN, "expected %<(%>")) return; } + else if (parse_profile_quality (op, &quality)) + { + tree q; + if (!c_parser_require (parser, CPP_OPEN_PAREN, "expected %<(%>")) + return; + + if (!c_parser_next_token_is (parser, CPP_NUMBER) + || (TREE_CODE (q = c_parser_peek_token (parser)->value) + != INTEGER_CST)) + { + c_parser_error (parser, "expected count value"); + return; + } + + specs->entry_bb_count + = profile_count::from_gcov_type (TREE_INT_CST_LOW (q), quality); + c_parser_consume_token (parser); + if (!c_parser_require (parser, CPP_CLOSE_PAREN, "expected %<)%>")) + return; + } else if (specs->declspec_il != cdil_gimple) /* Allow only one IL specifier and none on RTL. */ ; @@ -1757,10 +1890,12 @@ c_parser_gimple_goto_stmt (gimple_parser &parser, if (cfun->curr_properties & PROP_cfg) { int dest_index; - if (c_parser_gimple_parse_bb_spec (label, &dest_index)) + profile_probability prob; + if (c_parser_gimple_parse_bb_spec_edge_probability (label, parser, + &dest_index, &prob)) { parser.push_edge (parser.current_bb->index, dest_index, - EDGE_FALLTHRU); + EDGE_FALLTHRU, prob); return; } } @@ -1811,10 +1946,12 @@ c_parser_gimple_if_stmt (gimple_parser &parser, gimple_seq *seq) label = c_parser_peek_token (parser)->value; c_parser_consume_token (parser); int dest_index; + profile_probability prob; if ((cfun->curr_properties & PROP_cfg) - && c_parser_gimple_parse_bb_spec (label, &dest_index)) + && c_parser_gimple_parse_bb_spec_edge_probability (label, parser, + &dest_index, &prob)) parser.push_edge (parser.current_bb->index, dest_index, - EDGE_TRUE_VALUE); + EDGE_TRUE_VALUE, prob); else t_label = lookup_label_for_goto (loc, label); if (! c_parser_require (parser, CPP_SEMICOLON, "expected %<;%>")) @@ -1844,14 +1981,16 @@ c_parser_gimple_if_stmt (gimple_parser &parser, gimple_seq *seq) return; } label = c_parser_peek_token (parser)->value; + c_parser_consume_token (parser); int dest_index; + profile_probability prob; if ((cfun->curr_properties & PROP_cfg) - && c_parser_gimple_parse_bb_spec (label, &dest_index)) + && c_parser_gimple_parse_bb_spec_edge_probability (label, parser, + &dest_index, &prob)) parser.push_edge (parser.current_bb->index, dest_index, - EDGE_FALSE_VALUE); + EDGE_FALSE_VALUE, prob); else f_label = lookup_label_for_goto (loc, label); - c_parser_consume_token (parser); if (! c_parser_require (parser, CPP_SEMICOLON, "expected %<;%>")) return; } diff --git a/gcc/c/gimple-parser.h b/gcc/c/gimple-parser.h index 383ad768759..cc28c0f7bf9 100644 --- a/gcc/c/gimple-parser.h +++ b/gcc/c/gimple-parser.h @@ -22,7 +22,8 @@ along with GCC; see the file COPYING3. If not see /* Gimple parsing functions. */ extern void c_parser_parse_gimple_body (c_parser *, char *, - enum c_declspec_il); + enum c_declspec_il, + profile_count); extern void c_parser_gimple_or_rtl_pass_list (c_parser *, c_declspecs *); #endif diff --git a/gcc/gimple-pretty-print.c b/gcc/gimple-pretty-print.c index 69bae0d10d0..7e3916bff86 100644 --- a/gcc/gimple-pretty-print.c +++ b/gcc/gimple-pretty-print.c @@ -2704,6 +2704,10 @@ dump_gimple_bb_header (FILE *outf, basic_block bb, int indent, fprintf (outf, "%*s__BB(%d", indent, "", bb->index); if (bb->loop_father->header == bb) fprintf (outf, ",loop_header(%d)", bb->loop_father->num); + if (bb->count.initialized_p ()) + fprintf (outf, ",%s(%d)", + profile_quality_as_string (bb->count.quality ()), + bb->count.value ()); fprintf (outf, "):\n"); } else @@ -2760,6 +2764,15 @@ pp_cfg_jump (pretty_printer *buffer, edge e, dump_flags_t flags) { pp_string (buffer, "goto __BB"); pp_decimal_int (buffer, e->dest->index); + if (e->probability.initialized_p ()) + { + pp_string (buffer, "("); + pp_string (buffer, + profile_quality_as_string (e->probability.quality ())); + pp_string (buffer, "("); + pp_decimal_int (buffer, e->probability.value ()); + pp_string (buffer, "))"); + } pp_semicolon (buffer); } else diff --git a/gcc/params.def b/gcc/params.def index 904b3cbdf16..5830cb5d6e4 100644 --- a/gcc/params.def +++ b/gcc/params.def @@ -1419,6 +1419,12 @@ DEFPARAM(PARAM_LOOP_VERSIONING_MAX_OUTER_INSNS, " loops.", 100, 0, 0) +DEFPARAM(PARAM_GIMPLE_FE_COMPUTED_HOT_BB_THRESHOLD, + "gimple-fe-computed-hot-bb-threshold", + "The number of executions of a basic block which is considered hot." + " The parameters is used only in GIMPLE FE.", + 0, 0, 0) + /* Local variables: diff --git a/gcc/predict.c b/gcc/predict.c index b010c20ff7d..87467482e91 100644 --- a/gcc/predict.c +++ b/gcc/predict.c @@ -132,8 +132,8 @@ get_hot_bb_threshold () { if (min_count == -1) { - min_count - = profile_info->sum_max / PARAM_VALUE (HOT_BB_COUNT_FRACTION); + gcov_type t = profile_info->sum_max / PARAM_VALUE (HOT_BB_COUNT_FRACTION); + set_hot_bb_threshold (t); if (dump_file) fprintf (dump_file, "Setting hotness threshold to %" PRId64 ".\n", min_count); @@ -141,6 +141,14 @@ get_hot_bb_threshold () return min_count; } +/* Determine the threshold for hot BB counts and do not set the value. */ + +gcov_type +get_current_hot_bb_threshold () +{ + return min_count; +} + /* Set the threshold for hot BB counts. */ void diff --git a/gcc/predict.h b/gcc/predict.h index c1f2f0307dd..fe87dd6fc92 100644 --- a/gcc/predict.h +++ b/gcc/predict.h @@ -52,6 +52,7 @@ enum prediction extern profile_probability split_branch_probability; extern gcov_type get_hot_bb_threshold (void); +extern gcov_type get_current_hot_bb_threshold (void); extern void set_hot_bb_threshold (gcov_type); extern bool maybe_hot_count_p (struct function *, profile_count); extern bool maybe_hot_bb_p (struct function *, const_basic_block); diff --git a/gcc/profile-count.c b/gcc/profile-count.c index 8c58f8666f0..4f0bac002d7 100644 --- a/gcc/profile-count.c +++ b/gcc/profile-count.c @@ -33,34 +33,57 @@ along with GCC; see the file COPYING3. If not see #include "wide-int.h" #include "sreal.h" +/* Names from profile_quality enum values. */ + +const char *profile_quality_names[] = +{ + "uninitialized", + "guessed_local", + "guessed_global0", + "guessed_global0adjusted", + "guessed", + "afdo", + "adjusted", + "precise" +}; + /* Get a string describing QUALITY. */ const char * profile_quality_as_string (enum profile_quality quality) { - switch (quality) - { - default: - gcc_unreachable (); - case profile_uninitialized: - return "uninitialized"; - case profile_guessed_local: - return "guessed_local"; - case profile_guessed_global0: - return "guessed_global0"; - case profile_guessed_global0adjusted: - return "guessed_global0adjusted"; - case profile_guessed: - return "guessed"; - case profile_afdo: - return "afdo"; - case profile_adjusted: - return "adjusted"; - case profile_precise: - return "precise"; - } + return profile_quality_names[quality]; +} + +/* Parse VALUE as profile quality and return true when a valid QUALITY. */ + +bool +parse_profile_quality (const char *value, profile_quality *quality) +{ + for (unsigned i = 0; i < ARRAY_SIZE (profile_quality_names); i++) + if (strcmp (profile_quality_names[i], value) == 0) + { + *quality = (profile_quality)i; + return true; + } + + return false; } +/* Display names from profile_quality enum values. */ + +const char *profile_quality_display_names[] = +{ + NULL, + "estimated locally", + "estimated locally, globally 0", + "estimated locally, globally 0 adjusted", + "adjusted", + "auto FDO", + "guessed", + "precise" +}; + /* Dump THIS to F. */ void @@ -69,23 +92,8 @@ profile_count::dump (FILE *f) const if (!initialized_p ()) fprintf (f, "uninitialized"); else - { - fprintf (f, "%" PRId64, m_val); - if (m_quality == profile_guessed_local) - fprintf (f, " (estimated locally)"); - else if (m_quality == profile_guessed_global0) - fprintf (f, " (estimated locally, globally 0)"); - else if (m_quality == profile_guessed_global0adjusted) - fprintf (f, " (estimated locally, globally 0 adjusted)"); - else if (m_quality == profile_adjusted) - fprintf (f, " (adjusted)"); - else if (m_quality == profile_afdo) - fprintf (f, " (auto FDO)"); - else if (m_quality == profile_guessed) - fprintf (f, " (guessed)"); - else if (m_quality == profile_precise) - fprintf (f, " (precise)"); - } + fprintf (f, "%" PRId64 " (%s)", m_val, + profile_quality_display_names[m_quality]); } /* Dump THIS to stderr. */ @@ -362,7 +370,7 @@ profile_count::combine_with_ipa_count (profile_count ipa) Conversions back and forth are used to read the coverage and get it into internal representation. */ profile_count -profile_count::from_gcov_type (gcov_type v) +profile_count::from_gcov_type (gcov_type v, profile_quality quality) { profile_count ret; gcc_checking_assert (v >= 0); @@ -371,7 +379,7 @@ profile_count::from_gcov_type (gcov_type v) "Capping gcov count %" PRId64 " to max_count %" PRId64 "\n", (int64_t) v, (int64_t) max_count); ret.m_val = MIN (v, (gcov_type)max_count); - ret.m_quality = profile_precise; + ret.m_quality = quality; return ret; } diff --git a/gcc/profile-count.h b/gcc/profile-count.h index d6de61f0a61..2669f0d5fff 100644 --- a/gcc/profile-count.h +++ b/gcc/profile-count.h @@ -60,6 +60,8 @@ enum profile_quality { }; extern const char *profile_quality_as_string (enum profile_quality); +extern bool parse_profile_quality (const char *value, + profile_quality *quality); /* The base value for branch probability notes and edge probabilities. */ #define REG_BR_PROB_BASE 10000 @@ -149,6 +151,13 @@ class GTY((user)) profile_probability friend class profile_count; public: + profile_probability (): m_val (uninitialized_probability), + m_quality (profile_guessed) + {} + + profile_probability (uint32_t val, profile_quality quality): + m_val (val), m_quality (quality) + {} /* Named probabilities. */ static profile_probability never () @@ -558,6 +567,12 @@ public: return initialized_p () && other.initialized_p () && m_val >= other.m_val; } + /* Get the value of the count. */ + uint32_t value () const { return m_val; } + + /* Get the quality of the count. */ + enum profile_quality quality () const { return m_quality; } + /* Output THIS to F. */ void dump (FILE *f) const; @@ -675,7 +690,6 @@ private: return ipa_p () == other.ipa_p (); } public: - /* Used for counters which are expected to be never executed. */ static profile_count zero () { @@ -737,6 +751,9 @@ public: return m_quality == profile_precise; } + /* Get the value of the count. */ + uint32_t value () const { return m_val; } + /* Get the quality of the count. */ enum profile_quality quality () const { return m_quality; } @@ -1136,7 +1153,8 @@ public: /* The profiling runtime uses gcov_type, which is usually 64bit integer. Conversions back and forth are used to read the coverage and get it into internal representation. */ - static profile_count from_gcov_type (gcov_type v); + static profile_count from_gcov_type (gcov_type v, + profile_quality quality = profile_precise); /* LTO streaming support. */ static profile_count stream_in (struct lto_input_block *); diff --git a/gcc/testsuite/gcc.dg/gimplefe-37.c b/gcc/testsuite/gcc.dg/gimplefe-37.c new file mode 100644 index 00000000000..d3ea186d7f9 --- /dev/null +++ b/gcc/testsuite/gcc.dg/gimplefe-37.c @@ -0,0 +1,27 @@ +/* { dg-do compile } */ +/* { dg-options "-O2 -fgimple -fdump-tree-optimized --param=gimple-fe-computed-hot-bb-threshold=10 " } */ + +int __GIMPLE (ssa,startwith("slsr"),precise(3)) +main (int argc) +{ + int _1; + + __BB(2,precise(3)): + if (argc_2(D) == 2) + goto __BB3(precise(44739243)); + else + goto __BB4(precise(89478485)); + + __BB(3,precise(1)): + goto __BB4(precise(134217728)); + + __BB(4,precise(3)): + _1 = __PHI (__BB2: 0, __BB3: 12); + return _1; +} + + +/* { dg-final { scan-tree-dump-times "<bb \[0-9\]> \\\[count: 3" 2 "optimized" } } */ +/* { dg-final { scan-tree-dump-times "<bb \[0-9\]> \\\[count: 2" 1 "optimized" } } */ +/* { dg-final { scan-tree-dump-times "goto <bb \[0-9\]>; \\\[33\\\.33%\\\]" 1 "optimized" } } */ +/* { dg-final { scan-tree-dump-times "goto <bb \[0-9\]>; \\\[66\\\.67%\\\]" 1 "optimized" } } */ diff --git a/gcc/testsuite/gcc.dg/gimplefe-38.c b/gcc/testsuite/gcc.dg/gimplefe-38.c new file mode 100644 index 00000000000..64532d87da5 --- /dev/null +++ b/gcc/testsuite/gcc.dg/gimplefe-38.c @@ -0,0 +1,27 @@ +/* { dg-do compile } */ +/* { dg-options "-O2 -fgimple -fdump-tree-slsr" } */ + +int __GIMPLE (ssa,startwith("slsr"),guessed_local(1073741824)) +main (int argc) +{ + int _1; + + __BB(2,guessed_local(1073741824)): + if (argc_2(D) == 2) + goto __BB3(guessed(16777216)); + else + goto __BB4(guessed(117440512)); + + __BB(3,guessed_local(134217728)): + goto __BB4(precise(134217728)); + + __BB(4,guessed_local(1073741824)): + _1 = __PHI (__BB2: 0, __BB3: 12); + return _1; +} + + +/* { dg-final { scan-tree-dump-times "<bb \[0-9\]> \\\[local count: 1073741824" 2 "slsr" } } */ +/* { dg-final { scan-tree-dump-times "<bb \[0-9\]> \\\[local count: 134217728" 1 "slsr" } } */ +/* { dg-final { scan-tree-dump-times "goto <bb \[0-9\]>; \\\[12\\\.50%\\\]" 1 "slsr" } } */ +/* { dg-final { scan-tree-dump-times "goto <bb \[0-9\]>; \\\[87\\\.50%\\\]" 1 "slsr" } } */ diff --git a/gcc/tree-cfg.c b/gcc/tree-cfg.c index 587408150fb..f7ebac674db 100644 --- a/gcc/tree-cfg.c +++ b/gcc/tree-cfg.c @@ -7872,13 +7872,32 @@ dump_function_to_file (tree fndecl, FILE *file, dump_flags_t flags) current_function_decl = fndecl; if (flags & TDF_GIMPLE) { + static bool hotness_bb_param_printed = false; + if (get_current_hot_bb_threshold () != -1 + && !hotness_bb_param_printed) + { + hotness_bb_param_printed = true; + fprintf (file, + "/* --param=gimple-fe-computed-hot-bb-threshold=%" PRId64 + " */\n", get_current_hot_bb_threshold ()); + } + print_generic_expr (file, TREE_TYPE (TREE_TYPE (fndecl)), dump_flags | TDF_SLIM); - fprintf (file, " __GIMPLE (%s)\n%s (", + fprintf (file, " __GIMPLE (%s", (fun->curr_properties & PROP_ssa) ? "ssa" : (fun->curr_properties & PROP_cfg) ? "cfg" - : "", - function_name (fun)); + : ""); + + if (cfun->cfg) + { + basic_block bb = ENTRY_BLOCK_PTR_FOR_FN (cfun); + if (bb->count.initialized_p ()) + fprintf (file, ",%s(%d)", + profile_quality_as_string (bb->count.quality ()), + bb->count.value ()); + fprintf (file, ")\n%s (", function_name (fun)); + } } else fprintf (file, "%s %s(", function_name (fun), tmclone ? "[tm-clone] " : ""); -- 2.21.0 ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH][stage1] Support profile (BB counts and edge probabilities) in GIMPLE FE. 2019-05-07 12:00 ` Martin Liška @ 2019-05-07 12:56 ` Richard Biener 2019-05-07 14:33 ` Martin Liška 2019-05-10 6:39 ` Bernhard Reutner-Fischer 1 sibling, 1 reply; 22+ messages in thread From: Richard Biener @ 2019-05-07 12:56 UTC (permalink / raw) To: Martin Liška; +Cc: Joseph S. Myers, Jan Hubicka, GCC Patches On Tue, May 7, 2019 at 2:00 PM Martin Liška <mliska@suse.cz> wrote: > > On 5/6/19 4:02 PM, Richard Biener wrote: > > On Mon, May 6, 2019 at 9:59 AM Martin Liška <mliska@suse.cz> wrote: > >> > >> On 5/2/19 2:31 PM, Richard Biener wrote: > >>> On Mon, Apr 29, 2019 at 2:51 PM Martin Liška <mliska@suse.cz> wrote: > >>>> > >>>> On 4/26/19 3:18 PM, Richard Biener wrote: > >>>>> On Wed, Apr 10, 2019 at 10:12 AM Martin Liška <mliska@suse.cz> wrote: > >>>>>> > >>>>>> On 4/9/19 3:19 PM, Jan Hubicka wrote: > >>>>>>>> Hi. > >>>>>>>> > >>>>>>>> There's updated version that supports profile quality for both counts > >>>>>>>> and probabilities. I'm wondering whether ENTRY and EXIT BBs needs to > >>>>>>>> have set probability. Apparently, I haven't seen any verifier that > >>>>>>>> would complain. > >>>>>>> > >>>>>>> Well, you do not need to define it but then several cases will > >>>>>>> degenerate. In particular BB frequencies (for callgraph profile or > >>>>>>> hot/cold decisions) are calculated as ratios of entry BB and given BB > >>>>>>> count. If entry BB is undefined you will get those undefined and > >>>>>>> heuristics will resort to conservative answers. > >>>>>>> > >>>>>>> I do not think we use exit block count. > >>>>>>> > >>>>>>> Honza > >>>>>>> > >>>>>> > >>>>>> Thank you Honza for explanation. I'm sending version of the patch > >>>>>> that supports entry BB count. > >>>>>> > >>>>>> I've been testing the patch right now. > >>>>> > >>>>> Can you move the GIMPLE/RTL FE specific data in c_declspecs to > >>>>> a substructure accessed via indirection? I guess enlarging that > >>>>> isn't really what we should do. You'd move gimple_or_rtl_pass > >>>>> there and make that pointer one to a struct aux_fe_data > >>>>> (lifetime managed by the respective RTL/GIMPLE FE, thus > >>>>> to be freed there)? Joseph, do you agree or is adding more > >>>>> stuff to c_declspecs OK (I would guess it could be a few more > >>>>> elements in the future). > >>>> > >>>> Let's wait here for Joseph. > >>> > >>> So looks like it won't matter so let's go with the current approach > >>> for the moment. > >>> > >>>>> > >>>>> -c_parser_gimple_parse_bb_spec (tree val, int *index) > >>>>> +c_parser_gimple_parse_bb_spec (tree val, gimple_parser &parser, > >>>>> + int *index, profile_probability *probablity) > >>>>> { > >>>>> > >>>>> so this will allow specifying probability in PHI node arguments. > >>>>> I think we want to split this into the already existing part > >>>>> and a c_parser_gimple_parse_bb_spec_with_edge_probability > >>>>> to be used at edge sources. > >>>> > >>>> Yes, that's a good idea! > >>>> > >>>>> > >>>>> + if (cfun->curr_properties & PROP_cfg) > >>>>> + { > >>>>> + update_max_bb_count (); > >>>>> + set_hot_bb_threshold (hot_bb_threshold); > >>>>> + ENTRY_BLOCK_PTR_FOR_FN (cfun)->count = entry_bb_count; > >>>>> > >>>>> I guess the last one should be before update_max_bb_count ()? > >>>> > >>>> Same here. > >>>> > >>>>> > >>>>> + } > >>>>> > >>>>> + /* Parse profile: quality(value) */ > >>>>> else > >>>>> { > >>>>> - c_parser_error (parser, "unknown block specifier"); > >>>>> - return return_p; > >>>>> + tree q; > >>>>> + profile_quality quality; > >>>>> + tree v = c_parser_peek_token (parser)->value; > >>>>> > >>>>> peek next token before the if/else and do > >>>>> > >>>>> else if (!strcmp (...)) > >>>>> > >>>>> as in the loop_header case. I expected we can somehow share > >>>>> parsing of profile quality and BB/edge count/frequency? How's > >>>>> the expected syntax btw, comments in the code should tell us. > >>>>> I'm guessing it's quality-id '(' number ')' and thus it should be > >>>>> really shareable between edge and BB count and also __GIMPLE > >>>>> header parsing? So parse_profile_quality should be > >>>>> parse_profile () instead, resulting in a combined value > >>>>> (we do use the same for edge/bb?). > >>>> > >>>> It's problematic, there are different error messages for count/frequency. > >>>> Moreover call to parse_profile_quality in c_parser_gimple_or_rtl_pass_list > >>>> is a way how to test that next 'token' is a profile count. > >>> > >>> Who cares about error messages... But sure, I'm just proposing to > >>> merge testing for next token and actual parsing. > >> > >> After I've done removal of hot_bb_threshold parsing, there are just 2 usages > >> of parse_profile_quality. I would like to leave it as it, not introducing a wrappers. > >> > >>> > >>>>> > >>>>> + else if (!strcmp (op, "hot_bb_threshold")) > >>>>> + { > >>>>> > >>>>> I'm not sure about this - it doesn't make sense to specify this > >>>>> on a per-function base since it seems to control a global > >>>>> variable (booo!)? > >>>> > >>>> Yep, shame on me! > >>>> > >>>>> Isn't this instead computed on-demand > >>>>> based on profile_info->sum_max? > >>>> > >>>> No it's a global value shared among functions. > >>>> > >>>>> If not then I think > >>>>> we need an alternate way of funneling in global state into > >>>>> the GIMPLE FE. > >>>> > >>>> What about --param gimple-fe-hot-bb-threshold ? > >>> > >>> I thought about that, yes ... in absence can it actually be > >>> "computed"? > >> > >> Renamed to it. > >> > >> Patch can bootstrap on x86_64-linux-gnu and survives regression tests. > >> > >> Ready to be installed? > > > > @@ -470,7 +576,8 @@ c_parser_gimple_compound_statement (gimple_parser > > &parser, gimple_seq *seq) > > last_basic_block_for_fn (cfun) = index + 1; > > n_basic_blocks_for_fn (cfun)++; > > if (!parser.current_bb) > > - parser.push_edge (ENTRY_BLOCK, bb->index, EDGE_FALLTHRU); > > + parser.push_edge (ENTRY_BLOCK, bb->index, EDGE_FALLTHRU, > > + profile_probability::uninitialized ()); > > > > /* We leave the proper setting to fixup. */ > > struct loop *loop_father = loops_for_fn (cfun)->tree_root; > > > > the fallthru edge from ENTRY to the single-succ of ENTRY should have > > 100% probability and be "known", no? Or do we actually use uninitialized > > for fallthru edges? > > Fixed by setting to ::always (). > > > > > diff --git a/gcc/params.def b/gcc/params.def > > index 3c9c5fc0f13..840b81f43cc 100644 > > --- a/gcc/params.def > > +++ b/gcc/params.def > > @@ -1414,6 +1414,12 @@ DEFPARAM(PARAM_LOOP_VERSIONING_MAX_OUTER_INSNS, > > " loops.", > > 100, 0, 0) > > > > +DEFPARAM(PARAM_GIMPLE_FE_COMPUTED_HOT_BB_THRESHOLD, > > + "gimple-fe-computed-hot-bb-threshold", > > + "The number of executions of a basic block which is considered hot." > > + " The parameters is used only in GIMPLE FE.", > > + 0, 0, 0) > > + > > /* > > > > Local variables: > > diff --git a/gcc/predict.c b/gcc/predict.c > > index b010c20ff7d..12a484a799a 100644 > > --- a/gcc/predict.c > > +++ b/gcc/predict.c > > @@ -132,8 +132,11 @@ get_hot_bb_threshold () > > { > > if (min_count == -1) > > { > > - min_count > > - = profile_info->sum_max / PARAM_VALUE (HOT_BB_COUNT_FRACTION); > > + if (flag_gimple) > > + min_count = PARAM_VALUE (PARAM_GIMPLE_FE_COMPUTED_HOT_BB_THRESHOLD); > > + else > > + min_count > > + = profile_info->sum_max / PARAM_VALUE (HOT_BB_COUNT_FRACTION); > > if (dump_file) > > fprintf (dump_file, "Setting hotness threshold to %" PRId64 ".\n", > > min_count); > > > > Ick. I'd rather do this somewhere in the frontend where PARAM_VALUE > > is initialized > > via set_hot_bb_threshold. Even redundantly for each parsed GIMPLE > > function would > > be OK and better IMHO. > > I've done that in c_parser_parse_gimple_body. > > > That way you don't need get_current_hot_bb_threshold(?). > > We still want it for dump_function_to_file that prints the value in a comment. But that can use the existing get_hot_bb_threshold since we never want to dump -1 in case min_count was never initialized. > > > > OK with that changes. > > > > We need to think about switch stmt parsing where we currently have > > > > switch (argc_2(D)) {default: L2; case 0: L0; case 1: L0; case 2: L1; } > > > > __BB(3): > > L0: > > a_4 = 0; > > goto __BB6; > > ... > > > > extending this to > > > > switch (argc_2(D)) { default: L2 (guessed(10)); ... > > > > might be possible so we have profile on the edges. > > I can work on that during this stage1. > > Martin > > > > > After the patch all -gimple dumps have profile - probably good > > but also visually disturbing. Ah well. > > > > Hope to do loop flags as followup soon. > > > > Thanks, > > Richard. > > > >> Thanks, > >> Martin > >> > >>> > >>> Richard. > >>> > >>>> Thanks, > >>>> Martin > >>>> > >>>>> > >>>>> Thanks, > >>>>> Richard. > >>>>> > >>>>> > >>>>>> > >>>>>> Martin > >>>> > >> > ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH][stage1] Support profile (BB counts and edge probabilities) in GIMPLE FE. 2019-05-07 12:56 ` Richard Biener @ 2019-05-07 14:33 ` Martin Liška 2019-05-07 14:44 ` Richard Biener 0 siblings, 1 reply; 22+ messages in thread From: Martin Liška @ 2019-05-07 14:33 UTC (permalink / raw) To: Richard Biener; +Cc: Joseph S. Myers, Jan Hubicka, GCC Patches On 5/7/19 2:56 PM, Richard Biener wrote: > But that can use the existing get_hot_bb_threshold since we never want > to dump -1 in case min_count was never initialized. Yes. But the function will call: get_hot_bb_threshold () { if (min_count == -1) { gcov_type t = profile_info->sum_max / PARAM_VALUE (HOT_BB_COUNT_FRACTION); set_hot_bb_threshold (t); ... which will cause a segfault in non-PGO run. Note that: static gcov_type min_count = -1; is a non-exported variable so that's why I simply added the getter. Hope it's fine as is? Martin ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH][stage1] Support profile (BB counts and edge probabilities) in GIMPLE FE. 2019-05-07 14:33 ` Martin Liška @ 2019-05-07 14:44 ` Richard Biener 2019-05-09 10:01 ` Martin Liška 0 siblings, 1 reply; 22+ messages in thread From: Richard Biener @ 2019-05-07 14:44 UTC (permalink / raw) To: Martin Liška; +Cc: Joseph S. Myers, Jan Hubicka, GCC Patches On May 7, 2019 4:33:08 PM GMT+02:00, "Martin Liška" <mliska@suse.cz> wrote: >On 5/7/19 2:56 PM, Richard Biener wrote: >> But that can use the existing get_hot_bb_threshold since we never >want >> to dump -1 in case min_count was never initialized. > >Yes. But the function will call: > >get_hot_bb_threshold () >{ > if (min_count == -1) > { >gcov_type t = profile_info->sum_max / PARAM_VALUE >(HOT_BB_COUNT_FRACTION); > set_hot_bb_threshold (t); >... > >which will cause a segfault in non-PGO run. Note that: >static gcov_type min_count = -1; > >is a non-exported variable so that's why I simply added the getter. > >Hope it's fine as is? Oh, I see. Hmm, so we should get away with no min_coubt when all counter kinds are non-pgo? Richard. > >Martin ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH][stage1] Support profile (BB counts and edge probabilities) in GIMPLE FE. 2019-05-07 14:44 ` Richard Biener @ 2019-05-09 10:01 ` Martin Liška 0 siblings, 0 replies; 22+ messages in thread From: Martin Liška @ 2019-05-09 10:01 UTC (permalink / raw) To: Richard Biener; +Cc: Joseph S. Myers, Jan Hubicka, GCC Patches On 5/7/19 4:44 PM, Richard Biener wrote: > On May 7, 2019 4:33:08 PM GMT+02:00, "Martin LiÅ¡ka" <mliska@suse.cz> wrote: >> On 5/7/19 2:56 PM, Richard Biener wrote: >>> But that can use the existing get_hot_bb_threshold since we never >> want >>> to dump -1 in case min_count was never initialized. >> >> Yes. But the function will call: >> >> get_hot_bb_threshold () >> { >> if (min_count == -1) >> { >> gcov_type t = profile_info->sum_max / PARAM_VALUE >> (HOT_BB_COUNT_FRACTION); >> set_hot_bb_threshold (t); >> ... >> >> which will cause a segfault in non-PGO run. Note that: >> static gcov_type min_count = -1; >> >> is a non-exported variable so that's why I simply added the getter. >> >> Hope it's fine as is? > > Oh, I see. Hmm, so we should get away with no min_coubt when all counter kinds are non-pgo? Yes, in that situation we don't want to print: /* --param=gimple-fe-computed-hot-bb-threshold=xyz */ Martin > > Richard. > >> >> Martin > ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH][stage1] Support profile (BB counts and edge probabilities) in GIMPLE FE. 2019-05-07 12:00 ` Martin Liška 2019-05-07 12:56 ` Richard Biener @ 2019-05-10 6:39 ` Bernhard Reutner-Fischer 1 sibling, 0 replies; 22+ messages in thread From: Bernhard Reutner-Fischer @ 2019-05-10 6:39 UTC (permalink / raw) To: gcc-patches, Martin Liška, Richard Biener Cc: Joseph S. Myers, Jan Hubicka, GCC Patches On 7 May 2019 14:00:32 CEST, "Martin Liška" <mliska@suse.cz> wrote: /The parameters is/s/parameters/parameter/ thanks, ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH][stage1] Support profile (BB counts and edge probabilities) in GIMPLE FE. 2019-04-26 13:29 ` Richard Biener 2019-04-29 12:54 ` Martin Liška @ 2019-04-30 2:05 ` Joseph Myers 1 sibling, 0 replies; 22+ messages in thread From: Joseph Myers @ 2019-04-30 2:05 UTC (permalink / raw) To: Richard Biener; +Cc: Martin Liška, Jan Hubicka, GCC Patches On Fri, 26 Apr 2019, Richard Biener wrote: > Can you move the GIMPLE/RTL FE specific data in c_declspecs to > a substructure accessed via indirection? I guess enlarging that > isn't really what we should do. You'd move gimple_or_rtl_pass > there and make that pointer one to a struct aux_fe_data > (lifetime managed by the respective RTL/GIMPLE FE, thus > to be freed there)? Joseph, do you agree or is adding more > stuff to c_declspecs OK (I would guess it could be a few more > elements in the future). c_declspecs is shortlived (on the parser obstack, freed after each external declaration is parsed). I don't think its size is particularly important (this is however a guess, not based on any actual measurements of cc1 memory usage). -- Joseph S. Myers joseph@codesourcery.com ^ permalink raw reply [flat|nested] 22+ messages in thread
end of thread, other threads:[~2019-05-10 6:39 UTC | newest]
Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-04-05 12:32 [PATCH][stage1] Support profile (BB counts and edge probabilities) in GIMPLE FE Martin Liška
2019-04-08 9:11 ` Richard Biener
2019-04-08 13:35 ` Martin Liška
2019-04-09 13:15 ` Martin Liška
2019-04-09 14:02 ` Jan Hubicka
2019-04-10 9:57 ` Martin Liška
2019-04-26 13:29 ` Richard Biener
2019-04-29 12:54 ` Martin Liška
2019-05-02 12:31 ` Richard Biener
2019-05-06 7:59 ` Martin Liška
2019-05-06 8:00 ` [PATCH 2/2] Support {MIN,MAX}_EXPR " Martin Liška
2019-05-06 11:35 ` Richard Biener
2019-05-07 12:01 ` Martin Liška
2019-05-07 12:58 ` Richard Biener
2019-05-06 14:02 ` [PATCH][stage1] Support profile (BB counts and edge probabilities) " Richard Biener
2019-05-07 12:00 ` Martin Liška
2019-05-07 12:56 ` Richard Biener
2019-05-07 14:33 ` Martin Liška
2019-05-07 14:44 ` Richard Biener
2019-05-09 10:01 ` Martin Liška
2019-05-10 6:39 ` Bernhard Reutner-Fischer
2019-04-30 2:05 ` Joseph Myers
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).