From patchwork Mon Jul 1 15:36:58 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: David Malcolm X-Patchwork-Id: 256167 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (Client CN "localhost", Issuer "www.qmailtoaster.com" (not verified)) by ozlabs.org (Postfix) with ESMTPS id 8EED02C02EC for ; Tue, 2 Jul 2013 01:37:15 +1000 (EST) DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender :message-id:subject:from:to:date:content-type:mime-version; q= dns; s=default; b=fxX8dWtI4INc03+Kus57933WLkRJUB9JeA2vXYrON9gMew tYMZeKtN70yfjQkgnnWfEhLvxarHWuWIATfnh4wULT1mHKe3SOd4la8uzjPcKiGP zBmlbQcWkkQRQyTgWwIgD/f1ispWPUfzd+v7WkeJ8EJevOkB9925ShoJI4m3w= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender :message-id:subject:from:to:date:content-type:mime-version; s= default; bh=ZfH0Ss7NOBYdfX6cdiarLJh+82o=; b=gHFNnMdOyAvzd5KSeh/e hFXEjWiHjvyApZC1UgTmIfJ4tLjSSYJRi56Jcr7NIerBh/VoxUipFkFGf5EJ1ww/ DSQfIt8KkeyXU2f4KRQjt6C/LPTauAwDBlYJE4ah9S4jR/IwzmLwq4QN2JxyUZ4j 8b9hYDLSX0z+Bmxayuxddfk= Received: (qmail 31036 invoked by alias); 1 Jul 2013 15:37:08 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org Received: (qmail 31016 invoked by uid 89); 1 Jul 2013 15:37:06 -0000 X-Spam-SWARE-Status: No, score=-5.8 required=5.0 tests=AWL, BAYES_00, RCVD_IN_HOSTKARMA_W, RCVD_IN_HOSTKARMA_WL, RP_MATCHES_RCVD, SPF_HELO_PASS, SPF_PASS, TW_FN, TW_KF autolearn=no version=3.3.1 Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.84/v0.84-167-ge50287c) with ESMTP; Mon, 01 Jul 2013 15:37:04 +0000 Received: from int-mx02.intmail.prod.int.phx2.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id r61Fb2B9001325 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Mon, 1 Jul 2013 11:37:03 -0400 Received: from [10.18.25.93] ([10.18.25.93]) by int-mx02.intmail.prod.int.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id r61Fb1qb000940 for ; Mon, 1 Jul 2013 11:37:02 -0400 Message-ID: <1372693018.1789.110.camel@surprise> Subject: [C++ PATCH] Implement new "force_static" attribute From: David Malcolm To: gcc-patches@gcc.gnu.org Date: Mon, 01 Jul 2013 11:36:58 -0400 Mime-Version: 1.0 X-Virus-Found: No My plan for removal of global variables in gcc 4.9 [1] calls for several hundred new classes, which will be singletons in a classic monolithic build, but have multiple instances in a shared-library build. In order to avoid the register pressure of passing a redundant "this" pointer around for the classic case, I've been looking at optimizing singletons. I'm attaching an optimization for this: a new "force_static" attribute for the C++ frontend, which when added to a class implicitly adds "static" to all members of said class. This gives a way of avoiding a "this" pointer in the classic build (in stages 2 and 3, once the attribute is recognized), whilst supporting it in a shared-library build, with relatively little boilerplate, preprocessor hackery or syntactic differences. See: http://dmalcolm.fedorapeople.org/gcc/global-state/singletons.html#another-singleton-removal-optimization for more information on how this would be used in GCC itself. With this optimization, the generated machine code *with classes* (with "methods" and "fields") is identical to that with just functions and global variables (apart from the ordering of the functions/"methods" within the .text sections of their respective .o files). [2] FWIW I've also been looking at another approach: http://dmalcolm.fedorapeople.org/gcc/global-state/singletons.html#a-singleton-removal-optimization which is even lower boilerplate, though I don't have that working yet; it touches the internals of classes and methods much more deeply. BTW, I'm not 100% sold on "force_static" as the name of the attribute; would "implicit_static" be a better name? (the latter is growing on me). Successfully bootstrapped on x86_64-unknown-linux-gnu; all old testcases have the same results as an unpatched build, and all new testcases pass (using r200562 as the baseline). Dave [1] See http://gcc.gnu.org/ml/gcc/2013-06/msg00215.html [2] I've written an "asmdiff" tool to help check this: https://github.com/davidmalcolm/asmdiff diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h index 2931ac5..17cf35e 100644 --- a/gcc/cp/cp-tree.h +++ b/gcc/cp/cp-tree.h @@ -1416,6 +1416,7 @@ struct GTY(()) lang_type_class { unsigned has_complex_move_assign : 1; unsigned has_constexpr_ctor : 1; unsigned is_final : 1; + unsigned force_static : 1; /* When adding a flag here, consider whether or not it ought to apply to a template instance if it applies to the template. If @@ -1424,7 +1425,7 @@ struct GTY(()) lang_type_class { /* There are some bits left to fill out a 32-bit word. Keep track of this by updating the size of this bitfield whenever you add or remove a flag. */ - unsigned dummy : 2; + unsigned dummy : 1; tree primary_base; vec *vcall_indices; @@ -1536,6 +1537,10 @@ struct GTY((variable_size)) lang_type { #define CLASSTYPE_FINAL(NODE) \ (LANG_TYPE_CLASS_CHECK (NODE)->is_final) +/* Nonzero means that NODE (a class type) has the force_static + attribute. */ +#define CLASSTYPE_FORCE_STATIC(NODE) \ + (LANG_TYPE_CLASS_CHECK (NODE)->force_static) /* Nonzero means that this _CLASSTYPE node overloads operator=(X&). */ #define TYPE_HAS_COPY_ASSIGN(NODE) (LANG_TYPE_CLASS_CHECK (NODE)->has_copy_assign) diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c index 9eb1d12..d12b2a8 100644 --- a/gcc/cp/decl.c +++ b/gcc/cp/decl.c @@ -7575,10 +7575,24 @@ grokfndecl (tree ctype, { if (quals) { - error (ctype - ? G_("static member function %qD cannot have cv-qualifier") - : G_("non-member function %qD cannot have cv-qualifier"), - decl); + if (ctype) + { + if (CLASS_TYPE_P (ctype) && CLASSTYPE_FORCE_STATIC (ctype)) + /* It's useful to be able to mark methods of force_static + classes as "const" etc so that we can toggle the + presence of the "force_static" attribute without needing + to remove the const qualifiers. + + Silently discard such markings. */ + ; + else + error (G_("static member function %qD cannot have" + " cv-qualifier"), + decl); + } + else + error (G_("non-member function %qD cannot have cv-qualifier"), + decl); quals = TYPE_UNQUALIFIED; } @@ -9243,7 +9257,9 @@ grokdeclarator (const cp_declarator *declarator, explicitp = decl_spec_seq_has_spec_p (declspecs, ds_explicit); storage_class = declspecs->storage_class; - if (storage_class == sc_static) + if (storage_class == sc_static + || (current_class_type && + CLASSTYPE_FORCE_STATIC (current_class_type))) staticp = 1 + (decl_context == FIELD); if (virtualp && staticp == 2) diff --git a/gcc/cp/ptree.c b/gcc/cp/ptree.c index f4ca003..f4e5cd0 100644 --- a/gcc/cp/ptree.c +++ b/gcc/cp/ptree.c @@ -165,6 +165,8 @@ cxx_print_type (FILE *file, tree node, int indent) fprintf (file, " interface-only"); if (CLASSTYPE_INTERFACE_UNKNOWN (node)) fprintf (file, " interface-unknown"); + if (CLASSTYPE_FORCE_STATIC (node)) + fprintf (file, " force_static"); } } diff --git a/gcc/cp/tree.c b/gcc/cp/tree.c index 8524f6c..b5d5879 100644 --- a/gcc/cp/tree.c +++ b/gcc/cp/tree.c @@ -47,6 +47,7 @@ static tree handle_java_interface_attribute (tree *, tree, tree, int, bool *); static tree handle_com_interface_attribute (tree *, tree, tree, int, bool *); static tree handle_init_priority_attribute (tree *, tree, tree, int, bool *); static tree handle_abi_tag_attribute (tree *, tree, tree, int, bool *); +static tree handle_force_static_attribute (tree *, tree, tree, int, bool *); /* If REF is an lvalue, returns the kind of lvalue that REF is. Otherwise, returns clk_none. */ @@ -3157,6 +3158,8 @@ const struct attribute_spec cxx_attribute_table[] = handle_init_priority_attribute, false }, { "abi_tag", 1, -1, false, false, false, handle_abi_tag_attribute, true }, + { "force_static", 0, 0, false, true, false, + handle_force_static_attribute, false }, { NULL, 0, 0, false, false, false, NULL, false } }; @@ -3378,6 +3381,39 @@ handle_abi_tag_attribute (tree* node, tree name, tree args, return NULL_TREE; } +/* Handle a "force_static" attribute; arguments as in + struct attribute_spec.handler. */ +static tree +handle_force_static_attribute (tree* node, + tree name, + tree /*args*/, + int /*flags*/, + bool* /*no_add_attrs*/) +{ + if (!CLASS_TYPE_P (*node)) + { + warning (OPT_Wattributes, "%qE attribute can only be applied " + "to class definitions", name); + return NULL_TREE; + } + + /* The attribute affects how decls within the class body are grokked. + If the attribute is seen after the body of the class has been handled, + it's too late. */ + if (TYPE_FIELDS (*node)) + { + warning (OPT_Wattributes, + ("%qE attribute must be applied after the class keyword," + " not after the closing brace"), + name); + return NULL_TREE; + } + + CLASSTYPE_FORCE_STATIC (*node) = 1; + + return NULL_TREE; +} + /* Return a new PTRMEM_CST of the indicated TYPE. The MEMBER is the thing pointed to by the constant. */ diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi index 6ce26ef..6d33191 100644 --- a/gcc/doc/extend.texi +++ b/gcc/doc/extend.texi @@ -5539,6 +5539,46 @@ and caught in another, the class must have default visibility. Otherwise the two shared objects are unable to use the same typeinfo node and exception handling will break. +@item force_static +In C++, a class or struct can be marked with the ``force_static'' attribute +to make it easier to implement singletons. + +This implicitly adds the ``static'' keyword to all methods and data, so +that the methods lose the implicit ``this'' that they would otherwise +have, thus allowing for more efficient generated code. + +The attribute must appear between the ``class'' or ``struct'' keyword and +the name of the type; it may not appear between the closing parenthesis +of the body of the type and the trailing semicolon. + +@smallexample +class __attribute__((force_static)) foo +@{ +public: + void some_method (); + +private: + int some_field; +@}; + +/* Instances of the type can still be created, but they become empty. */ +extern foo the_foo; + +/* Any instances share the same data, as if there was just one + instance. */ +int foo::some_field; + +void bar (void) +@{ + /* You can invoke methods on such a class... */ + the_foo.some_method (); + + /* ...where the attribute makes the above be implicitly equivalent to + this: */ + foo::some_method (); +@} +@end smallexample + @end table To specify multiple attributes, separate them by commas within the diff --git a/gcc/testsuite/g++.dg/ext/force_static/attribute.C b/gcc/testsuite/g++.dg/ext/force_static/attribute.C new file mode 100644 index 0000000..5f315e4 --- /dev/null +++ b/gcc/testsuite/g++.dg/ext/force_static/attribute.C @@ -0,0 +1,7 @@ +class __attribute__((force_static)) foo +{ +private: + int i; +}; + +int foo::i; diff --git a/gcc/testsuite/g++.dg/ext/force_static/callsite.C b/gcc/testsuite/g++.dg/ext/force_static/callsite.C new file mode 100644 index 0000000..3a11884 --- /dev/null +++ b/gcc/testsuite/g++.dg/ext/force_static/callsite.C @@ -0,0 +1,19 @@ +// { dg-options "-fdump-tree-original" } + +class __attribute__((force_static)) foo +{ +public: + int some_method (int i); + +private: + int some_field; +}; + +int test_callsite () +{ + extern foo the_foo; + return the_foo.some_method (42); +} + +// { dg-final { scan-tree-dump "foo::some_method" "original" } } +// { dg-final { cleanup-tree-dump original } } diff --git a/gcc/testsuite/g++.dg/ext/force_static/const.C b/gcc/testsuite/g++.dg/ext/force_static/const.C new file mode 100644 index 0000000..5f2581f --- /dev/null +++ b/gcc/testsuite/g++.dg/ext/force_static/const.C @@ -0,0 +1,7 @@ +class __attribute__((force_static)) foo +{ +public: + /* It's harmless to have a "const" qualifier on a method of + such a class. */ + void bar () const; +}; diff --git a/gcc/testsuite/g++.dg/ext/force_static/ctor.C b/gcc/testsuite/g++.dg/ext/force_static/ctor.C new file mode 100644 index 0000000..7d499ec --- /dev/null +++ b/gcc/testsuite/g++.dg/ext/force_static/ctor.C @@ -0,0 +1,5 @@ +class __attribute__((force_static)) foo +{ +public: + foo(); /* { dg-error "constructor cannot be static member function" } */ +}; diff --git a/gcc/testsuite/g++.dg/ext/force_static/dtor.C b/gcc/testsuite/g++.dg/ext/force_static/dtor.C new file mode 100644 index 0000000..a62ba57 --- /dev/null +++ b/gcc/testsuite/g++.dg/ext/force_static/dtor.C @@ -0,0 +1,5 @@ +class __attribute__((force_static)) foo +{ +public: + ~foo(); /* { dg-error "destructor cannot be static member function" } */ +}; diff --git a/gcc/testsuite/g++.dg/ext/force_static/method.C b/gcc/testsuite/g++.dg/ext/force_static/method.C new file mode 100644 index 0000000..b94de84 --- /dev/null +++ b/gcc/testsuite/g++.dg/ext/force_static/method.C @@ -0,0 +1,19 @@ +// { dg-options "-fdump-tree-original" } + +class __attribute__((force_static)) foo +{ +public: + int some_method (int i); + +private: + int some_field; +}; + +int foo::some_method (int i) +{ + some_field += i; + return some_field; +} + +// { dg-final { scan-tree-dump-not "this" "original" } } +// { dg-final { cleanup-tree-dump original } } diff --git a/gcc/testsuite/g++.dg/ext/force_static/not-a-class.C b/gcc/testsuite/g++.dg/ext/force_static/not-a-class.C new file mode 100644 index 0000000..0cda06c --- /dev/null +++ b/gcc/testsuite/g++.dg/ext/force_static/not-a-class.C @@ -0,0 +1 @@ +int i __attribute__((force_static)); /* { dg-warning "'force_static' attribute can only be applied to class definitions" } */ diff --git a/gcc/testsuite/g++.dg/ext/force_static/struct.C b/gcc/testsuite/g++.dg/ext/force_static/struct.C new file mode 100644 index 0000000..8ba5a50 --- /dev/null +++ b/gcc/testsuite/g++.dg/ext/force_static/struct.C @@ -0,0 +1,6 @@ +struct __attribute__((force_static)) foo +{ + int i; +}; + +int foo::i; diff --git a/gcc/testsuite/g++.dg/ext/force_static/trailing.C b/gcc/testsuite/g++.dg/ext/force_static/trailing.C new file mode 100644 index 0000000..b6dfa2f --- /dev/null +++ b/gcc/testsuite/g++.dg/ext/force_static/trailing.C @@ -0,0 +1,3 @@ +class foo +{ +} __attribute__((force_static)); /* { dg-warning "'force_static' attribute must be applied after the class keyword, not after the closing brace" } */ diff --git a/gcc/testsuite/g++.dg/ext/force_static/vfunc.C b/gcc/testsuite/g++.dg/ext/force_static/vfunc.C new file mode 100644 index 0000000..b0da35d --- /dev/null +++ b/gcc/testsuite/g++.dg/ext/force_static/vfunc.C @@ -0,0 +1,5 @@ +class __attribute__((force_static)) foo +{ +public: + virtual void bar (); /* { dg-error "member 'bar' cannot be declared both virtual and static" } */ +};