* RFC (branch prediction): PATCH to implement P0479R5, [[likely]] and [[unlikely]]. @ 2018-11-13 4:43 Jason Merrill 2018-11-13 14:20 ` Martin Liška 0 siblings, 1 reply; 11+ messages in thread From: Jason Merrill @ 2018-11-13 4:43 UTC (permalink / raw) To: gcc-patches [[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 ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: RFC (branch prediction): PATCH to implement P0479R5, [[likely]] and [[unlikely]]. 2018-11-13 4:43 RFC (branch prediction): PATCH to implement P0479R5, [[likely]] and [[unlikely]] Jason Merrill @ 2018-11-13 14:20 ` Martin Liška 2018-11-13 19:43 ` Jason Merrill 0 siblings, 1 reply; 11+ messages in thread From: Martin Liška @ 2018-11-13 14:20 UTC (permalink / raw) To: Jason Merrill, gcc-patches; +Cc: Jan Hubicka [-- 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; } ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: RFC (branch prediction): PATCH to implement P0479R5, [[likely]] and [[unlikely]]. 2018-11-13 14:20 ` Martin Liška @ 2018-11-13 19:43 ` Jason Merrill 2018-11-15 10:51 ` Martin Liška 0 siblings, 1 reply; 11+ messages in thread From: Jason Merrill @ 2018-11-13 19:43 UTC (permalink / raw) To: Martin Liška; +Cc: gcc-patches List, Jan Hubicka 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 ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: RFC (branch prediction): PATCH to implement P0479R5, [[likely]] and [[unlikely]]. 2018-11-13 19:43 ` Jason Merrill @ 2018-11-15 10:51 ` Martin Liška 2018-11-15 12:14 ` Jan Hubicka 0 siblings, 1 reply; 11+ messages in thread From: Martin Liška @ 2018-11-15 10:51 UTC (permalink / raw) To: Jason Merrill; +Cc: gcc-patches List, Jan Hubicka 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 > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: RFC (branch prediction): PATCH to implement P0479R5, [[likely]] and [[unlikely]]. 2018-11-15 10:51 ` Martin Liška @ 2018-11-15 12:14 ` Jan Hubicka 2018-11-16 18:36 ` Jason Merrill 0 siblings, 1 reply; 11+ messages in thread From: Jan Hubicka @ 2018-11-15 12:14 UTC (permalink / raw) To: Martin Liška; +Cc: Jason Merrill, gcc-patches List > > 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 > > > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: RFC (branch prediction): PATCH to implement P0479R5, [[likely]] and [[unlikely]]. 2018-11-15 12:14 ` Jan Hubicka @ 2018-11-16 18:36 ` Jason Merrill 2018-11-16 21:23 ` Jan Hubicka 2018-11-30 10:26 ` Martin Liška 0 siblings, 2 replies; 11+ messages in thread From: Jason Merrill @ 2018-11-16 18:36 UTC (permalink / raw) To: Jan Hubicka; +Cc: Martin Liška, gcc-patches List [-- 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. ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: RFC (branch prediction): PATCH to implement P0479R5, [[likely]] and [[unlikely]]. 2018-11-16 18:36 ` Jason Merrill @ 2018-11-16 21:23 ` Jan Hubicka 2018-11-30 10:26 ` Martin Liška 1 sibling, 0 replies; 11+ messages in thread From: Jan Hubicka @ 2018-11-16 21:23 UTC (permalink / raw) To: Jason Merrill; +Cc: Martin Liška, gcc-patches List > 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 ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: RFC (branch prediction): PATCH to implement P0479R5, [[likely]] and [[unlikely]]. 2018-11-16 18:36 ` Jason Merrill 2018-11-16 21:23 ` Jan Hubicka @ 2018-11-30 10:26 ` Martin Liška 2019-02-18 12:44 ` Martin Liška 1 sibling, 1 reply; 11+ messages in thread From: Martin Liška @ 2018-11-30 10:26 UTC (permalink / raw) To: Jason Merrill, Jan Hubicka; +Cc: gcc-patches List 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 ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: RFC (branch prediction): PATCH to implement P0479R5, [[likely]] and [[unlikely]]. 2018-11-30 10:26 ` Martin Liška @ 2019-02-18 12:44 ` Martin Liška 2019-02-18 19:34 ` Jason Merrill 0 siblings, 1 reply; 11+ messages in thread From: Martin Liška @ 2019-02-18 12:44 UTC (permalink / raw) To: Jason Merrill, Jan Hubicka; +Cc: gcc-patches List 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 > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: RFC (branch prediction): PATCH to implement P0479R5, [[likely]] and [[unlikely]]. 2019-02-18 12:44 ` Martin Liška @ 2019-02-18 19:34 ` Jason Merrill 2019-02-19 9:53 ` Martin Liška 0 siblings, 1 reply; 11+ messages in thread From: Jason Merrill @ 2019-02-18 19:34 UTC (permalink / raw) To: Martin Liška, Jan Hubicka; +Cc: gcc-patches List [-- 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"); } ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: RFC (branch prediction): PATCH to implement P0479R5, [[likely]] and [[unlikely]]. 2019-02-18 19:34 ` Jason Merrill @ 2019-02-19 9:53 ` Martin Liška 0 siblings, 0 replies; 11+ messages in thread From: Martin Liška @ 2019-02-19 9:53 UTC (permalink / raw) To: Jason Merrill, Jan Hubicka; +Cc: gcc-patches List On 2/18/19 8:34 PM, Jason Merrill wrote: > Fixed thus: Works for me, thanks! Martin ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2019-02-19 9:53 UTC | newest] Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-11-13 4:43 RFC (branch prediction): PATCH to implement P0479R5, [[likely]] and [[unlikely]] Jason Merrill 2018-11-13 14:20 ` Martin Liška 2018-11-13 19:43 ` Jason Merrill 2018-11-15 10:51 ` Martin Liška 2018-11-15 12:14 ` Jan Hubicka 2018-11-16 18:36 ` Jason Merrill 2018-11-16 21:23 ` Jan Hubicka 2018-11-30 10:26 ` Martin Liška 2019-02-18 12:44 ` Martin Liška 2019-02-18 19:34 ` Jason Merrill 2019-02-19 9:53 ` Martin Liška
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).