On Thu, Nov 15, 2018 at 7:14 AM Jan Hubicka <hubi...@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.
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 <ja...@redhat.com>
+2018-11-16 Jason Merrill <ja...@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 <mse...@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 <ja...@redhat.com>
+2018-11-16 Jason Merrill <ja...@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.
