RFC (branch prediction): PATCH to implement P0479R5, [[likely]] and [[unlikely]].

RFC (branch prediction): PATCH to implement P0479R5, [[likely]] and [[unlikely]].

[Jason Merrill]

[[likely]] and [[unlikely]] are equivalent to the GNU hot/cold attributes,
except that they can be applied to arbitrary statements as well as labels;
this is most likely to be useful for marking if/else branches as likely or
unlikely.  Conveniently, PREDICT_EXPR fits the bill nicely as a
representation.

I also had to fix marking case labels as hot/cold, which didn't work before.
Which then required me to force __attribute ((fallthrough)) to apply to the
statement rather than the label.

Tested x86_64-pc-linux-gnu.  Does this seem like a sane implementation
approach to people with more experience with PREDICT_EXPR?

gcc/
	* gimplify.c (gimplify_case_label_expr): Handle hot/cold attributes.
gcc/c-family/
	* c-lex.c (c_common_has_attribute): Handle likely/unlikely.
gcc/cp/
	* parser.c (cp_parser_std_attribute): Handle likely/unlikely.
	(cp_parser_statement): Call process_stmt_hotness_attribute.
	(cp_parser_label_for_labeled_statement): Apply attributes to case.
	* cp-gimplify.c (lookup_hotness_attribute, remove_hotness_attribute)
	(process_stmt_hotness_attribute): New.
	* decl.c (finish_case_label): Give label in template type void.
	* pt.c (tsubst_expr) [CASE_LABEL_EXPR]: Copy attributes.
	[PREDICT_EXPR]: Handle.
---
 gcc/cp/cp-tree.h                          |  2 +
 gcc/c-family/c-lex.c                      |  4 +-
 gcc/cp/cp-gimplify.c                      | 42 +++++++++++++++++++++
 gcc/cp/decl.c                             |  2 +-
 gcc/cp/parser.c                           | 45 +++++++++++++++++++----
 gcc/cp/pt.c                               | 12 +++++-
 gcc/gimplify.c                            | 10 ++++-
 gcc/testsuite/g++.dg/cpp2a/attr-likely1.C | 38 +++++++++++++++++++
 gcc/testsuite/g++.dg/cpp2a/attr-likely2.C | 10 +++++
 gcc/testsuite/g++.dg/cpp2a/feat-cxx2a.C   | 12 ++++++
 gcc/ChangeLog                             |  4 ++
 gcc/c-family/ChangeLog                    |  4 ++
 gcc/cp/ChangeLog                          | 12 ++++++
 13 files changed, 184 insertions(+), 13 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/cpp2a/attr-likely1.C
 create mode 100644 gcc/testsuite/g++.dg/cpp2a/attr-likely2.C

diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index c4d79c0cf7f..c55352ec5ff 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -7541,6 +7541,8 @@ extern bool cxx_omp_disregard_value_expr	(tree, bool);
 extern void cp_fold_function			(tree);
 extern tree cp_fully_fold			(tree);
 extern void clear_fold_cache			(void);
+extern tree lookup_hotness_attribute		(tree);
+extern tree process_stmt_hotness_attribute	(tree);
 
 /* in name-lookup.c */
 extern tree strip_using_decl                    (tree);
diff --git a/gcc/c-family/c-lex.c b/gcc/c-family/c-lex.c
index 28a820a2a3d..3cc015083e0 100644
--- a/gcc/c-family/c-lex.c
+++ b/gcc/c-family/c-lex.c
@@ -356,7 +356,9 @@ c_common_has_attribute (cpp_reader *pfile)
 		       || is_attribute_p ("nodiscard", attr_name)
 		       || is_attribute_p ("fallthrough", attr_name))
 		result = 201603;
-	      else if (is_attribute_p ("no_unique_address", attr_name))
+	      else if (is_attribute_p ("no_unique_address", attr_name)
+		       || is_attribute_p ("likely", attr_name)
+		       || is_attribute_p ("unlikely", attr_name))
 		result = 201803;
 	      if (result)
 		attr_name = NULL_TREE;
diff --git a/gcc/cp/cp-gimplify.c b/gcc/cp/cp-gimplify.c
index eb761b118a1..f8212187162 100644
--- a/gcc/cp/cp-gimplify.c
+++ b/gcc/cp/cp-gimplify.c
@@ -2674,4 +2674,46 @@ cp_fold (tree x)
   return x;
 }
 
+/* Look up either "hot" or "cold" in attribute list LIST.  */
+
+tree
+lookup_hotness_attribute (tree list)
+{
+  tree attr = lookup_attribute ("hot", list);
+  if (attr)
+    return attr;
+  return lookup_attribute ("cold", list);
+}
+
+/* Remove both "hot" and "cold" attributes from LIST.  */
+
+static tree
+remove_hotness_attribute (tree list)
+{
+  return remove_attribute ("hot", remove_attribute ("cold", list));
+}
+
+/* If [[likely]] or [[unlikely]] appear on this statement, turn it into a
+   PREDICT_EXPR.  */
+
+tree
+process_stmt_hotness_attribute (tree std_attrs)
+{
+  if (std_attrs == error_mark_node)
+    return std_attrs;
+  if (tree attr = lookup_hotness_attribute (std_attrs))
+    {
+      tree name = get_attribute_name (attr);
+      bool hot = is_attribute_p ("hot", name);
+      tree pred = build_predict_expr (hot ? PRED_HOT_LABEL : PRED_COLD_LABEL,
+				      hot ? TAKEN : NOT_TAKEN);
+      add_stmt (pred);
+      if (tree other = lookup_hotness_attribute (TREE_CHAIN (attr)))
+	warning (OPT_Wattributes, "ignoring attribute %qE after earlier %qE",
+		 get_attribute_name (other), name);
+      std_attrs = remove_hotness_attribute (std_attrs);
+    }
+  return std_attrs;
+}
+
 #include "gt-cp-cp-gimplify.h"
diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c
index 42994055d5f..73f27038e5c 100644
--- a/gcc/cp/decl.c
+++ b/gcc/cp/decl.c
@@ -3624,7 +3624,7 @@ finish_case_label (location_t loc, tree low_value, tree high_value)
 
       /* For templates, just add the case label; we'll do semantic
 	 analysis at instantiation-time.  */
-      label = build_decl (loc, LABEL_DECL, NULL_TREE, NULL_TREE);
+      label = build_decl (loc, LABEL_DECL, NULL_TREE, void_type_node);
       return add_stmt (build_case_label (low_value, high_value, label));
     }
 
diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
index 0428f6dda90..bdc35fa0cb8 100644
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -10875,6 +10875,7 @@ cp_parser_statement (cp_parser* parser, tree in_statement_expr,
   /* Peek at the next token.  */
   token = cp_lexer_peek_token (parser->lexer);
   /* Remember the location of the first token in the statement.  */
+  cp_token *statement_token = token;
   statement_location = token->location;
   add_debug_begin_stmt (statement_location);
   /* If this is a keyword, then that will often determine what kind of
@@ -10896,12 +10897,14 @@ cp_parser_statement (cp_parser* parser, tree in_statement_expr,
 
 	case RID_IF:
 	case RID_SWITCH:
+	  std_attrs = process_stmt_hotness_attribute (std_attrs);
 	  statement = cp_parser_selection_statement (parser, if_p, chain);
 	  break;
 
 	case RID_WHILE:
 	case RID_DO:
 	case RID_FOR:
+	  std_attrs = process_stmt_hotness_attribute (std_attrs);
 	  statement = cp_parser_iteration_statement (parser, if_p, false, 0);
 	  break;
 
@@ -10909,6 +10912,7 @@ cp_parser_statement (cp_parser* parser, tree in_statement_expr,
 	case RID_CONTINUE:
 	case RID_RETURN:
 	case RID_GOTO:
+	  std_attrs = process_stmt_hotness_attribute (std_attrs);
 	  statement = cp_parser_jump_statement (parser);
 	  break;
 
@@ -10918,15 +10922,24 @@ cp_parser_statement (cp_parser* parser, tree in_statement_expr,
 	case RID_AT_FINALLY:
 	case RID_AT_SYNCHRONIZED:
 	case RID_AT_THROW:
+	  std_attrs = process_stmt_hotness_attribute (std_attrs);
 	  statement = cp_parser_objc_statement (parser);
 	  break;
 
 	case RID_TRY:
+	  std_attrs = process_stmt_hotness_attribute (std_attrs);
 	  statement = cp_parser_try_block (parser);
 	  break;
 
 	case RID_NAMESPACE:
 	  /* This must be a namespace alias definition.  */
+	  if (std_attrs != NULL_TREE)
+	    {
+	      /*  Attributes should be parsed as part of the the
+		  declaration, so let's un-parse them.  */
+	      saved_tokens.rollback();
+	      std_attrs = NULL_TREE;
+	    }
 	  cp_parser_declaration_statement (parser);
 	  return;
 	  
@@ -10935,9 +10948,11 @@ cp_parser_statement (cp_parser* parser, tree in_statement_expr,
 	case RID_SYNCHRONIZED:
 	case RID_ATOMIC_NOEXCEPT:
 	case RID_ATOMIC_CANCEL:
+	  std_attrs = process_stmt_hotness_attribute (std_attrs);
 	  statement = cp_parser_transaction (parser, token);
 	  break;
 	case RID_TRANSACTION_CANCEL:
+	  std_attrs = process_stmt_hotness_attribute (std_attrs);
 	  statement = cp_parser_transaction_cancel (parser);
 	  break;
 
@@ -10996,12 +11011,9 @@ cp_parser_statement (cp_parser* parser, tree in_statement_expr,
       if (cp_lexer_next_token_is_not (parser->lexer, CPP_SEMICOLON))
 	{
 	  if (std_attrs != NULL_TREE)
-	    {
-	      /*  Attributes should be parsed as part of the the
-		  declaration, so let's un-parse them.  */
-	      saved_tokens.rollback();
-	      std_attrs = NULL_TREE;
-	    }
+	    /* Attributes should be parsed as part of the declaration,
+	       so let's un-parse them.  */
+	    saved_tokens.rollback();
 
 	  cp_parser_parse_tentatively (parser);
 	  /* Try to parse the declaration-statement.  */
@@ -11009,11 +11021,16 @@ cp_parser_statement (cp_parser* parser, tree in_statement_expr,
 	  /* If that worked, we're done.  */
 	  if (cp_parser_parse_definitely (parser))
 	    return;
+	  /* It didn't work, restore the post-attribute position.  */
+	  if (std_attrs)
+	    cp_lexer_set_token_position (parser->lexer, statement_token);
 	}
       /* All preceding labels have been parsed at this point.  */
       if (loc_after_labels != NULL)
 	*loc_after_labels = statement_location;
 
+      std_attrs = process_stmt_hotness_attribute (std_attrs);
+
       /* Look for an expression-statement instead.  */
       statement = cp_parser_expression_statement (parser, in_statement_expr);
 
@@ -11126,7 +11143,10 @@ cp_parser_label_for_labeled_statement (cp_parser* parser, tree attributes)
 	  {
 	    tree l = finish_case_label (token->location, expr, expr_hi);
 	    if (l && TREE_CODE (l) == CASE_LABEL_EXPR)
-	      FALLTHROUGH_LABEL_P (CASE_LABEL (l)) = fallthrough_p;
+	      {
+		label = CASE_LABEL (l);
+		FALLTHROUGH_LABEL_P (label) = fallthrough_p;
+	      }
 	  }
 	else
 	  error_at (token->location,
@@ -11143,7 +11163,10 @@ cp_parser_label_for_labeled_statement (cp_parser* parser, tree attributes)
 	{
 	  tree l = finish_case_label (token->location, NULL_TREE, NULL_TREE);
 	  if (l && TREE_CODE (l) == CASE_LABEL_EXPR)
-	    FALLTHROUGH_LABEL_P (CASE_LABEL (l)) = fallthrough_p;
+	      {
+		label = CASE_LABEL (l);
+		FALLTHROUGH_LABEL_P (label) = fallthrough_p;
+	      }
 	}
       else
 	error_at (token->location, "case label not within a switch statement");
@@ -11173,6 +11196,8 @@ cp_parser_label_for_labeled_statement (cp_parser* parser, tree attributes)
       cp_parser_parse_tentatively (parser);
       attrs = cp_parser_gnu_attributes_opt (parser);
       if (attrs == NULL_TREE
+	  /* And fallthrough always binds to the expression-statement.  */
+	  || attribute_fallthrough_p (attrs)
 	  || cp_lexer_next_token_is_not (parser->lexer, CPP_SEMICOLON))
 	cp_parser_abort_tentative_parse (parser);
       else if (!cp_parser_parse_definitely (parser))
@@ -25528,6 +25553,10 @@ cp_parser_std_attribute (cp_parser *parser, tree attr_ns)
       else if (is_attribute_p ("optimize_for_synchronized", attr_id))
 	TREE_PURPOSE (attribute)
 	  = get_identifier ("transaction_callable");
+      else if (is_attribute_p ("likely", attr_id))
+	TREE_PURPOSE (attribute) = get_identifier ("hot");
+      else if (is_attribute_p ("unlikely", attr_id))
+	TREE_PURPOSE (attribute) = get_identifier ("cold");
       /* Transactional Memory attributes are GNU attributes.  */
       else if (tm_attr_to_mask (attr_id))
 	TREE_PURPOSE (attribute) = attr_id;
diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
index 0c33c8e1527..dfd9a436465 100644
--- a/gcc/cp/pt.c
+++ b/gcc/cp/pt.c
@@ -17175,12 +17175,17 @@ tsubst_expr (tree t, tree args, tsubst_flags_t complain, tree in_decl,
 
     case CASE_LABEL_EXPR:
       {
+	tree decl = CASE_LABEL (t);
 	tree low = RECUR (CASE_LOW (t));
 	tree high = RECUR (CASE_HIGH (t));
 	tree l = finish_case_label (EXPR_LOCATION (t), low, high);
 	if (l && TREE_CODE (l) == CASE_LABEL_EXPR)
-	  FALLTHROUGH_LABEL_P (CASE_LABEL (l))
-	    = FALLTHROUGH_LABEL_P (CASE_LABEL (t));
+	  {
+	    tree label = CASE_LABEL (l);
+	    FALLTHROUGH_LABEL_P (label) = FALLTHROUGH_LABEL_P (decl);
+	    if (DECL_ATTRIBUTES (decl) != NULL_TREE)
+	      cplus_decl_attributes (&label, DECL_ATTRIBUTES (decl), 0);
+	  }
       }
       break;
 
@@ -17731,6 +17736,9 @@ tsubst_expr (tree t, tree args, tsubst_flags_t complain, tree in_decl,
 			  RECUR (TREE_OPERAND (t, 1)),
 			  RECUR (TREE_OPERAND (t, 2))));
 
+    case PREDICT_EXPR:
+      RETURN (add_stmt (copy_node (t)));
+
     default:
       gcc_assert (!STATEMENT_CODE_P (TREE_CODE (t)));
 
diff --git a/gcc/gimplify.c b/gcc/gimplify.c
index d7cb7840a5d..7fd0ab297f2 100644
--- a/gcc/gimplify.c
+++ b/gcc/gimplify.c
@@ -2504,11 +2504,19 @@ gimplify_case_label_expr (tree *expr_p, gimple_seq *pre_p)
     if (ctxp->case_labels.exists ())
       break;
 
-  label_stmt = gimple_build_label (CASE_LABEL (*expr_p));
+  tree label = CASE_LABEL (*expr_p);
+  label_stmt = gimple_build_label (label);
   gimple_set_location (label_stmt, EXPR_LOCATION (*expr_p));
   ctxp->case_labels.safe_push (*expr_p);
   gimplify_seq_add_stmt (pre_p, label_stmt);
 
+  if (lookup_attribute ("cold", DECL_ATTRIBUTES (label)))
+    gimple_seq_add_stmt (pre_p, gimple_build_predict (PRED_COLD_LABEL,
+						      NOT_TAKEN));
+  else if (lookup_attribute ("hot", DECL_ATTRIBUTES (label)))
+    gimple_seq_add_stmt (pre_p, gimple_build_predict (PRED_HOT_LABEL,
+						      TAKEN));
+
   return GS_ALL_DONE;
 }
 
diff --git a/gcc/testsuite/g++.dg/cpp2a/attr-likely1.C b/gcc/testsuite/g++.dg/cpp2a/attr-likely1.C
new file mode 100644
index 00000000000..43de249bd5a
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp2a/attr-likely1.C
@@ -0,0 +1,38 @@
+// { dg-do compile { target c++2a } }
+// { dg-additional-options -fdump-tree-gimple }
+// { dg-final { scan-tree-dump-times "hot label" 5 "gimple" } }
+// { dg-final { scan-tree-dump-times "cold label" 3 "gimple" } }
+
+bool b;
+
+template <class T> int f()
+{
+  if (b)
+    [[likely]] return 0;
+  else
+    [[unlikely]] flabel: return 1;
+  switch (b)
+    {
+      [[likely]] case true: break;
+    };
+  return 1;
+}
+
+int main()
+{
+  if (b)
+    [[likely]] return 0;
+  else if (b)
+    [[unlikely]] elabel:
+      return 1;
+  else
+    [[likely]] b = false;
+
+  f<int>();
+
+  switch (b)
+    {
+      [[likely]] case true: break;
+      [[unlikely]] case false: break;
+    };
+}
diff --git a/gcc/testsuite/g++.dg/cpp2a/attr-likely2.C b/gcc/testsuite/g++.dg/cpp2a/attr-likely2.C
new file mode 100644
index 00000000000..3c951828c95
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp2a/attr-likely2.C
@@ -0,0 +1,10 @@
+// { dg-do compile { target c++2a } }
+
+bool b;
+int main()
+{
+  if (b)
+    [[likely, likely]] b;	// { dg-warning "ignoring" }
+  else
+    [[likely]] [[unlikely]] b;	// { dg-warning "ignoring" }
+}
diff --git a/gcc/testsuite/g++.dg/cpp2a/feat-cxx2a.C b/gcc/testsuite/g++.dg/cpp2a/feat-cxx2a.C
index dba77179b82..b80cc342364 100644
--- a/gcc/testsuite/g++.dg/cpp2a/feat-cxx2a.C
+++ b/gcc/testsuite/g++.dg/cpp2a/feat-cxx2a.C
@@ -456,6 +456,18 @@
 #    error "__has_cpp_attribute(no_unique_address) != 201803"
 #  endif
 
+#  if ! __has_cpp_attribute(likely)
+#    error "__has_cpp_attribute(likely)"
+#  elif __has_cpp_attribute(likely) != 201803
+#    error "__has_cpp_attribute(likely) != 201803"
+#  endif
+
+#  if ! __has_cpp_attribute(unlikely)
+#    error "__has_cpp_attribute(unlikely)"
+#  elif __has_cpp_attribute(unlikely) != 201803
+#    error "__has_cpp_attribute(unlikely) != 201803"
+#  endif
+
 #else
 #  error "__has_cpp_attribute"
 #endif
diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index 3f98c9001f4..52961030239 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,7 @@
+2018-11-12  Jason Merrill  <jason@redhat.com>
+
+	* gimplify.c (gimplify_case_label_expr): Handle hot/cold attributes.
+
 2018-11-13  Alan Modra  <amodra@gmail.com>
 
 	* config/rs6000/rs6000.c (rs6000_secondary_reload_inner): Negate
diff --git a/gcc/c-family/ChangeLog b/gcc/c-family/ChangeLog
index 165f9b7efcc..910dd628e87 100644
--- a/gcc/c-family/ChangeLog
+++ b/gcc/c-family/ChangeLog
@@ -1,3 +1,7 @@
+2018-11-11  Jason Merrill  <jason@redhat.com>
+
+	* c-lex.c (c_common_has_attribute): Handle likely/unlikely.
+
 2018-11-12  Jason Merrill  <jason@redhat.com>
 
 	* c-cppbuiltin.c (c_cpp_builtins): Define
diff --git a/gcc/cp/ChangeLog b/gcc/cp/ChangeLog
index 5497a0829e3..0da4b720e0e 100644
--- a/gcc/cp/ChangeLog
+++ b/gcc/cp/ChangeLog
@@ -1,3 +1,15 @@
+2018-11-12  Jason Merrill  <jason@redhat.com>
+
+	Implement P0479R5, [[likely]] and [[unlikely]].
+	* parser.c (cp_parser_std_attribute): Handle likely/unlikely.
+	(cp_parser_statement): Call process_stmt_hotness_attribute.
+	(cp_parser_label_for_labeled_statement): Apply attributes to case.
+	* cp-gimplify.c (lookup_hotness_attribute, remove_hotness_attribute)
+	(process_stmt_hotness_attribute): New.
+	* decl.c (finish_case_label): Give label in template type void.
+	* pt.c (tsubst_expr) [CASE_LABEL_EXPR]: Copy attributes.
+	[PREDICT_EXPR]: Handle.
+
 2018-11-12  Jason Merrill  <jason@redhat.com>
 
 	Implement P0722R3, destroying operator delete.

base-commit: 76b94d4ba654e9af1882865933343d11f5c3b18b
-- 
2.17.2

Re: RFC (branch prediction): PATCH to implement P0479R5, [[likely]] and [[unlikely]].

[Martin Liška]

[-- Attachment #1: Type: text/plain, Size: 19965 bytes --]

On 11/13/18 5:43 AM, Jason Merrill wrote:
> [[likely]] and [[unlikely]] are equivalent to the GNU hot/cold attributes,
> except that they can be applied to arbitrary statements as well as labels;
> this is most likely to be useful for marking if/else branches as likely or
> unlikely.  Conveniently, PREDICT_EXPR fits the bill nicely as a
> representation.
> 
> I also had to fix marking case labels as hot/cold, which didn't work before.
> Which then required me to force __attribute ((fallthrough)) to apply to the
> statement rather than the label.
> 
> Tested x86_64-pc-linux-gnu.  Does this seem like a sane implementation
> approach to people with more experience with PREDICT_EXPR?

Hi.

In general it makes sense to implement it the same way. Question is how much
should the hold/cold attribute should be close to __builtin_expect.

Let me present few examples and differences that I see:

1) ./xgcc -B. -O2 -fdump-tree-profile_estimate=/dev/stdout /tmp/test1.C

;; Function foo (_Z3foov, funcdef_no=0, decl_uid=2301, cgraph_uid=1, symbol_order=3)

Predictions for bb 2
  first match heuristics: 90.00%
  combined heuristics: 90.00%
  __builtin_expect heuristics of edge 2->3: 90.00%

As seen here __builtin_expect is stronger as it's first match heuristics and has probability == 90%.

;; Function bar (_Z3barv, funcdef_no=1, decl_uid=2303, cgraph_uid=2, symbol_order=4)

Predictions for bb 2
  DS theory heuristics: 74.48%
  combined heuristics: 74.48%
  opcode values nonequal (on trees) heuristics of edge 2->3: 34.00%
  hot label heuristics of edge 2->3: 85.00%

Here we combine hot label prediction with the opcode one, resulting in quite poor result 75%.
So maybe cold/hot prediction cal also happen first match.

2) ./xgcc -B. -O2 -fdump-tree-profile_estimate=/dev/stdout /tmp/test2.C
...
foo ()
{
...
  switch (_3) <default: <L3> [3.33%], case 3: <L0> [3.33%], case 42: <L1> [3.33%], case 333: <L2> [90.00%]>

while:

bar ()
{
  switch (a.1_1) <default: <L3> [25.00%], case 3: <L0> [25.00%], case 42: <L1> [25.00%], case 333: <L2> [25.00%]>
...

Note that support for __builtin_expect was enhanced in this stage1. I can definitely cover also situations when one uses
hot/cold for labels. So definitely place for improvement.

3) last example where one can use the attribute for function decl, resulting in:
__attribute__((hot, noinline))
foo ()
{
..

Hope it's desired? If so I would cover that with a test-case in test-suite.

Jason can you please point to C++ specification of the attributes?
Would you please consider an error diagnostics for situations written in test4.C?
Such situation is then silently ignored in profile_estimate pass:

Predictions for bb 2
  hot label heuristics of edge 2->4 (edge pair duplicate): 85.00%
  hot label heuristics of edge 2->3 (edge pair duplicate): 85.00%
...

Thanks,
Martin

> 
> gcc/
> 	* gimplify.c (gimplify_case_label_expr): Handle hot/cold attributes.
> gcc/c-family/
> 	* c-lex.c (c_common_has_attribute): Handle likely/unlikely.
> gcc/cp/
> 	* parser.c (cp_parser_std_attribute): Handle likely/unlikely.
> 	(cp_parser_statement): Call process_stmt_hotness_attribute.
> 	(cp_parser_label_for_labeled_statement): Apply attributes to case.
> 	* cp-gimplify.c (lookup_hotness_attribute, remove_hotness_attribute)
> 	(process_stmt_hotness_attribute): New.
> 	* decl.c (finish_case_label): Give label in template type void.
> 	* pt.c (tsubst_expr) [CASE_LABEL_EXPR]: Copy attributes.
> 	[PREDICT_EXPR]: Handle.
> ---
>  gcc/cp/cp-tree.h                          |  2 +
>  gcc/c-family/c-lex.c                      |  4 +-
>  gcc/cp/cp-gimplify.c                      | 42 +++++++++++++++++++++
>  gcc/cp/decl.c                             |  2 +-
>  gcc/cp/parser.c                           | 45 +++++++++++++++++++----
>  gcc/cp/pt.c                               | 12 +++++-
>  gcc/gimplify.c                            | 10 ++++-
>  gcc/testsuite/g++.dg/cpp2a/attr-likely1.C | 38 +++++++++++++++++++
>  gcc/testsuite/g++.dg/cpp2a/attr-likely2.C | 10 +++++
>  gcc/testsuite/g++.dg/cpp2a/feat-cxx2a.C   | 12 ++++++
>  gcc/ChangeLog                             |  4 ++
>  gcc/c-family/ChangeLog                    |  4 ++
>  gcc/cp/ChangeLog                          | 12 ++++++
>  13 files changed, 184 insertions(+), 13 deletions(-)
>  create mode 100644 gcc/testsuite/g++.dg/cpp2a/attr-likely1.C
>  create mode 100644 gcc/testsuite/g++.dg/cpp2a/attr-likely2.C
> 
> diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
> index c4d79c0cf7f..c55352ec5ff 100644
> --- a/gcc/cp/cp-tree.h
> +++ b/gcc/cp/cp-tree.h
> @@ -7541,6 +7541,8 @@ extern bool cxx_omp_disregard_value_expr	(tree, bool);
>  extern void cp_fold_function			(tree);
>  extern tree cp_fully_fold			(tree);
>  extern void clear_fold_cache			(void);
> +extern tree lookup_hotness_attribute		(tree);
> +extern tree process_stmt_hotness_attribute	(tree);
>  
>  /* in name-lookup.c */
>  extern tree strip_using_decl                    (tree);
> diff --git a/gcc/c-family/c-lex.c b/gcc/c-family/c-lex.c
> index 28a820a2a3d..3cc015083e0 100644
> --- a/gcc/c-family/c-lex.c
> +++ b/gcc/c-family/c-lex.c
> @@ -356,7 +356,9 @@ c_common_has_attribute (cpp_reader *pfile)
>  		       || is_attribute_p ("nodiscard", attr_name)
>  		       || is_attribute_p ("fallthrough", attr_name))
>  		result = 201603;
> -	      else if (is_attribute_p ("no_unique_address", attr_name))
> +	      else if (is_attribute_p ("no_unique_address", attr_name)
> +		       || is_attribute_p ("likely", attr_name)
> +		       || is_attribute_p ("unlikely", attr_name))
>  		result = 201803;
>  	      if (result)
>  		attr_name = NULL_TREE;
> diff --git a/gcc/cp/cp-gimplify.c b/gcc/cp/cp-gimplify.c
> index eb761b118a1..f8212187162 100644
> --- a/gcc/cp/cp-gimplify.c
> +++ b/gcc/cp/cp-gimplify.c
> @@ -2674,4 +2674,46 @@ cp_fold (tree x)
>    return x;
>  }
>  
> +/* Look up either "hot" or "cold" in attribute list LIST.  */
> +
> +tree
> +lookup_hotness_attribute (tree list)
> +{
> +  tree attr = lookup_attribute ("hot", list);
> +  if (attr)
> +    return attr;
> +  return lookup_attribute ("cold", list);
> +}
> +
> +/* Remove both "hot" and "cold" attributes from LIST.  */
> +
> +static tree
> +remove_hotness_attribute (tree list)
> +{
> +  return remove_attribute ("hot", remove_attribute ("cold", list));
> +}
> +
> +/* If [[likely]] or [[unlikely]] appear on this statement, turn it into a
> +   PREDICT_EXPR.  */
> +
> +tree
> +process_stmt_hotness_attribute (tree std_attrs)
> +{
> +  if (std_attrs == error_mark_node)
> +    return std_attrs;
> +  if (tree attr = lookup_hotness_attribute (std_attrs))
> +    {
> +      tree name = get_attribute_name (attr);
> +      bool hot = is_attribute_p ("hot", name);
> +      tree pred = build_predict_expr (hot ? PRED_HOT_LABEL : PRED_COLD_LABEL,
> +				      hot ? TAKEN : NOT_TAKEN);
> +      add_stmt (pred);
> +      if (tree other = lookup_hotness_attribute (TREE_CHAIN (attr)))
> +	warning (OPT_Wattributes, "ignoring attribute %qE after earlier %qE",
> +		 get_attribute_name (other), name);
> +      std_attrs = remove_hotness_attribute (std_attrs);
> +    }
> +  return std_attrs;
> +}
> +
>  #include "gt-cp-cp-gimplify.h"
> diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c
> index 42994055d5f..73f27038e5c 100644
> --- a/gcc/cp/decl.c
> +++ b/gcc/cp/decl.c
> @@ -3624,7 +3624,7 @@ finish_case_label (location_t loc, tree low_value, tree high_value)
>  
>        /* For templates, just add the case label; we'll do semantic
>  	 analysis at instantiation-time.  */
> -      label = build_decl (loc, LABEL_DECL, NULL_TREE, NULL_TREE);
> +      label = build_decl (loc, LABEL_DECL, NULL_TREE, void_type_node);
>        return add_stmt (build_case_label (low_value, high_value, label));
>      }
>  
> diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
> index 0428f6dda90..bdc35fa0cb8 100644
> --- a/gcc/cp/parser.c
> +++ b/gcc/cp/parser.c
> @@ -10875,6 +10875,7 @@ cp_parser_statement (cp_parser* parser, tree in_statement_expr,
>    /* Peek at the next token.  */
>    token = cp_lexer_peek_token (parser->lexer);
>    /* Remember the location of the first token in the statement.  */
> +  cp_token *statement_token = token;
>    statement_location = token->location;
>    add_debug_begin_stmt (statement_location);
>    /* If this is a keyword, then that will often determine what kind of
> @@ -10896,12 +10897,14 @@ cp_parser_statement (cp_parser* parser, tree in_statement_expr,
>  
>  	case RID_IF:
>  	case RID_SWITCH:
> +	  std_attrs = process_stmt_hotness_attribute (std_attrs);
>  	  statement = cp_parser_selection_statement (parser, if_p, chain);
>  	  break;
>  
>  	case RID_WHILE:
>  	case RID_DO:
>  	case RID_FOR:
> +	  std_attrs = process_stmt_hotness_attribute (std_attrs);
>  	  statement = cp_parser_iteration_statement (parser, if_p, false, 0);
>  	  break;
>  
> @@ -10909,6 +10912,7 @@ cp_parser_statement (cp_parser* parser, tree in_statement_expr,
>  	case RID_CONTINUE:
>  	case RID_RETURN:
>  	case RID_GOTO:
> +	  std_attrs = process_stmt_hotness_attribute (std_attrs);
>  	  statement = cp_parser_jump_statement (parser);
>  	  break;
>  
> @@ -10918,15 +10922,24 @@ cp_parser_statement (cp_parser* parser, tree in_statement_expr,
>  	case RID_AT_FINALLY:
>  	case RID_AT_SYNCHRONIZED:
>  	case RID_AT_THROW:
> +	  std_attrs = process_stmt_hotness_attribute (std_attrs);
>  	  statement = cp_parser_objc_statement (parser);
>  	  break;
>  
>  	case RID_TRY:
> +	  std_attrs = process_stmt_hotness_attribute (std_attrs);
>  	  statement = cp_parser_try_block (parser);
>  	  break;
>  
>  	case RID_NAMESPACE:
>  	  /* This must be a namespace alias definition.  */
> +	  if (std_attrs != NULL_TREE)
> +	    {
> +	      /*  Attributes should be parsed as part of the the
> +		  declaration, so let's un-parse them.  */
> +	      saved_tokens.rollback();
> +	      std_attrs = NULL_TREE;
> +	    }
>  	  cp_parser_declaration_statement (parser);
>  	  return;
>  	  
> @@ -10935,9 +10948,11 @@ cp_parser_statement (cp_parser* parser, tree in_statement_expr,
>  	case RID_SYNCHRONIZED:
>  	case RID_ATOMIC_NOEXCEPT:
>  	case RID_ATOMIC_CANCEL:
> +	  std_attrs = process_stmt_hotness_attribute (std_attrs);
>  	  statement = cp_parser_transaction (parser, token);
>  	  break;
>  	case RID_TRANSACTION_CANCEL:
> +	  std_attrs = process_stmt_hotness_attribute (std_attrs);
>  	  statement = cp_parser_transaction_cancel (parser);
>  	  break;
>  
> @@ -10996,12 +11011,9 @@ cp_parser_statement (cp_parser* parser, tree in_statement_expr,
>        if (cp_lexer_next_token_is_not (parser->lexer, CPP_SEMICOLON))
>  	{
>  	  if (std_attrs != NULL_TREE)
> -	    {
> -	      /*  Attributes should be parsed as part of the the
> -		  declaration, so let's un-parse them.  */
> -	      saved_tokens.rollback();
> -	      std_attrs = NULL_TREE;
> -	    }
> +	    /* Attributes should be parsed as part of the declaration,
> +	       so let's un-parse them.  */
> +	    saved_tokens.rollback();
>  
>  	  cp_parser_parse_tentatively (parser);
>  	  /* Try to parse the declaration-statement.  */
> @@ -11009,11 +11021,16 @@ cp_parser_statement (cp_parser* parser, tree in_statement_expr,
>  	  /* If that worked, we're done.  */
>  	  if (cp_parser_parse_definitely (parser))
>  	    return;
> +	  /* It didn't work, restore the post-attribute position.  */
> +	  if (std_attrs)
> +	    cp_lexer_set_token_position (parser->lexer, statement_token);
>  	}
>        /* All preceding labels have been parsed at this point.  */
>        if (loc_after_labels != NULL)
>  	*loc_after_labels = statement_location;
>  
> +      std_attrs = process_stmt_hotness_attribute (std_attrs);
> +
>        /* Look for an expression-statement instead.  */
>        statement = cp_parser_expression_statement (parser, in_statement_expr);
>  
> @@ -11126,7 +11143,10 @@ cp_parser_label_for_labeled_statement (cp_parser* parser, tree attributes)
>  	  {
>  	    tree l = finish_case_label (token->location, expr, expr_hi);
>  	    if (l && TREE_CODE (l) == CASE_LABEL_EXPR)
> -	      FALLTHROUGH_LABEL_P (CASE_LABEL (l)) = fallthrough_p;
> +	      {
> +		label = CASE_LABEL (l);
> +		FALLTHROUGH_LABEL_P (label) = fallthrough_p;
> +	      }
>  	  }
>  	else
>  	  error_at (token->location,
> @@ -11143,7 +11163,10 @@ cp_parser_label_for_labeled_statement (cp_parser* parser, tree attributes)
>  	{
>  	  tree l = finish_case_label (token->location, NULL_TREE, NULL_TREE);
>  	  if (l && TREE_CODE (l) == CASE_LABEL_EXPR)
> -	    FALLTHROUGH_LABEL_P (CASE_LABEL (l)) = fallthrough_p;
> +	      {
> +		label = CASE_LABEL (l);
> +		FALLTHROUGH_LABEL_P (label) = fallthrough_p;
> +	      }
>  	}
>        else
>  	error_at (token->location, "case label not within a switch statement");
> @@ -11173,6 +11196,8 @@ cp_parser_label_for_labeled_statement (cp_parser* parser, tree attributes)
>        cp_parser_parse_tentatively (parser);
>        attrs = cp_parser_gnu_attributes_opt (parser);
>        if (attrs == NULL_TREE
> +	  /* And fallthrough always binds to the expression-statement.  */
> +	  || attribute_fallthrough_p (attrs)
>  	  || cp_lexer_next_token_is_not (parser->lexer, CPP_SEMICOLON))
>  	cp_parser_abort_tentative_parse (parser);
>        else if (!cp_parser_parse_definitely (parser))
> @@ -25528,6 +25553,10 @@ cp_parser_std_attribute (cp_parser *parser, tree attr_ns)
>        else if (is_attribute_p ("optimize_for_synchronized", attr_id))
>  	TREE_PURPOSE (attribute)
>  	  = get_identifier ("transaction_callable");
> +      else if (is_attribute_p ("likely", attr_id))
> +	TREE_PURPOSE (attribute) = get_identifier ("hot");
> +      else if (is_attribute_p ("unlikely", attr_id))
> +	TREE_PURPOSE (attribute) = get_identifier ("cold");
>        /* Transactional Memory attributes are GNU attributes.  */
>        else if (tm_attr_to_mask (attr_id))
>  	TREE_PURPOSE (attribute) = attr_id;
> diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
> index 0c33c8e1527..dfd9a436465 100644
> --- a/gcc/cp/pt.c
> +++ b/gcc/cp/pt.c
> @@ -17175,12 +17175,17 @@ tsubst_expr (tree t, tree args, tsubst_flags_t complain, tree in_decl,
>  
>      case CASE_LABEL_EXPR:
>        {
> +	tree decl = CASE_LABEL (t);
>  	tree low = RECUR (CASE_LOW (t));
>  	tree high = RECUR (CASE_HIGH (t));
>  	tree l = finish_case_label (EXPR_LOCATION (t), low, high);
>  	if (l && TREE_CODE (l) == CASE_LABEL_EXPR)
> -	  FALLTHROUGH_LABEL_P (CASE_LABEL (l))
> -	    = FALLTHROUGH_LABEL_P (CASE_LABEL (t));
> +	  {
> +	    tree label = CASE_LABEL (l);
> +	    FALLTHROUGH_LABEL_P (label) = FALLTHROUGH_LABEL_P (decl);
> +	    if (DECL_ATTRIBUTES (decl) != NULL_TREE)
> +	      cplus_decl_attributes (&label, DECL_ATTRIBUTES (decl), 0);
> +	  }
>        }
>        break;
>  
> @@ -17731,6 +17736,9 @@ tsubst_expr (tree t, tree args, tsubst_flags_t complain, tree in_decl,
>  			  RECUR (TREE_OPERAND (t, 1)),
>  			  RECUR (TREE_OPERAND (t, 2))));
>  
> +    case PREDICT_EXPR:
> +      RETURN (add_stmt (copy_node (t)));
> +
>      default:
>        gcc_assert (!STATEMENT_CODE_P (TREE_CODE (t)));
>  
> diff --git a/gcc/gimplify.c b/gcc/gimplify.c
> index d7cb7840a5d..7fd0ab297f2 100644
> --- a/gcc/gimplify.c
> +++ b/gcc/gimplify.c
> @@ -2504,11 +2504,19 @@ gimplify_case_label_expr (tree *expr_p, gimple_seq *pre_p)
>      if (ctxp->case_labels.exists ())
>        break;
>  
> -  label_stmt = gimple_build_label (CASE_LABEL (*expr_p));
> +  tree label = CASE_LABEL (*expr_p);
> +  label_stmt = gimple_build_label (label);
>    gimple_set_location (label_stmt, EXPR_LOCATION (*expr_p));
>    ctxp->case_labels.safe_push (*expr_p);
>    gimplify_seq_add_stmt (pre_p, label_stmt);
>  
> +  if (lookup_attribute ("cold", DECL_ATTRIBUTES (label)))
> +    gimple_seq_add_stmt (pre_p, gimple_build_predict (PRED_COLD_LABEL,
> +						      NOT_TAKEN));
> +  else if (lookup_attribute ("hot", DECL_ATTRIBUTES (label)))
> +    gimple_seq_add_stmt (pre_p, gimple_build_predict (PRED_HOT_LABEL,
> +						      TAKEN));
> +
>    return GS_ALL_DONE;
>  }
>  
> diff --git a/gcc/testsuite/g++.dg/cpp2a/attr-likely1.C b/gcc/testsuite/g++.dg/cpp2a/attr-likely1.C
> new file mode 100644
> index 00000000000..43de249bd5a
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/cpp2a/attr-likely1.C
> @@ -0,0 +1,38 @@
> +// { dg-do compile { target c++2a } }
> +// { dg-additional-options -fdump-tree-gimple }
> +// { dg-final { scan-tree-dump-times "hot label" 5 "gimple" } }
> +// { dg-final { scan-tree-dump-times "cold label" 3 "gimple" } }
> +
> +bool b;
> +
> +template <class T> int f()
> +{
> +  if (b)
> +    [[likely]] return 0;
> +  else
> +    [[unlikely]] flabel: return 1;
> +  switch (b)
> +    {
> +      [[likely]] case true: break;
> +    };
> +  return 1;
> +}
> +
> +int main()
> +{
> +  if (b)
> +    [[likely]] return 0;
> +  else if (b)
> +    [[unlikely]] elabel:
> +      return 1;
> +  else
> +    [[likely]] b = false;
> +
> +  f<int>();
> +
> +  switch (b)
> +    {
> +      [[likely]] case true: break;
> +      [[unlikely]] case false: break;
> +    };
> +}
> diff --git a/gcc/testsuite/g++.dg/cpp2a/attr-likely2.C b/gcc/testsuite/g++.dg/cpp2a/attr-likely2.C
> new file mode 100644
> index 00000000000..3c951828c95
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/cpp2a/attr-likely2.C
> @@ -0,0 +1,10 @@
> +// { dg-do compile { target c++2a } }
> +
> +bool b;
> +int main()
> +{
> +  if (b)
> +    [[likely, likely]] b;	// { dg-warning "ignoring" }
> +  else
> +    [[likely]] [[unlikely]] b;	// { dg-warning "ignoring" }
> +}
> diff --git a/gcc/testsuite/g++.dg/cpp2a/feat-cxx2a.C b/gcc/testsuite/g++.dg/cpp2a/feat-cxx2a.C
> index dba77179b82..b80cc342364 100644
> --- a/gcc/testsuite/g++.dg/cpp2a/feat-cxx2a.C
> +++ b/gcc/testsuite/g++.dg/cpp2a/feat-cxx2a.C
> @@ -456,6 +456,18 @@
>  #    error "__has_cpp_attribute(no_unique_address) != 201803"
>  #  endif
>  
> +#  if ! __has_cpp_attribute(likely)
> +#    error "__has_cpp_attribute(likely)"
> +#  elif __has_cpp_attribute(likely) != 201803
> +#    error "__has_cpp_attribute(likely) != 201803"
> +#  endif
> +
> +#  if ! __has_cpp_attribute(unlikely)
> +#    error "__has_cpp_attribute(unlikely)"
> +#  elif __has_cpp_attribute(unlikely) != 201803
> +#    error "__has_cpp_attribute(unlikely) != 201803"
> +#  endif
> +
>  #else
>  #  error "__has_cpp_attribute"
>  #endif
> diff --git a/gcc/ChangeLog b/gcc/ChangeLog
> index 3f98c9001f4..52961030239 100644
> --- a/gcc/ChangeLog
> +++ b/gcc/ChangeLog
> @@ -1,3 +1,7 @@
> +2018-11-12  Jason Merrill  <jason@redhat.com>
> +
> +	* gimplify.c (gimplify_case_label_expr): Handle hot/cold attributes.
> +
>  2018-11-13  Alan Modra  <amodra@gmail.com>
>  
>  	* config/rs6000/rs6000.c (rs6000_secondary_reload_inner): Negate
> diff --git a/gcc/c-family/ChangeLog b/gcc/c-family/ChangeLog
> index 165f9b7efcc..910dd628e87 100644
> --- a/gcc/c-family/ChangeLog
> +++ b/gcc/c-family/ChangeLog
> @@ -1,3 +1,7 @@
> +2018-11-11  Jason Merrill  <jason@redhat.com>
> +
> +	* c-lex.c (c_common_has_attribute): Handle likely/unlikely.
> +
>  2018-11-12  Jason Merrill  <jason@redhat.com>
>  
>  	* c-cppbuiltin.c (c_cpp_builtins): Define
> diff --git a/gcc/cp/ChangeLog b/gcc/cp/ChangeLog
> index 5497a0829e3..0da4b720e0e 100644
> --- a/gcc/cp/ChangeLog
> +++ b/gcc/cp/ChangeLog
> @@ -1,3 +1,15 @@
> +2018-11-12  Jason Merrill  <jason@redhat.com>
> +
> +	Implement P0479R5, [[likely]] and [[unlikely]].
> +	* parser.c (cp_parser_std_attribute): Handle likely/unlikely.
> +	(cp_parser_statement): Call process_stmt_hotness_attribute.
> +	(cp_parser_label_for_labeled_statement): Apply attributes to case.
> +	* cp-gimplify.c (lookup_hotness_attribute, remove_hotness_attribute)
> +	(process_stmt_hotness_attribute): New.
> +	* decl.c (finish_case_label): Give label in template type void.
> +	* pt.c (tsubst_expr) [CASE_LABEL_EXPR]: Copy attributes.
> +	[PREDICT_EXPR]: Handle.
> +
>  2018-11-12  Jason Merrill  <jason@redhat.com>
>  
>  	Implement P0722R3, destroying operator delete.
> 
> base-commit: 76b94d4ba654e9af1882865933343d11f5c3b18b
> 


[-- Attachment #2: test1.C --]
[-- Type: text/x-c++src, Size: 232 bytes --]

int a, b, c;

void
__attribute__((noinline))
foo()
{
  if (__builtin_expect (a == 123, 1))
    c = 5;
}

void
__attribute__((noinline))
bar()
{
  if (a == 123)
    [[likely]] c = 5;
}

int main()
{
  foo ();
  bar ();
  return 0;
}

[-- Attachment #3: test2.C --]
[-- Type: text/x-c++src, Size: 419 bytes --]

int a, b, c;

void
foo ()
{
  switch (__builtin_expect(a, 333))
  {
    case 3: __builtin_puts("a"); break;
    case 42: __builtin_puts("e"); break;
    case 333: __builtin_puts("i"); break;
  } 
}

void
bar ()
{
  switch (a)
  {
    case 3: __builtin_puts("a"); break;
    case 42: __builtin_puts("e"); break;
    [[likely]] case 333: __builtin_puts("i"); break;
  } 
}

int main()
{
  foo ();
  bar ();
  return 0;
}

[-- Attachment #4: test3.C --]
[-- Type: text/x-c++src, Size: 287 bytes --]

int a, b, c;

void
__attribute__((noinline))
[[likely]]
foo()
{
  if (__builtin_expect (a == 123, 1))
    c = 5;
}

void
__attribute__((noinline))
bar()
{
  if (a == 123)
    [[likely]] c = 5;
}

int main(int argc, char **argv)
{
  if (argc)
    foo ();
  else
    bar ();
  return 0;
}

[-- Attachment #5: test4.C --]
[-- Type: text/x-c++src, Size: 161 bytes --]

int a, b, c;

void
__attribute__((noinline))
bar()
{
  if (a == 123)
    [[likely]] c = 5;
  else
    [[likely]] b = 77;
}

int main()
{
  bar ();
  return 0;
}

Re: RFC (branch prediction): PATCH to implement P0479R5, [[likely]] and [[unlikely]].

[Jason Merrill]

On Tue, Nov 13, 2018 at 9:20 AM Martin Liška <mliska@suse.cz> wrote:
>
> On 11/13/18 5:43 AM, Jason Merrill wrote:
> > [[likely]] and [[unlikely]] are equivalent to the GNU hot/cold attributes,
> > except that they can be applied to arbitrary statements as well as labels;
> > this is most likely to be useful for marking if/else branches as likely or
> > unlikely.  Conveniently, PREDICT_EXPR fits the bill nicely as a
> > representation.
> >
> > I also had to fix marking case labels as hot/cold, which didn't work before.
> > Which then required me to force __attribute ((fallthrough)) to apply to the
> > statement rather than the label.
> >
> > Tested x86_64-pc-linux-gnu.  Does this seem like a sane implementation
> > approach to people with more experience with PREDICT_EXPR?
>
> Hi.
>
> In general it makes sense to implement it the same way. Question is how much
> should the hold/cold attribute should be close to __builtin_expect.
>
> Let me present few examples and differences that I see:
>
> 1) ./xgcc -B. -O2 -fdump-tree-profile_estimate=/dev/stdout /tmp/test1.C
>
> ;; Function foo (_Z3foov, funcdef_no=0, decl_uid=2301, cgraph_uid=1, symbol_order=3)
>
> Predictions for bb 2
>   first match heuristics: 90.00%
>   combined heuristics: 90.00%
>   __builtin_expect heuristics of edge 2->3: 90.00%
>
> As seen here __builtin_expect is stronger as it's first match heuristics and has probability == 90%.
>
> ;; Function bar (_Z3barv, funcdef_no=1, decl_uid=2303, cgraph_uid=2, symbol_order=4)
>
> Predictions for bb 2
>   DS theory heuristics: 74.48%
>   combined heuristics: 74.48%
>   opcode values nonequal (on trees) heuristics of edge 2->3: 34.00%
>   hot label heuristics of edge 2->3: 85.00%
>
> Here we combine hot label prediction with the opcode one, resulting in quite poor result 75%.
> So maybe cold/hot prediction cal also happen first match.

Makes sense.

> 2) ./xgcc -B. -O2 -fdump-tree-profile_estimate=/dev/stdout /tmp/test2.C
> ...
> foo ()
> {
> ...
>   switch (_3) <default: <L3> [3.33%], case 3: <L0> [3.33%], case 42: <L1> [3.33%], case 333: <L2> [90.00%]>
>
> while:
>
> bar ()
> {
>   switch (a.1_1) <default: <L3> [25.00%], case 3: <L0> [25.00%], case 42: <L1> [25.00%], case 333: <L2> [25.00%]>
> ...
>
> Note that support for __builtin_expect was enhanced in this stage1. I can definitely cover also situations when one uses
> hot/cold for labels. So definitely place for improvement.

Hmm, the gimplifier should be adding a PREDICT_EXPR for the case label
now, is it just ignored currently?

> 3) last example where one can use the attribute for function decl, resulting in:
> __attribute__((hot, noinline))
> foo ()
> {
> ..
>
> Hope it's desired? If so I would cover that with a test-case in test-suite.

[[likely]] and [[unlikely]] don't apply to functions; I suppose I
should diagnose that.

> Jason can you please point to C++ specification of the attributes?

http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2018/p0479r5.html

> Would you please consider an error diagnostics for situations written in test4.C?

A warning seems appropriate.  You think the front end is the right
place for that?

Jason

Re: RFC (branch prediction): PATCH to implement P0479R5, [[likely]] and [[unlikely]].

[Martin Liška]

On 11/13/18 8:42 PM, Jason Merrill wrote:
> On Tue, Nov 13, 2018 at 9:20 AM Martin Liška <mliska@suse.cz> wrote:
>>
>> On 11/13/18 5:43 AM, Jason Merrill wrote:
>>> [[likely]] and [[unlikely]] are equivalent to the GNU hot/cold attributes,
>>> except that they can be applied to arbitrary statements as well as labels;
>>> this is most likely to be useful for marking if/else branches as likely or
>>> unlikely.  Conveniently, PREDICT_EXPR fits the bill nicely as a
>>> representation.
>>>
>>> I also had to fix marking case labels as hot/cold, which didn't work before.
>>> Which then required me to force __attribute ((fallthrough)) to apply to the
>>> statement rather than the label.
>>>
>>> Tested x86_64-pc-linux-gnu.  Does this seem like a sane implementation
>>> approach to people with more experience with PREDICT_EXPR?
>>
>> Hi.
>>
>> In general it makes sense to implement it the same way. Question is how much
>> should the hold/cold attribute should be close to __builtin_expect.
>>
>> Let me present few examples and differences that I see:
>>
>> 1) ./xgcc -B. -O2 -fdump-tree-profile_estimate=/dev/stdout /tmp/test1.C
>>
>> ;; Function foo (_Z3foov, funcdef_no=0, decl_uid=2301, cgraph_uid=1, symbol_order=3)
>>
>> Predictions for bb 2
>>   first match heuristics: 90.00%
>>   combined heuristics: 90.00%
>>   __builtin_expect heuristics of edge 2->3: 90.00%
>>
>> As seen here __builtin_expect is stronger as it's first match heuristics and has probability == 90%.
>>
>> ;; Function bar (_Z3barv, funcdef_no=1, decl_uid=2303, cgraph_uid=2, symbol_order=4)
>>
>> Predictions for bb 2
>>   DS theory heuristics: 74.48%
>>   combined heuristics: 74.48%
>>   opcode values nonequal (on trees) heuristics of edge 2->3: 34.00%
>>   hot label heuristics of edge 2->3: 85.00%
>>
>> Here we combine hot label prediction with the opcode one, resulting in quite poor result 75%.
>> So maybe cold/hot prediction cal also happen first match.
> 
> Makes sense.

Good, let me prepare patch for it.

> 
>> 2) ./xgcc -B. -O2 -fdump-tree-profile_estimate=/dev/stdout /tmp/test2.C
>> ...
>> foo ()
>> {
>> ...
>>   switch (_3) <default: <L3> [3.33%], case 3: <L0> [3.33%], case 42: <L1> [3.33%], case 333: <L2> [90.00%]>
>>
>> while:
>>
>> bar ()
>> {
>>   switch (a.1_1) <default: <L3> [25.00%], case 3: <L0> [25.00%], case 42: <L1> [25.00%], case 333: <L2> [25.00%]>
>> ...
>>
>> Note that support for __builtin_expect was enhanced in this stage1. I can definitely cover also situations when one uses
>> hot/cold for labels. So definitely place for improvement.
> 
> Hmm, the gimplifier should be adding a PREDICT_EXPR for the case label
> now, is it just ignored currently?

Yes, for switch multi-branches yes. I'll prepare patch for it, should be quite straightforward.

> 
>> 3) last example where one can use the attribute for function decl, resulting in:
>> __attribute__((hot, noinline))
>> foo ()
>> {
>> ..
>>
>> Hope it's desired? If so I would cover that with a test-case in test-suite.
> 
> [[likely]] and [[unlikely]] don't apply to functions; I suppose I
> should diagnose that.

Yes.

> 
>> Jason can you please point to C++ specification of the attributes?
> 
> http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2018/p0479r5.html
> 
>> Would you please consider an error diagnostics for situations written in test4.C?
> 
> A warning seems appropriate.  You think the front end is the right
> place for that?

Probably yes. Note that middle-end can optimize about dead branches and so that
theoretically one can end up with a branching where e.g. both branches are [[likely]].
I wouldn't bother users with these.

Martin

> 
> Jason
> 

Re: RFC (branch prediction): PATCH to implement P0479R5, [[likely]] and [[unlikely]].

[Jan Hubicka]

> > A warning seems appropriate.  You think the front end is the right
> > place for that?
> 
> Probably yes. Note that middle-end can optimize about dead branches and so that
> theoretically one can end up with a branching where e.g. both branches are [[likely]].
> I wouldn't bother users with these.

Note that what really happens in this case is that if conditional is
constant propagated and it has predict_expr, the predict_expr stays and
will get assigned to the random control dependence edge which controls
execution of the original statement.  This is not very intuitive
behaviour. Does C++ say what should happen in this case?
One option would be to deal with this gratefully at high level gimple
and turn predict_exprs into edge probabilities eariler than we do normal
branch prediction (which is intended to be later so profile does not end
up unnecesarily inconsistent)

Honza
> 
> Martin
> 
> > 
> > Jason
> > 
> 

Re: RFC (branch prediction): PATCH to implement P0479R5, [[likely]] and [[unlikely]].

[Jason Merrill]

[-- Attachment #1: Type: text/plain, Size: 1092 bytes --]

On Thu, Nov 15, 2018 at 7:14 AM Jan Hubicka <hubicka@ucw.cz> wrote:
>
> > > A warning seems appropriate.  You think the front end is the right
> > > place for that?
> >
> > Probably yes. Note that middle-end can optimize about dead branches and so that
> > theoretically one can end up with a branching where e.g. both branches are [[likely]].
> > I wouldn't bother users with these.
>
> Note that what really happens in this case is that if conditional is
> constant propagated and it has predict_expr, the predict_expr stays and
> will get assigned to the random control dependence edge which controls
> execution of the original statement.  This is not very intuitive
> behaviour. Does C++ say what should happen in this case?

No, it doesn't say much.

> One option would be to deal with this gratefully at high level gimple
> and turn predict_exprs into edge probabilities eariler than we do normal
> branch prediction (which is intended to be later so profile does not end
> up unnecesarily inconsistent)

Sounds reasonable.

This additional patch implements the suggested diagnostics.

[-- Attachment #2: likely-2.diff --]
[-- Type: text/x-patch, Size: 9757 bytes --]

diff --git a/gcc/c-family/c-common.h b/gcc/c-family/c-common.h
index 8eeeba75319..4aa61426114 100644
--- a/gcc/c-family/c-common.h
+++ b/gcc/c-family/c-common.h
@@ -1330,6 +1330,7 @@ extern int parse_tm_stmt_attr (tree, int);
 extern int tm_attr_to_mask (tree);
 extern tree tm_mask_to_attr (int);
 extern tree find_tm_attribute (tree);
+extern const struct attribute_spec::exclusions attr_cold_hot_exclusions[];
 
 /* A bitmap of flags to positional_argument.  */
 enum posargflags {
diff --git a/gcc/c-family/c-attribs.c b/gcc/c-family/c-attribs.c
index c9afa1f78f4..88b24cd6cd4 100644
--- a/gcc/c-family/c-attribs.c
+++ b/gcc/c-family/c-attribs.c
@@ -163,7 +163,7 @@ static const struct attribute_spec::exclusions attr_aligned_exclusions[] =
   ATTR_EXCL (NULL, false, false, false)
 };
 
-static const struct attribute_spec::exclusions attr_cold_hot_exclusions[] =
+extern const struct attribute_spec::exclusions attr_cold_hot_exclusions[] =
 {
   ATTR_EXCL ("cold", true, true, true),
   ATTR_EXCL ("hot", true, true, true),
diff --git a/gcc/cp/cp-gimplify.c b/gcc/cp/cp-gimplify.c
index f8212187162..5cb54adf60f 100644
--- a/gcc/cp/cp-gimplify.c
+++ b/gcc/cp/cp-gimplify.c
@@ -34,6 +34,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "stringpool.h"
 #include "attribs.h"
 #include "asan.h"
+#include "gcc-rich-location.h"
 
 /* Forward declarations.  */
 
@@ -158,6 +159,26 @@ genericize_eh_spec_block (tree *stmt_p)
   TREE_NO_WARNING (TREE_OPERAND (*stmt_p, 1)) = true;
 }
 
+/* Return the first non-compound statement in STMT.  */
+
+tree
+first_stmt (tree stmt)
+{
+  switch (TREE_CODE (stmt))
+    {
+    case STATEMENT_LIST:
+      if (tree_statement_list_node *p = STATEMENT_LIST_HEAD (stmt))
+	return first_stmt (p->stmt);
+      return void_node;
+
+    case BIND_EXPR:
+      return first_stmt (BIND_EXPR_BODY (stmt));
+
+    default:
+      return stmt;
+    }
+}
+
 /* Genericize an IF_STMT by turning it into a COND_EXPR.  */
 
 static void
@@ -171,6 +192,24 @@ genericize_if_stmt (tree *stmt_p)
   then_ = THEN_CLAUSE (stmt);
   else_ = ELSE_CLAUSE (stmt);
 
+  if (then_ && else_)
+    {
+      tree ft = first_stmt (then_);
+      tree fe = first_stmt (else_);
+      br_predictor pr;
+      if (TREE_CODE (ft) == PREDICT_EXPR
+	  && TREE_CODE (fe) == PREDICT_EXPR
+	  && (pr = PREDICT_EXPR_PREDICTOR (ft)) == PREDICT_EXPR_PREDICTOR (fe)
+	  && (pr == PRED_HOT_LABEL || pr == PRED_COLD_LABEL))
+	{
+	  gcc_rich_location richloc (EXPR_LOC_OR_LOC (ft, locus));
+	  richloc.add_range (EXPR_LOC_OR_LOC (fe, locus));
+	  warning_at (&richloc, OPT_Wattributes,
+		      "both branches of %<if%> statement marked as %qs",
+		      predictor_name (pr));
+	}
+    }
+
   if (!then_)
     then_ = build_empty_stmt (locus);
   if (!else_)
@@ -2679,10 +2718,16 @@ cp_fold (tree x)
 tree
 lookup_hotness_attribute (tree list)
 {
-  tree attr = lookup_attribute ("hot", list);
-  if (attr)
-    return attr;
-  return lookup_attribute ("cold", list);
+  for (; list; list = TREE_CHAIN (list))
+    {
+      tree name = get_attribute_name (list);
+      if (is_attribute_p ("hot", name)
+	  || is_attribute_p ("cold", name)
+	  || is_attribute_p ("likely", name)
+	  || is_attribute_p ("unlikely", name))
+	break;
+    }
+  return list;
 }
 
 /* Remove both "hot" and "cold" attributes from LIST.  */
@@ -2690,7 +2735,11 @@ lookup_hotness_attribute (tree list)
 static tree
 remove_hotness_attribute (tree list)
 {
-  return remove_attribute ("hot", remove_attribute ("cold", list));
+  list = remove_attribute ("hot", list);
+  list = remove_attribute ("cold", list);
+  list = remove_attribute ("likely", list);
+  list = remove_attribute ("unlikely", list);
+  return list;
 }
 
 /* If [[likely]] or [[unlikely]] appear on this statement, turn it into a
@@ -2704,9 +2753,11 @@ process_stmt_hotness_attribute (tree std_attrs)
   if (tree attr = lookup_hotness_attribute (std_attrs))
     {
       tree name = get_attribute_name (attr);
-      bool hot = is_attribute_p ("hot", name);
+      bool hot = (is_attribute_p ("hot", name)
+		  || is_attribute_p ("likely", name));
       tree pred = build_predict_expr (hot ? PRED_HOT_LABEL : PRED_COLD_LABEL,
 				      hot ? TAKEN : NOT_TAKEN);
+      SET_EXPR_LOCATION (pred, input_location);
       add_stmt (pred);
       if (tree other = lookup_hotness_attribute (TREE_CHAIN (attr)))
 	warning (OPT_Wattributes, "ignoring attribute %qE after earlier %qE",
diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
index 618fb3d8521..360eb72676d 100644
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -25574,10 +25574,6 @@ cp_parser_std_attribute (cp_parser *parser, tree attr_ns)
       else if (is_attribute_p ("optimize_for_synchronized", attr_id))
 	TREE_PURPOSE (attribute)
 	  = get_identifier ("transaction_callable");
-      else if (is_attribute_p ("likely", attr_id))
-	TREE_PURPOSE (attribute) = get_identifier ("hot");
-      else if (is_attribute_p ("unlikely", attr_id))
-	TREE_PURPOSE (attribute) = get_identifier ("cold");
       /* Transactional Memory attributes are GNU attributes.  */
       else if (tm_attr_to_mask (attr_id))
 	TREE_PURPOSE (attribute) = attr_id;
diff --git a/gcc/cp/tree.c b/gcc/cp/tree.c
index 02a9856acbf..38fa94e23d6 100644
--- a/gcc/cp/tree.c
+++ b/gcc/cp/tree.c
@@ -4466,6 +4466,32 @@ handle_no_unique_addr_attribute (tree* node,
   return NULL_TREE;
 }
 
+/* The C++20 [[likely]] and [[unlikely]] attributes on labels map to the GNU
+   hot/cold attributes.  */
+
+static tree
+handle_likeliness_attribute (tree *node, tree name, tree args,
+			     int flags, bool *no_add_attrs)
+{
+  *no_add_attrs = true;
+  if (TREE_CODE (*node) == LABEL_DECL
+      || TREE_CODE (*node) == FUNCTION_DECL)
+    {
+      if (args)
+	warning (OPT_Wattributes, "%qE attribute takes no arguments", name);
+      tree bname = (is_attribute_p ("likely", name)
+		    ? get_identifier ("hot") : get_identifier ("cold"));
+      if (TREE_CODE (*node) == FUNCTION_DECL)
+	warning (OPT_Wattributes, "ISO C++ %qE attribute does not apply to "
+		 "functions; treating as %<[[gnu::%E]]%>", name, bname);
+      tree battr = build_tree_list (bname, NULL_TREE);
+      decl_attributes (node, battr, flags);
+      return NULL_TREE;
+    }
+  else
+    return error_mark_node;
+}
+
 /* Table of valid C++ attributes.  */
 const struct attribute_spec cxx_attribute_table[] =
 {
@@ -4489,6 +4515,10 @@ const struct attribute_spec std_attribute_table[] =
     handle_nodiscard_attribute, NULL },
   { "no_unique_address", 0, 0, true, false, false, false,
     handle_no_unique_addr_attribute, NULL },
+  { "likely", 0, 0, false, false, false, false,
+    handle_likeliness_attribute, attr_cold_hot_exclusions },
+  { "unlikely", 0, 0, false, false, false, false,
+    handle_likeliness_attribute, attr_cold_hot_exclusions },
   { NULL, 0, 0, false, false, false, false, NULL, NULL }
 };
 
diff --git a/gcc/testsuite/g++.dg/cpp2a/attr-likely2.C b/gcc/testsuite/g++.dg/cpp2a/attr-likely2.C
index 3c951828c95..6c59610528e 100644
--- a/gcc/testsuite/g++.dg/cpp2a/attr-likely2.C
+++ b/gcc/testsuite/g++.dg/cpp2a/attr-likely2.C
@@ -6,5 +6,7 @@ int main()
   if (b)
     [[likely, likely]] b;	// { dg-warning "ignoring" }
   else
-    [[likely]] [[unlikely]] b;	// { dg-warning "ignoring" }
+    [[unlikely]] [[likely]] b;	// { dg-warning "ignoring" }
+
+  [[likely, unlikely]] lab:;	// { dg-warning "ignoring" }
 }
diff --git a/gcc/testsuite/g++.dg/cpp2a/attr-likely3.C b/gcc/testsuite/g++.dg/cpp2a/attr-likely3.C
new file mode 100644
index 00000000000..bb1265ddb6e
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp2a/attr-likely3.C
@@ -0,0 +1,8 @@
+// { dg-do compile { target c++2a } }
+
+[[likely]] void f() { }		// { dg-warning "function" }
+
+int main()
+{
+  f();
+}
diff --git a/gcc/testsuite/g++.dg/cpp2a/attr-likely4.C b/gcc/testsuite/g++.dg/cpp2a/attr-likely4.C
new file mode 100644
index 00000000000..bf0dc4c5d4e
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp2a/attr-likely4.C
@@ -0,0 +1,19 @@
+// { dg-do compile { target c++2a } }
+
+int a, b, c;
+
+void
+__attribute__((noinline))
+bar()
+{
+  if (a == 123)
+    [[likely]] c = 5;		// { dg-warning "both" }
+  else
+    [[likely]] b = 77;
+}
+
+int main()
+{
+  bar ();
+  return 0;
+}
diff --git a/gcc/c-family/ChangeLog b/gcc/c-family/ChangeLog
index 74c8e98835b..855e61ae39b 100644
--- a/gcc/c-family/ChangeLog
+++ b/gcc/c-family/ChangeLog
@@ -1,6 +1,7 @@
-2018-11-11  Jason Merrill  <jason@redhat.com>
+2018-11-16  Jason Merrill  <jason@redhat.com>
 
 	* c-lex.c (c_common_has_attribute): Handle likely/unlikely.
+	* c-attribs.c (attr_cold_hot_exclusions): Make public.
 
 2018-11-15  Martin Sebor  <msebor@redhat.com>
 
diff --git a/gcc/cp/ChangeLog b/gcc/cp/ChangeLog
index f1ee72e068a..8c9992718ac 100644
--- a/gcc/cp/ChangeLog
+++ b/gcc/cp/ChangeLog
@@ -1,11 +1,14 @@
-2018-11-12  Jason Merrill  <jason@redhat.com>
+2018-11-16  Jason Merrill  <jason@redhat.com>
 
 	Implement P0479R5, [[likely]] and [[unlikely]].
-	* parser.c (cp_parser_std_attribute): Handle likely/unlikely.
-	(cp_parser_statement): Call process_stmt_hotness_attribute.
-	(cp_parser_label_for_labeled_statement): Apply attributes to case.
+	* tree.c (handle_likeliness_attribute): New.
+	(std_attribute_table): Add likely/unlikely.
 	* cp-gimplify.c (lookup_hotness_attribute, remove_hotness_attribute)
-	(process_stmt_hotness_attribute): New.
+	(process_stmt_hotness_attribute, first_stmt): New.
+	(genericize_if_stmt): Check for duplicate predictions.
+	* parser.c (cp_parser_statement): Call
+	process_stmt_hotness_attribute.
+	(cp_parser_label_for_labeled_statement): Apply attributes to case.
 	* decl.c (finish_case_label): Give label in template type void.
 	* pt.c (tsubst_expr) [CASE_LABEL_EXPR]: Copy attributes.
 	[PREDICT_EXPR]: Handle.

Re: RFC (branch prediction): PATCH to implement P0479R5, [[likely]] and [[unlikely]].

[Jan Hubicka]

> On Thu, Nov 15, 2018 at 7:14 AM Jan Hubicka <hubicka@ucw.cz> wrote:
> >
> > > > A warning seems appropriate.  You think the front end is the right
> > > > place for that?
> > >
> > > Probably yes. Note that middle-end can optimize about dead branches and so that
> > > theoretically one can end up with a branching where e.g. both branches are [[likely]].
> > > I wouldn't bother users with these.
> >
> > Note that what really happens in this case is that if conditional is
> > constant propagated and it has predict_expr, the predict_expr stays and
> > will get assigned to the random control dependence edge which controls
> > execution of the original statement.  This is not very intuitive
> > behaviour. Does C++ say what should happen in this case?
> 
> No, it doesn't say much.
> 
> > One option would be to deal with this gratefully at high level gimple
> > and turn predict_exprs into edge probabilities eariler than we do normal
> > branch prediction (which is intended to be later so profile does not end
> > up unnecesarily inconsistent)
> 
> Sounds reasonable.
> 
> This additional patch implements the suggested diagnostics.

Looks good to me. Next stage1 I will experiment with moving path
prediction up in the pass queue. It should be quite easy to do.

Honza

Re: RFC (branch prediction): PATCH to implement P0479R5, [[likely]] and [[unlikely]].

[Martin Liška]

Hi Jason.

Just small nits I noticed for:

cat test4.C
int a, b, c;

void
__attribute__((noinline))
bar()
{
  if (a == 123)
    [[likely]] c = 5;
  else
    [[likely]] b = 77;
}

int main()
{
  bar ();
  return 0;
}

$ g++ test4.C -c
test4.C: In function ‘void bar()’:
test4.C:8:16: warning: both branches of ‘if’ statement marked as ‘hot label’ [-Wattributes]
    8 |     [[likely]] c = 5;
      |                ^
    9 |   else
   10 |     [[likely]] b = 77;
      |                ~

1) I would expect 'likely' instead of 'hot label'
2) maybe we can put the carousel to the attribute instead of the first statement in the block?

Thanks,
Martin

Re: RFC (branch prediction): PATCH to implement P0479R5, [[likely]] and [[unlikely]].

[Martin Liška]

PING^1

On 11/30/18 11:26 AM, Martin Liška wrote:
> Hi Jason.
> 
> Just small nits I noticed for:
> 
> cat test4.C
> int a, b, c;
> 
> void
> __attribute__((noinline))
> bar()
> {
>   if (a == 123)
>     [[likely]] c = 5;
>   else
>     [[likely]] b = 77;
> }
> 
> int main()
> {
>   bar ();
>   return 0;
> }
> 
> $ g++ test4.C -c
> test4.C: In function ‘void bar()’:
> test4.C:8:16: warning: both branches of ‘if’ statement marked as ‘hot label’ [-Wattributes]
>     8 |     [[likely]] c = 5;
>       |                ^
>     9 |   else
>    10 |     [[likely]] b = 77;
>       |                ~
> 
> 1) I would expect 'likely' instead of 'hot label'
> 2) maybe we can put the carousel to the attribute instead of the first statement in the block?
> 
> Thanks,
> Martin
> 

Re: RFC (branch prediction): PATCH to implement P0479R5, [[likely]] and [[unlikely]].

[Jason Merrill]

[-- Attachment #1: Type: text/plain, Size: 887 bytes --]

On 2/18/19 7:44 AM, Martin Liška wrote:
> PING^1
> 
> On 11/30/18 11:26 AM, Martin Liška wrote:
>> Hi Jason.
>>
>> Just small nits I noticed for:
>>
>> cat test4.C
>> int a, b, c;
>>
>> void
>> __attribute__((noinline))
>> bar()
>> {
>>    if (a == 123)
>>      [[likely]] c = 5;
>>    else
>>      [[likely]] b = 77;
>> }
>>
>> int main()
>> {
>>    bar ();
>>    return 0;
>> }
>>
>> $ g++ test4.C -c
>> test4.C: In function ‘void bar()’:
>> test4.C:8:16: warning: both branches of ‘if’ statement marked as ‘hot label’ [-Wattributes]
>>      8 |     [[likely]] c = 5;
>>        |                ^
>>      9 |   else
>>     10 |     [[likely]] b = 77;
>>        |                ~
>>
>> 1) I would expect 'likely' instead of 'hot label'
>> 2) maybe we can put the carousel to the attribute instead of the first statement in the block?

Fixed thus:



[-- Attachment #2: likely-diag.diff --]
[-- Type: text/x-patch, Size: 6371 bytes --]

commit 4f0e3ea77fd14dc9931cade9add07f1aa70e8ef4
Author: Jason Merrill <jason@redhat.com>
Date:   Mon Feb 18 08:49:49 2019 -1000

            Improve duplicate [[likely]] diagnostic.
    
            * parser.c (cp_parser_statement): Make attrs_loc a range.  Pass it
            to process_stmt_hotness_attribute.
            * cp-gimplify.c (process_stmt_hotness_attribute): Take attrs_loc.
            (genericize_if_stmt): Use likely/unlikely instead of predictor_name.

diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index 60ca1366cf6..ac3654467ac 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -7576,7 +7576,7 @@ extern tree cp_fully_fold			(tree);
 extern tree cp_fully_fold_init			(tree);
 extern void clear_fold_cache			(void);
 extern tree lookup_hotness_attribute		(tree);
-extern tree process_stmt_hotness_attribute	(tree);
+extern tree process_stmt_hotness_attribute	(tree, location_t);
 
 /* in name-lookup.c */
 extern tree strip_using_decl                    (tree);
diff --git a/gcc/cp/cp-gimplify.c b/gcc/cp/cp-gimplify.c
index 33111bd14bf..56f717de85d 100644
--- a/gcc/cp/cp-gimplify.c
+++ b/gcc/cp/cp-gimplify.c
@@ -206,7 +206,7 @@ genericize_if_stmt (tree *stmt_p)
 	  richloc.add_range (EXPR_LOC_OR_LOC (fe, locus));
 	  warning_at (&richloc, OPT_Wattributes,
 		      "both branches of %<if%> statement marked as %qs",
-		      predictor_name (pr));
+		      pr == PRED_HOT_LABEL ? "likely" : "unlikely");
 	}
     }
 
@@ -2765,7 +2765,7 @@ remove_hotness_attribute (tree list)
    PREDICT_EXPR.  */
 
 tree
-process_stmt_hotness_attribute (tree std_attrs)
+process_stmt_hotness_attribute (tree std_attrs, location_t attrs_loc)
 {
   if (std_attrs == error_mark_node)
     return std_attrs;
@@ -2776,7 +2776,7 @@ process_stmt_hotness_attribute (tree std_attrs)
 		  || is_attribute_p ("likely", name));
       tree pred = build_predict_expr (hot ? PRED_HOT_LABEL : PRED_COLD_LABEL,
 				      hot ? TAKEN : NOT_TAKEN);
-      SET_EXPR_LOCATION (pred, input_location);
+      SET_EXPR_LOCATION (pred, attrs_loc);
       add_stmt (pred);
       if (tree other = lookup_hotness_attribute (TREE_CHAIN (attr)))
 	warning (OPT_Wattributes, "ignoring attribute %qE after earlier %qE",
diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
index ffecce4e29b..adb5f6f27a1 100644
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -11060,7 +11060,7 @@ cp_parser_statement (cp_parser* parser, tree in_statement_expr,
 {
   tree statement, std_attrs = NULL_TREE;
   cp_token *token;
-  location_t statement_location, attrs_location;
+  location_t statement_location, attrs_loc;
 
  restart:
   if (if_p != NULL)
@@ -11069,13 +11069,19 @@ cp_parser_statement (cp_parser* parser, tree in_statement_expr,
   statement = NULL_TREE;
 
   saved_token_sentinel saved_tokens (parser->lexer);
-  attrs_location = cp_lexer_peek_token (parser->lexer)->location;
+  attrs_loc = cp_lexer_peek_token (parser->lexer)->location;
   if (c_dialect_objc ())
     /* In obj-c++, seeing '[[' might be the either the beginning of
        c++11 attributes, or a nested objc-message-expression.  So
        let's parse the c++11 attributes tentatively.  */
     cp_parser_parse_tentatively (parser);
   std_attrs = cp_parser_std_attribute_spec_seq (parser);
+  if (std_attrs)
+    {
+      location_t end_loc
+	= cp_lexer_previous_token (parser->lexer)->location;
+      attrs_loc = make_location (attrs_loc, attrs_loc, end_loc);
+    }
   if (c_dialect_objc ())
     {
       if (!cp_parser_parse_definitely (parser))
@@ -11107,14 +11113,14 @@ cp_parser_statement (cp_parser* parser, tree in_statement_expr,
 
 	case RID_IF:
 	case RID_SWITCH:
-	  std_attrs = process_stmt_hotness_attribute (std_attrs);
+	  std_attrs = process_stmt_hotness_attribute (std_attrs, attrs_loc);
 	  statement = cp_parser_selection_statement (parser, if_p, chain);
 	  break;
 
 	case RID_WHILE:
 	case RID_DO:
 	case RID_FOR:
-	  std_attrs = process_stmt_hotness_attribute (std_attrs);
+	  std_attrs = process_stmt_hotness_attribute (std_attrs, attrs_loc);
 	  statement = cp_parser_iteration_statement (parser, if_p, false, 0);
 	  break;
 
@@ -11122,7 +11128,7 @@ cp_parser_statement (cp_parser* parser, tree in_statement_expr,
 	case RID_CONTINUE:
 	case RID_RETURN:
 	case RID_GOTO:
-	  std_attrs = process_stmt_hotness_attribute (std_attrs);
+	  std_attrs = process_stmt_hotness_attribute (std_attrs, attrs_loc);
 	  statement = cp_parser_jump_statement (parser);
 	  break;
 
@@ -11132,12 +11138,12 @@ cp_parser_statement (cp_parser* parser, tree in_statement_expr,
 	case RID_AT_FINALLY:
 	case RID_AT_SYNCHRONIZED:
 	case RID_AT_THROW:
-	  std_attrs = process_stmt_hotness_attribute (std_attrs);
+	  std_attrs = process_stmt_hotness_attribute (std_attrs, attrs_loc);
 	  statement = cp_parser_objc_statement (parser);
 	  break;
 
 	case RID_TRY:
-	  std_attrs = process_stmt_hotness_attribute (std_attrs);
+	  std_attrs = process_stmt_hotness_attribute (std_attrs, attrs_loc);
 	  statement = cp_parser_try_block (parser);
 	  break;
 
@@ -11158,11 +11164,11 @@ cp_parser_statement (cp_parser* parser, tree in_statement_expr,
 	case RID_SYNCHRONIZED:
 	case RID_ATOMIC_NOEXCEPT:
 	case RID_ATOMIC_CANCEL:
-	  std_attrs = process_stmt_hotness_attribute (std_attrs);
+	  std_attrs = process_stmt_hotness_attribute (std_attrs, attrs_loc);
 	  statement = cp_parser_transaction (parser, token);
 	  break;
 	case RID_TRANSACTION_CANCEL:
-	  std_attrs = process_stmt_hotness_attribute (std_attrs);
+	  std_attrs = process_stmt_hotness_attribute (std_attrs, attrs_loc);
 	  statement = cp_parser_transaction_cancel (parser);
 	  break;
 
@@ -11239,7 +11245,7 @@ cp_parser_statement (cp_parser* parser, tree in_statement_expr,
       if (loc_after_labels != NULL)
 	*loc_after_labels = statement_location;
 
-      std_attrs = process_stmt_hotness_attribute (std_attrs);
+      std_attrs = process_stmt_hotness_attribute (std_attrs, attrs_loc);
 
       /* Look for an expression-statement instead.  */
       statement = cp_parser_expression_statement (parser, in_statement_expr);
@@ -11269,7 +11275,7 @@ cp_parser_statement (cp_parser* parser, tree in_statement_expr,
 
   /* Allow "[[fallthrough]];", but warn otherwise.  */
   if (std_attrs != NULL_TREE)
-    warning_at (attrs_location,
+    warning_at (attrs_loc,
 		OPT_Wattributes,
 		"attributes at the beginning of statement are ignored");
 }

Re: RFC (branch prediction): PATCH to implement P0479R5, [[likely]] and [[unlikely]].

[Martin Liška]

On 2/18/19 8:34 PM, Jason Merrill wrote:
> Fixed thus:

Works for me, thanks!

Martin