Re: [RFC] c++: extend cold, hot attributes to classes
On 8/3/23 07:07, Javier Martinez via Gcc-patches wrote: Most code is cold. This patch extends support for attribute ((cold)) to C++ Classes, Unions, and Structs (RECORD_TYPES and UNION_TYPES) to benefit from encapsulation - reducing the verbosity of using the attribute where deserved. The ((hot)) attribute is also extended for its semantic relation. What is the sentiment on this use-case? Seems reasonable, but how do you expect this to be used? for gcc/c-family/ChangeLog * c-attribs.c (attribute_spec): Remove decl_required field for "hot" and "cold" attributes. (handle_cold_attribute): remove warning on RECORD_TYPE and UNION_TYPE when c_dialect_cxx. (handle_cold_attribute): remove warning on RECORD_TYPE and UNION_TYPE when c_dialect_cxx. for gcc/cp/ChangeLog * class.c (finish_struct) propagate hot and cold attributes to all FUNCTION_DECL when the class itself is marked hot or cold. for gcc/testsuite/ChangeLog * g++.dg/ext/attr-hotness.C: New. Signed-off-by: Javier Martinez diff --git a/gcc/c-family/c-attribs.c b/gcc/c-family/c-attribs.c index dc9579c..815df66 100644 --- a/gcc/c-family/c-attribs.c +++ b/gcc/c-family/c-attribs.c @@ -398,10 +398,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", @@ -837,22 +837,23 @@ handle_noreturn_attribute (tree *node, tree name, tree ARG_UNUSED (args), static tree handle_hot_attribute (tree *node, tree name, tree ARG_UNUSED (args), - int ARG_UNUSED (flags), bool *no_add_attrs) + int ARG_UNUSED (flags), bool *no_add_attrs) Try to avoid unnecessary whitespace changes. { - if (TREE_CODE (*node) == FUNCTION_DECL - || TREE_CODE (*node) == LABEL_DECL) + if ( (TREE_CODE (*node) == FUNCTION_DECL || +TREE_CODE (*node) == LABEL_DECL) + || ((TREE_CODE(*node) == RECORD_TYPE || + TREE_CODE(*node) == UNION_TYPE) && c_dialect_cxx())) In GCC we generally put a space before '(', not after it. https://www.gnu.org/prep/standards/html_node/Formatting.html#Formatting { /* Attribute hot processing is done later with lookup_attribute. */ } else { warning (OPT_Wattributes, "%qE attribute ignored", name); - *no_add_attrs = true; + *no_add_attrs = true; More unnecessary reformatting. } return NULL_TREE; } - We want to keep this newline between this function and the comment for the next function. /* Handle a "cold" and attribute; arguments as in struct attribute_spec.handler. */ @@ -860,15 +861,17 @@ static tree handle_cold_attribute (tree *node, tree name, tree ARG_UNUSED (args), int ARG_UNUSED (flags), bool *no_add_attrs) { - if (TREE_CODE (*node) == FUNCTION_DECL - || TREE_CODE (*node) == LABEL_DECL) + if ( (TREE_CODE (*node) == FUNCTION_DECL || +TREE_CODE (*node) == LABEL_DECL) + || ((TREE_CODE(*node) == RECORD_TYPE || + TREE_CODE(*node) == UNION_TYPE) && c_dialect_cxx())) As above. { /* Attribute cold processing is done later with lookup_attribute. */ } else { warning (OPT_Wattributes, "%qE attribute ignored", name); - *no_add_attrs = true; + *no_add_attrs = true; As above. } return NULL_TREE; diff --git a/gcc/cp/class.c b/gcc/cp/class.c index 07abe52..70f734f 100644 --- a/gcc/cp/class.c +++ b/gcc/cp/class.c @@ -7540,6 +7540,35 @@ finish_struct (tree t, tree attributes) && !LAMBDA_TYPE_P (t)) add_stmt (build_min (TAG_DEFN, t)); + Unnecessary extra blank line. + /* classes marked with hotness attributes propagate the attribute to Capitalize the beginning of the comment. + all methods. We propagate these here as there is a guarantee that + TYPE_FIELDS is populated, as opposed from within decl_attributes. */ + + tree has_cold_attr = lookup_attribute("cold", TYPE_ATTRIBUTES(t)); + tree has_hot_attr = lookup_attribute("hot", TYPE_ATTRIBUTES(t)); + + if ( has_cold_attr || has_hot_attr ) { + +/* hoisted out of the loop */ +tree attr_cold_id = get_identifier("cold"); +tree attr_hot_id = get_identifier("hot"); More space before parenthesis. + +for (tree f = TYPE_FIELDS (t); f; f = DECL_CHAIN (f)) + { +if (TREE_CODE (f) == FUNCTION_DECL) { + /* decl_attributes will take care of conflicts, + also prioritizing at
[RFC] c++: extend cold, hot attributes to classes
Most code is cold. This patch extends support for attribute ((cold)) to C++ Classes, Unions, and Structs (RECORD_TYPES and UNION_TYPES) to benefit from encapsulation - reducing the verbosity of using the attribute where deserved. The ((hot)) attribute is also extended for its semantic relation. What is the sentiment on this use-case? for gcc/c-family/ChangeLog * c-attribs.c (attribute_spec): Remove decl_required field for "hot" and "cold" attributes. (handle_cold_attribute): remove warning on RECORD_TYPE and UNION_TYPE when c_dialect_cxx. (handle_cold_attribute): remove warning on RECORD_TYPE and UNION_TYPE when c_dialect_cxx. for gcc/cp/ChangeLog * class.c (finish_struct) propagate hot and cold attributes to all FUNCTION_DECL when the class itself is marked hot or cold. for gcc/testsuite/ChangeLog * g++.dg/ext/attr-hotness.C: New. Signed-off-by: Javier Martinez diff --git a/gcc/c-family/c-attribs.c b/gcc/c-family/c-attribs.c index dc9579c..815df66 100644 --- a/gcc/c-family/c-attribs.c +++ b/gcc/c-family/c-attribs.c @@ -398,10 +398,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", @@ -837,22 +837,23 @@ handle_noreturn_attribute (tree *node, tree name, tree ARG_UNUSED (args), static tree handle_hot_attribute (tree *node, tree name, tree ARG_UNUSED (args), - int ARG_UNUSED (flags), bool *no_add_attrs) + int ARG_UNUSED (flags), bool *no_add_attrs) { - if (TREE_CODE (*node) == FUNCTION_DECL - || TREE_CODE (*node) == LABEL_DECL) + if ( (TREE_CODE (*node) == FUNCTION_DECL || +TREE_CODE (*node) == LABEL_DECL) + || ((TREE_CODE(*node) == RECORD_TYPE || + TREE_CODE(*node) == UNION_TYPE) && c_dialect_cxx())) { /* Attribute hot processing is done later with lookup_attribute. */ } else { warning (OPT_Wattributes, "%qE attribute ignored", name); - *no_add_attrs = true; + *no_add_attrs = true; } return NULL_TREE; } - /* Handle a "cold" and attribute; arguments as in struct attribute_spec.handler. */ @@ -860,15 +861,17 @@ static tree handle_cold_attribute (tree *node, tree name, tree ARG_UNUSED (args), int ARG_UNUSED (flags), bool *no_add_attrs) { - if (TREE_CODE (*node) == FUNCTION_DECL - || TREE_CODE (*node) == LABEL_DECL) + if ( (TREE_CODE (*node) == FUNCTION_DECL || +TREE_CODE (*node) == LABEL_DECL) + || ((TREE_CODE(*node) == RECORD_TYPE || + TREE_CODE(*node) == UNION_TYPE) && c_dialect_cxx())) { /* Attribute cold processing is done later with lookup_attribute. */ } else { warning (OPT_Wattributes, "%qE attribute ignored", name); - *no_add_attrs = true; + *no_add_attrs = true; } return NULL_TREE; diff --git a/gcc/cp/class.c b/gcc/cp/class.c index 07abe52..70f734f 100644 --- a/gcc/cp/class.c +++ b/gcc/cp/class.c @@ -7540,6 +7540,35 @@ finish_struct (tree t, tree attributes) && !LAMBDA_TYPE_P (t)) add_stmt (build_min (TAG_DEFN, t)); + + /* classes marked with hotness attributes propagate the attribute to + all methods. We propagate these here as there is a guarantee that + TYPE_FIELDS is populated, as opposed from within decl_attributes. */ + + tree has_cold_attr = lookup_attribute("cold", TYPE_ATTRIBUTES(t)); + tree has_hot_attr = lookup_attribute("hot", TYPE_ATTRIBUTES(t)); + + if ( has_cold_attr || has_hot_attr ) { + +/* hoisted out of the loop */ +tree attr_cold_id = get_identifier("cold"); +tree attr_hot_id = get_identifier("hot"); + +for (tree f = TYPE_FIELDS (t); f; f = DECL_CHAIN (f)) + { +if (TREE_CODE (f) == FUNCTION_DECL) { + /* decl_attributes will take care of conflicts, + also prioritizing attributes explicitly marked in methods */ + + if (has_cold_attr) { +decl_attributes (&f, tree_cons (attr_cold_id, NULL, NULL), 0); + } else if (has_hot_attr) { +decl_attributes (&f, tree_cons (attr_hot_id, NULL, NULL), 0); + } +} + } + } + return t; } 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 000..075c624 --- /dev/null +++ b/gcc/testsuite/g++.dg/ext/attr-hotness.C @@ -0,0 +1,15 @@ +/* { dg-do compile } */ +/* { dg-options "-O0 -Wattributes -fdump-tree-gimple" }