[stage1] Support profile (BB counts and edge probabilities) in GIMPLE FE.

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

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
>
>

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
>>
>>

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
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" } } */

> 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

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
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] " : "");

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

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

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).

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
>

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
>>

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
> >>
>

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
>>>>
>>

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
> >>>>
> >>
>

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

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

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
>

On 7 May 2019 14:00:32 CEST, "Martin Liška" <mliska@suse.cz> wrote:

/The parameters is/s/parameters/parameter/

thanks,