[PATCH v4] c++: extend cold, hot attributes to classes

[PATCH v4] c++: extend cold, hot attributes to classes

[Javier Martinez]

[-- Attachment #1.1: Type: text/plain, Size: 370 bytes --]

On Mon, Aug 14, 2023 at 8:32 PM Jason Merrill <jason@redhat.com> wrote:
> I think you also want to check for ATTR_FLAG_TYPE_IN_PLACE.
> [...]
> > +  propagate_class_warmth_attribute (t);
> Maybe call this in check_bases_and_members instead?

Yes, that is sensible. Done.

Thanks,
Javier

Signed-off-by: Javier Martinez <javier.martinez.bugzilla@gmail.com>

[-- Attachment #2: 0001-extend-cold-hot-attributes-to-classes.patch --]
[-- Type: text/x-patch, Size: 9862 bytes --]

Signed-off-by: Javier Martinez <javier.martinez.bugzilla@gmail.com>

gcc/c-family/ChangeLog:

        * c-attribs.cc (handle_hot_attribute): remove warning on RECORD_TYPE
        and UNION_TYPE when in c_dialect_xx.
        (handle_cold_attribute): Likewise.

gcc/cp/ChangeLog:

        * class.cc (propagate_class_warmth_attribute): New function.
        (check_bases_and_members): propagate hot and cold attributes
        to all FUNCTION_DECL when the record is marked hot or cold.
        * cp-tree.h (maybe_propagate_warmth_attributes): New function.
        * decl2.cc (maybe_propagate_warmth_attributes): New function.
        * method.cc (lazily_declare_fn): propagate hot and cold
        attributes to lazily declared functions when the record is
        marked hot or cold.

gcc/testsuite/ChangeLog:

        * g++.dg/ext/attr-hotness.C: New test.
---
 gcc/c-family/c-attribs.cc               | 50 ++++++++++++++++++++++++-
 gcc/cp/class.cc                         | 31 +++++++++++++++
 gcc/cp/cp-tree.h                        |  1 +
 gcc/cp/decl2.cc                         | 37 ++++++++++++++++++
 gcc/cp/method.cc                        |  6 +++
 gcc/testsuite/g++.dg/ext/attr-hotness.C | 16 ++++++++
 6 files changed, 139 insertions(+), 2 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/ext/attr-hotness.C

diff --git a/gcc/c-family/c-attribs.cc b/gcc/c-family/c-attribs.cc
index e2792ca6898..25083d597c0 100644
--- a/gcc/c-family/c-attribs.cc
+++ b/gcc/c-family/c-attribs.cc
@@ -452,10 +452,10 @@ const struct attribute_spec c_common_attribute_table[] =
   { "alloc_size",	      1, 2, false, true, true, false,
 			      handle_alloc_size_attribute,
 	                      attr_alloc_exclusions },
-  { "cold",                   0, 0, true,  false, false, false,
+  { "cold",		      0, 0, false,  false, false, false,
 			      handle_cold_attribute,
 	                      attr_cold_hot_exclusions },
-  { "hot",                    0, 0, true,  false, false, false,
+  { "hot",		      0, 0, false,  false, false, false,
 			      handle_hot_attribute,
 	                      attr_cold_hot_exclusions },
   { "no_address_safety_analysis",
@@ -1110,6 +1110,29 @@ handle_hot_attribute (tree *node, tree name, tree ARG_UNUSED (args),
     {
       /* Attribute hot processing is done later with lookup_attribute.  */
     }
+  else if ((TREE_CODE (*node) == RECORD_TYPE
+	    || TREE_CODE (*node) == UNION_TYPE)
+	   && c_dialect_cxx ()
+	   && (flags & (int) ATTR_FLAG_TYPE_IN_PLACE))
+    {
+      /* Check conflict here as decl_attributes will otherwise only catch
+	 it late at the function when the attribute is used on a class.  */
+      tree cold_attr = lookup_attribute ("cold", TYPE_ATTRIBUTES (*node));
+      if (cold_attr)
+	{
+	  warning (OPT_Wattributes, "ignoring attribute %qE because it "
+			"conflicts with attribute %qs", name, "cold");
+	  *no_add_attrs = true;
+	}
+    }
+  else if (flags & ((int) ATTR_FLAG_FUNCTION_NEXT
+		    | (int) ATTR_FLAG_DECL_NEXT))
+    {
+	/* Avoid applying the attribute to a function return type when
+	   used as:  void __attribute ((hot)) foo (void).  It will be
+	   passed to the function.  */
+	*no_add_attrs = true;
+    }
   else
     {
       warning (OPT_Wattributes, "%qE attribute ignored", name);
@@ -1131,6 +1154,29 @@ handle_cold_attribute (tree *node, tree name, tree ARG_UNUSED (args),
     {
       /* Attribute cold processing is done later with lookup_attribute.  */
     }
+  else if ((TREE_CODE (*node) == RECORD_TYPE
+	    || TREE_CODE (*node) == UNION_TYPE)
+	   && c_dialect_cxx ()
+	   && (flags & (int) ATTR_FLAG_TYPE_IN_PLACE))
+    {
+      /* Check conflict here as decl_attributes will otherwise only catch
+	 it late at the function when the attribute is used on a class.  */
+      tree hot_attr = lookup_attribute ("hot", TYPE_ATTRIBUTES (*node));
+      if (hot_attr)
+	{
+	  warning (OPT_Wattributes, "ignoring attribute %qE because it "
+			"conflicts with attribute %qs", name, "hot");
+	  *no_add_attrs = true;
+	}
+    }
+  else if (flags & ((int) ATTR_FLAG_FUNCTION_NEXT
+		    | (int) ATTR_FLAG_DECL_NEXT))
+    {
+	/* Avoid applying the attribute to a function return type when
+	   used as:  void __attribute ((cold)) foo (void).  It will be
+	   passed to the function.  */
+	*no_add_attrs = true;
+    }
   else
     {
       warning (OPT_Wattributes, "%qE attribute ignored", name);
diff --git a/gcc/cp/class.cc b/gcc/cp/class.cc
index 778759237dc..bf0b558967f 100644
--- a/gcc/cp/class.cc
+++ b/gcc/cp/class.cc
@@ -205,6 +205,7 @@ static tree get_vcall_index (tree, tree);
 static bool type_maybe_constexpr_default_constructor (tree);
 static bool type_maybe_constexpr_destructor (tree);
 static bool field_poverlapping_p (tree);
+static void propagate_class_warmth_attribute (tree);
 
 /* Set CURRENT_ACCESS_SPECIFIER based on the protection of DECL.  */
 
@@ -6253,6 +6254,12 @@ check_bases_and_members (tree t)
      allocating an array of this type.  */
   LANG_TYPE_CLASS_CHECK (t)->vec_new_uses_cookie
     = type_requires_array_cookie (t);
+
+  /* Classes marked hot or cold propagate the attribute to all members.  We
+     may do this now that methods are declared.  This does miss some lazily
+     declared special member functions (CLASSTYPE_LAZY_*), which are handled
+     in lazily_declare_fn later on.  */
+  propagate_class_warmth_attribute (t);
 }
 
 /* If T needs a pointer to its virtual function table, set TYPE_VFIELD
@@ -7733,6 +7740,30 @@ unreverse_member_declarations (tree t)
     }
 }
 
+/* Classes, structs or unions T marked with hotness attributes propagate
+   the attribute to all methods.  */
+
+void
+propagate_class_warmth_attribute (tree t)
+{
+
+  if (t == NULL_TREE
+      || !(TREE_CODE (t) == RECORD_TYPE
+	   || TREE_CODE (t) == UNION_TYPE))
+    return;
+
+  tree class_has_cold_attr = lookup_attribute ("cold",
+   				TYPE_ATTRIBUTES (t));
+  tree class_has_hot_attr = lookup_attribute ("hot",
+   				TYPE_ATTRIBUTES (t));
+
+  if (class_has_cold_attr || class_has_hot_attr)
+    for (tree f = TYPE_FIELDS (t); f; f = DECL_CHAIN (f))
+      if (TREE_CODE (f) == FUNCTION_DECL)
+	maybe_propagate_warmth_attributes (f, t);
+
+}
+
 tree
 finish_struct (tree t, tree attributes)
 {
diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index d051ee85f70..4931a45607e 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -7033,6 +7033,7 @@ extern int parm_index                           (tree);
 extern tree vtv_start_verification_constructor_init_function (void);
 extern tree vtv_finish_verification_constructor_init_function (tree);
 extern void cp_check_const_attributes (tree);
+extern void maybe_propagate_warmth_attributes (tree, tree);
 
 /* in error.cc */
 extern const char *type_as_string		(tree, int);
diff --git a/gcc/cp/decl2.cc b/gcc/cp/decl2.cc
index b402befba6d..b7c43e10a9d 100644
--- a/gcc/cp/decl2.cc
+++ b/gcc/cp/decl2.cc
@@ -1604,6 +1604,43 @@ cp_check_const_attributes (tree attributes)
     }
 }
 
+/* Copies hot or cold attributes to a function FN if present on the
+   encapsulating class, struct, or union TYPE.  */
+
+void
+maybe_propagate_warmth_attributes (tree fn, tree type)
+{
+  if (fn == NULL_TREE || type == NULL_TREE
+      || !(TREE_CODE (type) == RECORD_TYPE
+	   || TREE_CODE (type) == UNION_TYPE))
+    return;
+
+  tree has_cold_attr = lookup_attribute ("cold", TYPE_ATTRIBUTES (type));
+  tree has_hot_attr = lookup_attribute ("hot", TYPE_ATTRIBUTES (type));
+
+  if (has_cold_attr || has_hot_attr)
+    {
+
+      /* Transparently ignore the new warmth attribute if it
+	 conflicts with a present function attribute.  Otherwise
+	 decl_attribute would still honour the present attribute,
+	 but producing an undesired warning in the process.  */
+
+      if (has_cold_attr)
+	{
+	  if (lookup_attribute ("hot", DECL_ATTRIBUTES (fn)) == NULL)
+	    decl_attributes (&fn,
+		tree_cons (get_identifier ("cold"), NULL, NULL), 0);
+	}
+      else if (has_hot_attr)
+	{
+	  if (lookup_attribute ("cold", DECL_ATTRIBUTES (fn)) == NULL)
+	    decl_attributes (&fn,
+		tree_cons (get_identifier ("hot"), NULL, NULL), 0);
+	}
+    }
+}
+
 /* Return the last pushed declaration for the symbol DECL or NULL
    when no such declaration exists.  */
 
diff --git a/gcc/cp/method.cc b/gcc/cp/method.cc
index 4ba00175048..a70dd5d6adc 100644
--- a/gcc/cp/method.cc
+++ b/gcc/cp/method.cc
@@ -3592,6 +3592,12 @@ lazily_declare_fn (special_function_kind sfk, tree type)
     /* Create appropriate clones.  */
     clone_cdtor (fn, /*update_methods=*/true);
 
+  /* Classes, structs or unions TYPE marked with hotness attributes propagate
+     the attribute to all methods.  This is typically done in
+     check_bases_and_members, but we must also inject them here for deferred
+     lazily-declared functions.  */
+  maybe_propagate_warmth_attributes (fn, type);
+
   return fn;
 }
 
diff --git a/gcc/testsuite/g++.dg/ext/attr-hotness.C b/gcc/testsuite/g++.dg/ext/attr-hotness.C
new file mode 100644
index 00000000000..f9a6930304d
--- /dev/null
+++ b/gcc/testsuite/g++.dg/ext/attr-hotness.C
@@ -0,0 +1,16 @@
+/* { dg-do compile } */
+/* { dg-options "-O0 -Wattributes -fdump-tree-gimple" } */
+
+
+struct __attribute((cold)) A { __attribute((noinline, used)) void foo(void) { } };
+
+struct __attribute((hot)) B { __attribute((noinline, used)) void foo(void) { } };
+
+struct __attribute((hot, cold)) C { __attribute((noinline, used)) void foo(void) { } }; /* { dg-warning "ignoring attribute .cold. because it conflicts with attribute .hot." } */
+
+struct __attribute((cold, hot)) D { __attribute((noinline, used)) void foo(void) { } }; /* { dg-warning "ignoring attribute .hot. because it conflicts with attribute .cold." } */
+
+
+/* { dg-final { scan-tree-dump-times "cold" 2 "gimple" } } */
+/* { dg-final { scan-tree-dump-times "hot" 2 "gimple" } } */
+
-- 
2.34.1

Re: [PATCH v4] c++: extend cold, hot attributes to classes

[Jason Merrill]

On 8/15/23 09:41, Javier Martinez wrote:
> On Mon, Aug 14, 2023 at 8:32 PM Jason Merrill <jason@redhat.com 
> <mailto:jason@redhat.com>> wrote:
>  > I think you also want to check for ATTR_FLAG_TYPE_IN_PLACE.
>  > [...]
>  > > +  propagate_class_warmth_attribute (t);
>  > Maybe call this in check_bases_and_members instead?
> 
> Yes, that is sensible. Done.

You still need an update to doc/extend.texi for this additional use of 
the attribute.  Sorry I didn't think of that before.

> +	  warning (OPT_Wattributes, "ignoring attribute %qE because it "
> +			"conflicts with attribute %qs", name, "cold");
...
> +	  warning (OPT_Wattributes, "ignoring attribute %qE because it "
> +			"conflicts with attribute %qs", name, "hot");

Function arguments continuing on the next line should line up with the '('.

> +  tree class_has_cold_attr = lookup_attribute ("cold",
> +   				TYPE_ATTRIBUTES (t));
> +  tree class_has_hot_attr = lookup_attribute ("hot",
> +   				TYPE_ATTRIBUTES (t));

...so I'd suggest reformatting these lines as:

    tree class_has_cold_attr
      = lookup_attribute ("cold", TYPE_ATTRIBUTES (t));
    tree class_has_hot_attr
      = lookup_attribute ("hot", TYPE_ATTRIBUTES (t));

> +	    decl_attributes (&fn,
> +		tree_cons (get_identifier ("cold"), NULL, NULL), 0);

...and maybe use a local variable for the result of tree_cons.

> +  if (has_cold_attr || has_hot_attr)
> +    {
> +
> +      /* Transparently ignore the new warmth attribute if it

Unnecessary blank line.

Jason