From patchwork Tue Jul 24 15:20:52 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Laurynas Biveinis X-Patchwork-Id: 172895 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]) by ozlabs.org (Postfix) with SMTP id ECE6E2C008E for ; Wed, 25 Jul 2012 01:21:38 +1000 (EST) Comment: DKIM? See http://www.dkim.org DKIM-Signature: v=1; a=rsa-sha1; c=relaxed/relaxed; d=gcc.gnu.org; s=default; x=1343748100; h=Comment: DomainKey-Signature:Received:Received:Received:Received:Received: MIME-Version:Received:In-Reply-To:References:From:Date: Message-ID:Subject:To:Cc:Content-Type:Mailing-List:Precedence: List-Id:List-Unsubscribe:List-Archive:List-Post:List-Help:Sender: Delivered-To; bh=R3c/F69Uf6sy5KL+qSbTdHGt+ow=; b=MDnA+oaHmVrKnMY EufE03mcNTze8uErAARxaakLhQWwLJLpTfzF2dV+SFTEaDpzPvFDaCB1qRdAEZKv Z0g2xhhZZG3BnYbiLz08+LcVmboAkImdFpdfpipmZV38efy3jzszaf+/xQyUb+pY QVsW3CceYNyhsM8YfpM+AuRJ6RwI= Comment: DomainKeys? See http://antispam.yahoo.com/domainkeys DomainKey-Signature: a=rsa-sha1; q=dns; c=nofws; s=default; d=gcc.gnu.org; h=Received:Received:X-SWARE-Spam-Status:X-Spam-Check-By:Received:Received:Received:MIME-Version:Received:In-Reply-To:References:From:Date:Message-ID:Subject:To:Cc:Content-Type:X-IsSubscribed:Mailing-List:Precedence:List-Id:List-Unsubscribe:List-Archive:List-Post:List-Help:Sender:Delivered-To; b=UviFClzHBTHV+jNeDF1SCbGr1lZYZbnI1Vm3pVysBWv+eeXuCcHkNtimopD34A eYKW4gf1QYgmfIl+gHRz2FjEko0+dfnHHB4rLvPD3q9au4XxublYL1FUlLBiQzeI xl4Iorl0rbPTfwv4pYYgqL/Tr4zQ+8dmxWxcBvmPyBitk=; Received: (qmail 4402 invoked by alias); 24 Jul 2012 15:21:34 -0000 Received: (qmail 4378 invoked by uid 22791); 24 Jul 2012 15:21:32 -0000 X-SWARE-Spam-Status: No, hits=-4.7 required=5.0 tests=AWL, BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, FREEMAIL_FROM, KHOP_RCVD_TRUST, KHOP_THREADED, RCVD_IN_DNSWL_LOW, RCVD_IN_HOSTKARMA_YE X-Spam-Check-By: sourceware.org Received: from mail-yx0-f175.google.com (HELO mail-yx0-f175.google.com) (209.85.213.175) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Tue, 24 Jul 2012 15:21:14 +0000 Received: by yenl13 with SMTP id l13so8260641yen.20 for ; Tue, 24 Jul 2012 08:21:13 -0700 (PDT) Received: by 10.42.66.13 with SMTP id n13mr15915373ici.39.1343143272877; Tue, 24 Jul 2012 08:21:12 -0700 (PDT) MIME-Version: 1.0 Received: by 10.64.28.145 with HTTP; Tue, 24 Jul 2012 08:20:52 -0700 (PDT) In-Reply-To: References: From: Laurynas Biveinis Date: Tue, 24 Jul 2012 18:20:52 +0300 Message-ID: Subject: Re: [patch] Stop gengtype from emitting empty loops To: Steven Bosscher Cc: Richard Guenther , GCC Patches X-IsSubscribed: yes 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 >>> I think it merely points to a bogus GTY annotation, not sure if we want to >>> gobble this kind of gengtype hacks which only benefit -O0 ... > ../../trunk/gcc/../libcpp/include/line-map.h:168: option `length' may > not be applied to arrays of scalar types > ../../trunk/gcc/emit-rtl.c:5913: option `length' may not be applied to > arrays of scalar types > ../../trunk/gcc/java/jcf.h:85: option `length' may not be applied to > arrays of scalar types > ../../trunk/gcc/java/jcf.h:94: option `length' may not be applied to > arrays of scalar types Below is the patch that fixes all these. A gtype-desc.c diff shows a number of empty loops removed. Regarding vector, I didn't see any other way to fix it (except ignoring it) but invent a new type of vector for atomic types. > Index: gengtype.c > =================================================================== > --- gengtype.c (revision 189778) > +++ gengtype.c (working copy) > @@ -1256,7 +1256,17 @@ adjust_field_type (type_p t, options_p opt) > > for (; opt; opt = opt->next) > if (strcmp (opt->name, "length") == 0) > - length_p = 1; > + { > + if (length_p) > + error_at_line (&lexer_line, "duplicate `%s' option", opt->name); > + if (t->u.p->kind == TYPE_SCALAR && ! t->u.p->u.scalar_is_char) > + { > + error_at_line (&lexer_line, > + "option `%s' may not be applied to arrays > of scalar types", > + opt->name); > + } > + length_p = 1; > + } > else if ((strcmp (opt->name, "param_is") == 0 > || (strncmp (opt->name, "param", 5) == 0 > && ISDIGIT (opt->name[5]) s/arrays of scalar types/arrays of atomic types, add a ChangeLog entry and the snippet above is approved ;) libcpp: 2012-06-24 Laurynas Bivienis * include/line-map.h (line_map_macro): Use the "atomic" GTY option for the macro_locations field. gcc/java: 2012-07-24 Laurynas Biveinis * jcf.h (CPool): Use the "atomic" GTY option for the tags field. (bootstrap_method): Likewise for the bootstrap_arguments field. gcc: 2012-07-24 Laurynas Biveinis * doc/gty.texi: Clarify that GTY option "length" is only for arrays of non-atomic objects. Fix typo in the description of the "atomic" option. * vec.h: Describe the atomic object "A" type of the macros in the header comment. (VEC_T_GTY_ATOMIC, DEF_VEC_A, DEF_VEC_ALLOC_A): Define. * emit-rtl.c (locations_locators_vals): use the atomic object vector. OK for trunk if it passes testing? Index: libcpp/include/line-map.h =================================================================== --- libcpp/include/line-map.h (revision 189802) +++ libcpp/include/line-map.h (working copy) @@ -165,7 +165,7 @@ In the example above x1 (for token "+") is going to be the same as y1. x0 is the spelling location for the argument token "1", and x2 is the spelling location for the argument token "2". */ - source_location * GTY((length ("2 * %h.n_tokens"))) macro_locations; + source_location * GTY((atomic)) macro_locations; /* This is the location of the expansion point of the current macro map. It's the location of the macro name. That location is held Index: gcc/emit-rtl.c =================================================================== --- gcc/emit-rtl.c (revision 189802) +++ gcc/emit-rtl.c (working copy) @@ -5910,8 +5910,8 @@ static VEC(int,heap) *block_locators_locs; static GTY(()) VEC(tree,gc) *block_locators_blocks; static VEC(int,heap) *locations_locators_locs; -DEF_VEC_O(location_t); -DEF_VEC_ALLOC_O(location_t,heap); +DEF_VEC_A(location_t); +DEF_VEC_ALLOC_A(location_t,heap); static VEC(location_t,heap) *locations_locators_vals; int prologue_locator; int epilogue_locator; Index: gcc/vec.h =================================================================== --- gcc/vec.h (revision 189802) +++ gcc/vec.h (working copy) @@ -95,24 +95,25 @@ the 'space' predicate will tell you whether there is spare capacity in the vector. You will not normally need to use these two functions. - Vector types are defined using a DEF_VEC_{O,P,I}(TYPEDEF) macro, to + Vector types are defined using a DEF_VEC_{O,A,P,I}(TYPEDEF) macro, to get the non-memory allocation version, and then a - DEF_VEC_ALLOC_{O,P,I}(TYPEDEF,ALLOC) macro to get memory managed + DEF_VEC_ALLOC_{O,A,P,I}(TYPEDEF,ALLOC) macro to get memory managed vectors. Variables of vector type are declared using a VEC(TYPEDEF,ALLOC) macro. The ALLOC argument specifies the allocation strategy, and can be either 'gc' or 'heap' for garbage collected and heap allocated respectively. It can be 'none' to get a vector that must be explicitly allocated (for instance as a - trailing array of another structure). The characters O, P and I - indicate whether TYPEDEF is a pointer (P), object (O) or integral - (I) type. Be careful to pick the correct one, as you'll get an - awkward and inefficient API if you use the wrong one. There is a - check, which results in a compile-time warning, for the P and I - versions, but there is no check for the O versions, as that is not - possible in plain C. Due to the way GTY works, you must annotate - any structures you wish to insert or reference from a vector with a - GTY(()) tag. You need to do this even if you never declare the GC - allocated variants. + trailing array of another structure). The characters O, A, P and I + indicate whether TYPEDEF is a pointer (P), object (O), atomic object + (A) or integral (I) type. Be careful to pick the correct one, as + you'll get an awkward and inefficient API if you use the wrong one or + a even a crash if you pick the atomic object version when the object + version should have been chosen instead. There is a check, which + results in a compile-time warning, for the P and I versions, but there + is no check for the O versions, as that is not possible in plain C. + Due to the way GTY works, you must annotate any structures you wish to + insert or reference from a vector with a GTY(()) tag. You need to do + this even if you never declare the GC allocated variants. An example of their use would be, @@ -535,6 +536,13 @@ T GTY ((length ("%h.prefix.num"))) vec[1]; \ } VEC(T,B) +#define VEC_T_GTY_ATOMIC(T,B) \ +typedef struct GTY(()) VEC(T,B) \ +{ \ + struct vec_prefix prefix; \ + T GTY ((atomic)) vec[1]; \ +} VEC(T,B) + /* Derived vector type, user visible. */ #define VEC_TA_GTY(T,B,A,GTY) \ typedef struct GTY VEC(T,A) \ @@ -909,6 +917,14 @@ DEF_VEC_NONALLOC_FUNCS_O(T,A) \ struct vec_swallow_trailing_semi +/* Vector of atomic object. */ +#define DEF_VEC_A(T) \ +VEC_T_GTY_ATOMIC(T,base); \ +VEC_TA(T,base,none); \ +DEF_VEC_FUNC_O(T) \ +struct vec_swallow_trailing_semi +#define DEF_VEC_ALLOC_A(T,A) DEF_VEC_ALLOC_O(T,A) + #define DEF_VEC_FUNC_O(T) \ static inline unsigned VEC_OP (T,base,length) (const VEC(T,base) *vec_) \ { \ Index: gcc/java/jcf.h =================================================================== --- gcc/java/jcf.h (revision 189802) +++ gcc/java/jcf.h (working copy) @@ -82,7 +82,7 @@ /* The constant_pool_count. */ int count; - uint8* GTY((length ("%h.count"))) tags; + uint8 * GTY((atomic)) tags; union cpool_entry * GTY((length ("%h.count"), desc ("cpool_entry_is_tree (%1.tags%a)"))) data; @@ -91,7 +91,7 @@ typedef struct GTY(()) bootstrap_method { unsigned method_ref; unsigned num_arguments; - unsigned* GTY((length ("%h.num_arguments"))) bootstrap_arguments; + unsigned * GTY((atomic)) bootstrap_arguments; } bootstrap_method; typedef struct GTY(()) BootstrapMethods { Index: gcc/doc/gty.texi =================================================================== --- gcc/doc/gty.texi (revision 189802) +++ gcc/doc/gty.texi (working copy) @@ -134,8 +134,8 @@ @item length ("@var{expression}") There are two places the type machinery will need to be explicitly told -the length of an array. The first case is when a structure ends in a -variable-length array, like this: +the length of an array of non-atomic objects. The first case is when a +structure ends in a variable-length array, like this: @smallexample struct GTY(()) rtvec_def @{ int num_elem; /* @r{number of elements} */ @@ -163,6 +163,11 @@ static GTY((length("reg_known_value_size"))) rtx *reg_known_value; @end verbatim +Note that the @code{length} option is only meant for use with arrays of +non-atomic objects, that is, objects that contain pointers pointing to +other GTY-managed objects. For other arrays you should use @code{atomic} +or even @code{skip}. + @findex skip @item skip @@ -411,7 +416,7 @@ @smallexample struct GTY(()) my_struct @{ int number_of_elements; - unsigned int GTY ((atomic)) * elements; + unsigned int * GTY ((atomic)) elements; @}; @end smallexample In this case, @code{elements} is a pointer under GC, and the memory it