From patchwork Thu Aug 10 18:06:36 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Javier Martinez X-Patchwork-Id: 134134 Return-Path: Delivered-To: ouuuleilei@gmail.com Received: by 2002:a59:b824:0:b0:3f2:4152:657d with SMTP id z4csp594347vqi; Thu, 10 Aug 2023 11:07:35 -0700 (PDT) X-Google-Smtp-Source: AGHT+IFXzMDd7QPb7yn5+Ta3h8z8NCIfKx4NsBeURwEMgaNuBS9xppbHlcYQ2PrmwJ0HXwI1zZZw X-Received: by 2002:a05:6402:298e:b0:523:463d:1ed3 with SMTP id eq14-20020a056402298e00b00523463d1ed3mr3177232edb.15.1691690855563; Thu, 10 Aug 2023 11:07:35 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1691690855; cv=none; d=google.com; s=arc-20160816; b=R1+TTVwMbl3+PuNgsU6BPjV59UqtpD6mbYTVJ9RauPN0OH9pbkht1zyPjJFKYTQRu2 X54UREOv5mvKs/UoMKQO785kr8FzWErVRZRt/pTTqIKLyHwwwwvfGECkRdCAbbJ++HeK oXqHNROrzYS6/403etoXB70m9AiV9qOXhPn+fDxirbHDcR+Sxf0NfBHbT5k716GtWgXp SzHNJTl0CW0T3XHDkVYitqytyblQeZxK0zHCyz+ebtunXHrSl9Ur2ijN/xtp7ID0Uj96 g0EK0jESMBc3RS1VbJ4iH60IsSrtiW7/CQDCij87Diue6AWJI5gE+XJ1O2BieNU89yGq ORkQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=sender:errors-to:reply-to:from:list-subscribe:list-help:list-post :list-archive:list-unsubscribe:list-id:precedence :content-transfer-encoding:cc:to:subject:message-id:date :mime-version:dmarc-filter:delivered-to:dkim-signature:dkim-filter; bh=MzLuhIdxcHYj6D2Nxm3wGjLSkA5V/h8Rub6jbc00yBo=; fh=d5QGVRk2N5a5/4r89dmOt0RH0wgWbIxx7/c9RbuhBI0=; b=imK4Ju7hY4kC2eoFdyUXzYFndjnPMCU9EOQGi3RbbaHhdFdDm3Kmks2dYt/Br9Y4I8 sVLD8FvHi78uExLok5DIpXL3+uEy6gVKv7N8ijbVcPOzHf9oFrEE3NB1NISBhrsxVDTw C+yj3cmklerIT83kod/LNCXuuviMTdEbBA7KSTrrMnfCMA6Wc80wP2VhHU/RZUCdtjxe gSHQuX9FJjhkDuqp/BcDu5Ul5cIodalmlqZMs7jvzSv6OByK01qAEU5p7LE/T5TV25j9 GeQokIreDOMbOESpBL56c/ARkJyEIrfhsW/tc4hjKsAd1XktqAVZdBntxNTCjLFVpXA0 VbZA== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@gcc.gnu.org header.s=default header.b=pQ0iSAm3; spf=pass (google.com: domain of gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org designates 2620:52:3:1:0:246e:9693:128c as permitted sender) smtp.mailfrom="gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org"; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=gnu.org Received: from server2.sourceware.org (server2.sourceware.org. [2620:52:3:1:0:246e:9693:128c]) by mx.google.com with ESMTPS id o21-20020a056402039500b005233490a56bsi1895864edv.70.2023.08.10.11.07.35 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 10 Aug 2023 11:07:35 -0700 (PDT) Received-SPF: pass (google.com: domain of gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org designates 2620:52:3:1:0:246e:9693:128c as permitted sender) client-ip=2620:52:3:1:0:246e:9693:128c; Authentication-Results: mx.google.com; dkim=pass header.i=@gcc.gnu.org header.s=default header.b=pQ0iSAm3; spf=pass (google.com: domain of gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org designates 2620:52:3:1:0:246e:9693:128c as permitted sender) smtp.mailfrom="gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org"; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=gnu.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 237763857701 for ; Thu, 10 Aug 2023 18:07:34 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 237763857701 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1691690854; bh=MzLuhIdxcHYj6D2Nxm3wGjLSkA5V/h8Rub6jbc00yBo=; h=Date:Subject:To:Cc:List-Id:List-Unsubscribe:List-Archive: List-Post:List-Help:List-Subscribe:From:Reply-To:From; b=pQ0iSAm3PnkNS708gmrWr5tC2/TTt/gxH72VTuGO0VY6PJy3hk8ZQbdP+QlQ3yD8e o+53hgquFaWyYYwkMYj1n8Rnxq9NlUDDLa9mN+3NonEvRcd0HgRJHfagbMsUr5xIz2 yPWJ0VJT8qBoLZFSVP+YXDSUuHNoylTpoJMvhizU= X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from mail-lj1-x242.google.com (mail-lj1-x242.google.com [IPv6:2a00:1450:4864:20::242]) by sourceware.org (Postfix) with ESMTPS id 7568B3858D32 for ; Thu, 10 Aug 2023 18:06:49 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 7568B3858D32 Received: by mail-lj1-x242.google.com with SMTP id 38308e7fff4ca-2b9c66e2e36so13822011fa.1 for ; Thu, 10 Aug 2023 11:06:49 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1691690808; x=1692295608; h=cc:to:subject:message-id:date:from:mime-version:x-gm-message-state :from:to:cc:subject:date:message-id:reply-to; bh=wx8xn/y8fNvriO3h+uYQCQHLJh1VMCw3t4gS8E1F6nc=; b=UnwKSTpF3c686gI5lSHDJTiWIE5u4Zs/ges+qm/vXB9b3O2XbGTnz/8o1ENWEuF5Li 9aV+M/Xjn90+UzWPiRb0aDJjnU0YilfuiacpwXbKvyhzHDw8Yuf2/FZmuXSVdY6ZaIUZ 4tb3A3Sm1P1RLNrngc1bEPovFVeeqYpz1Jgx2wzirKzxABrguipX9tx/F6MqqPIWq5D8 qWns8Mm4pBE7JltAgypVdAkg9hGnXSAd3yhV2FGLG5wxebuf2r6HH8hVkb1OGWNWAvqp Fy+Ac1BjJguRF7nXu47EQe1m85EG5b9D86HryDrb26PcDUXNEe37mttYMussfNq+gnmr ZCNA== X-Gm-Message-State: AOJu0YzXXn3CArmjR7GVL3T3cpWxpWHRFfyPd+8eHCw8xeqj/2P1swck sXKNuXwKdDf9ASqozbwir0eAcQMS2xsXjCER7rLNp8Wp/BzUUkySTc4= X-Received: by 2002:a05:651c:389:b0:2b9:c689:3c33 with SMTP id e9-20020a05651c038900b002b9c6893c33mr883695ljp.19.1691690807386; Thu, 10 Aug 2023 11:06:47 -0700 (PDT) MIME-Version: 1.0 Date: Thu, 10 Aug 2023 20:06:36 +0200 Message-ID: Subject: [RFC PATCH v2] c++: extend cold, hot attributes to classes To: gcc-patches@gcc.gnu.org Cc: jason@redhat.com X-Spam-Status: No, score=-8.6 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_FROM, GIT_PATCH_0, HTML_MESSAGE, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-Content-Filtered-By: Mailman/MimeDel 2.1.29 X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-Patchwork-Original-From: Javier Martinez via Gcc-patches From: Javier Martinez Reply-To: Javier Martinez Errors-To: gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org Sender: "Gcc-patches" X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: 1773866430531386703 X-GMAIL-MSGID: 1773866430531386703 Thanks for the comments, Jason. v2: + Fix formatting, remove unnecessarily warning. On Tue, Aug 8, 2023 at 10:28 PM Jason Merrill wrote: > Seems reasonable, but how do you expect this to be used? We have large applications where some large classes are known to be used only at startup. Marking every member function as cold is impractical so we would mark those classes as cold. > I think doing this here will miss lazily-declared special member > functions; I wonder if you want to copy the attribute in grokclassfn > instead? I think they are being handled correctly with the current patch. Considered the following program: class __attribute((cold)) Foo { public: int m_x, m_y; auto operator<=>(const Foo&) const = default; }; int main(void) { Foo a{1,1}; Foo b{1,2}; std::set s; // OK s.insert(a); // OK std::cout << std::boolalpha << (a == b) << ' ' << (a != b) << ' ' << (a < b) << ' ' << (a <= b) << ' ' << (a > b) << ' ' << (a >= b) << ' ' << std::endl; } Per the rules for any operator<=> overload, a defaulted <=> overload will also allow the type to be compared with <, <=, >, and >=. If operator<=> is defaulted and operator== is not declared at all, then operator== is implicitly defaulted. The GIMPLE dump shows: > __attribute__((cold)) > struct strong_ordering Foo::operator<=> (const struct Foo * const this, const struct Foo & D.64195) > __attribute__((cold)) > bool Foo::operator== (const struct Foo * const this, const struct Foo & D.64206) I think this makes sense as add_implicitly_declared_members is called before my injection via finish_struct_1 -> check_bases_and_members. --- I would like some comment on the implications of: - { "cold", 0, 0, true, false, false, false, + { "cold", 0, 0, false, false, false, false, As I am now introducing a new warning that I cannot explain in an old test: testsuite/g++.dg/Wmissing-attributes.C:55:22: warning: 'hot' attribute ignored [-Wattributes] > template <> > void* > ATTR ((hot)) ATTR ((alloc_size (1))) ATTR ((malloc)) > missing_all(int); // T = char, same as above # Comparing 8 common sum files ## /bin/sh contrib/compare_tests /tmp/gxx-sum1.948624 /tmp/gxx-sum2.948624 Tests that now fail, but worked before (4 tests): g++.dg/Wmissing-attributes.C -std=gnu++14 (test for excess errors) g++.dg/Wmissing-attributes.C -std=gnu++17 (test for excess errors) g++.dg/Wmissing-attributes.C -std=gnu++20 (test for excess errors) g++.dg/Wmissing-attributes.C -std=gnu++98 (test for excess errors) New tests that PASS (20 tests): g++.dg/ext/attr-hotness.C -std=gnu++14 (test for warnings, line 11) g++.dg/ext/attr-hotness.C -std=gnu++14 (test for warnings, line 9) g++.dg/ext/attr-hotness.C -std=gnu++14 scan-tree-dump-times gimple "cold" 2 g++.dg/ext/attr-hotness.C -std=gnu++14 scan-tree-dump-times gimple "hot" 2 g++.dg/ext/attr-hotness.C -std=gnu++14 (test for excess errors) g++.dg/ext/attr-hotness.C -std=gnu++17 (test for warnings, line 11) g++.dg/ext/attr-hotness.C -std=gnu++17 (test for warnings, line 9) g++.dg/ext/attr-hotness.C -std=gnu++17 scan-tree-dump-times gimple "cold" 2 g++.dg/ext/attr-hotness.C -std=gnu++17 scan-tree-dump-times gimple "hot" 2 g++.dg/ext/attr-hotness.C -std=gnu++17 (test for excess errors) g++.dg/ext/attr-hotness.C -std=gnu++20 (test for warnings, line 11) g++.dg/ext/attr-hotness.C -std=gnu++20 (test for warnings, line 9) g++.dg/ext/attr-hotness.C -std=gnu++20 scan-tree-dump-times gimple "cold" 2 g++.dg/ext/attr-hotness.C -std=gnu++20 scan-tree-dump-times gimple "hot" 2 g++.dg/ext/attr-hotness.C -std=gnu++20 (test for excess errors) g++.dg/ext/attr-hotness.C -std=gnu++98 (test for warnings, line 11) g++.dg/ext/attr-hotness.C -std=gnu++98 (test for warnings, line 9) g++.dg/ext/attr-hotness.C -std=gnu++98 scan-tree-dump-times gimple "cold" 2 g++.dg/ext/attr-hotness.C -std=gnu++98 scan-tree-dump-times gimple "hot" 2 g++.dg/ext/attr-hotness.C -std=gnu++98 (test for excess errors) ## Differences found: # 1 differences in 8 common sum files found --- 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. (finish_struct): propagate hot and cold attributes to all FUNCTION_DECL when the class is marked hot or cold. gcc/testsuite/ChangeLog: * g++.dg/ext/attr-hotness.C: New test. Signed-off-by: Javier Martinez --- gcc/c-family/c-attribs.cc | 26 ++++++++++++-- gcc/cp/class.cc | 47 +++++++++++++++++++++++++ gcc/testsuite/g++.dg/ext/attr-hotness.C | 16 +++++++++ 3 files changed, 87 insertions(+), 2 deletions(-) create mode 100644 gcc/testsuite/g++.dg/ext/attr-hotness.C conflicts with attribute .cold." } */ + + +/* { dg-final { scan-tree-dump-times "cold" 2 "gimple" } } */ +/* { dg-final { scan-tree-dump-times "hot" 2 "gimple" } } */ + diff --git a/gcc/c-family/c-attribs.cc b/gcc/c-family/c-attribs.cc index e2792ca6898..090108ad94c 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,17 @@ 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 ()) + { + /* 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"); + } else { warning (OPT_Wattributes, "%qE attribute ignored", name); @@ -1131,6 +1142,17 @@ 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 ()) + { + /* 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"); + } else { warning (OPT_Wattributes, "%qE attribute ignored", name); diff --git a/gcc/cp/class.cc b/gcc/cp/class.cc index 778759237dc..9543ad69cb1 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. */ @@ -7733,6 +7734,48 @@ unreverse_member_declarations (tree t) } } +/* Classes marked with hotness attributes propagate the attribute to + all methods. */ +void +propagate_class_warmth_attribute (tree t) +{ + 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) + { + 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) + { + + /* 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 (class_has_cold_attr) + { + if (lookup_attribute ("hot", + DECL_ATTRIBUTES (f)) == NULL) + decl_attributes (&f, + tree_cons (attr_cold_id, NULL, NULL), 0); + } + else if (class_has_hot_attr) + { + if (lookup_attribute ("cold", + DECL_ATTRIBUTES (f)) == NULL) + decl_attributes (&f, + tree_cons (attr_hot_id, NULL, NULL), 0); + } + } + } +} + tree finish_struct (tree t, tree attributes) { @@ -7866,6 +7909,10 @@ finish_struct (tree t, tree attributes) && !LAMBDA_TYPE_P (t)) add_stmt (build_min (TAG_DEFN, t)); + /* This must be done after all lazily declared special member funcitons + have been injected. */ + propagate_class_warmth_attribute (t); + 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 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