diff mbox

: Use GTY atomic option for arrays, new atomic vector type

Message ID 5012909E.5070309@gmail.com
State New
Headers show

Commit Message

Laurynas Biveinis July 27, 2012, 12:59 p.m. UTC
This is a slightly expanded version of the patch in [1]. The main difference is that I expanded gengtype diagnostics to error on GTY length option applied to strings too, in addition to other arrays of atomic types. This in turn uncovered another place where a correct GTY option should be used .

Java and libcpp parts are already approved.

Tested on x86_64 linux, no regressions. OK for trunk?

[1] http://gcc.gnu.org/ml/gcc-patches/2012-07/msg01204.html

gcc:
2012-07-27  Laurynas Biveinis  <laurynas.biveinis@gmail.com>
	    Steven Bosscher  <steven@gcc.gnu.org>

	* gengtype.c (adjust_field_type): Diagnose duplicate "length"
	option applications and option being applied to arrays of atomic
	types.
	(walk_type): Allow "atomic" option on strings too.
	* dwarf2out.h (struct dw_vec_struct): Use the "atomic" GTY option
	for the array field.
	* 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.
	* 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.

gcc/java:
2012-07-24  Laurynas Biveinis  <laurynas.biveinis@gmail.com>

	* jcf.h (CPool): Use the "atomic" GTY option for the tags field.
	(bootstrap_method): Likewise for the bootstrap_arguments field.

libcpp:
2012-07-24  Laurynas Biveinis  <laurynas.biveinis@gmail.com>

	* include/line-map.h (line_map_macro): Use the "atomic" GTY option
	for the macro_locations field.

Comments

Richard Biener July 27, 2012, 1:21 p.m. UTC | #1
On Fri, Jul 27, 2012 at 2:59 PM, Laurynas Biveinis
<laurynas.biveinis@gmail.com> wrote:
> This is a slightly expanded version of the patch in [1]. The main difference is that I expanded gengtype diagnostics to error on GTY length option applied to strings too, in addition to other arrays of atomic types. This in turn uncovered another place where a correct GTY option should be used .
>
> Java and libcpp parts are already approved.
>
> Tested on x86_64 linux, no regressions. OK for trunk?

Ok.

Thanks,
Richard.

> [1] http://gcc.gnu.org/ml/gcc-patches/2012-07/msg01204.html
>
> gcc:
> 2012-07-27  Laurynas Biveinis  <laurynas.biveinis@gmail.com>
>             Steven Bosscher  <steven@gcc.gnu.org>
>
>         * gengtype.c (adjust_field_type): Diagnose duplicate "length"
>         option applications and option being applied to arrays of atomic
>         types.
>         (walk_type): Allow "atomic" option on strings too.
>         * dwarf2out.h (struct dw_vec_struct): Use the "atomic" GTY option
>         for the array field.
>         * 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.
>         * 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.
>
> gcc/java:
> 2012-07-24  Laurynas Biveinis  <laurynas.biveinis@gmail.com>
>
>         * jcf.h (CPool): Use the "atomic" GTY option for the tags field.
>         (bootstrap_method): Likewise for the bootstrap_arguments field.
>
> libcpp:
> 2012-07-24  Laurynas Biveinis  <laurynas.biveinis@gmail.com>
>
>         * include/line-map.h (line_map_macro): Use the "atomic" GTY option
>         for the macro_locations field.
>
>
> --
> Laurynas
diff mbox

Patch

gcc:
2012-07-27  Laurynas Biveinis  <laurynas.biveinis@gmail.com>
	    Steven Bosscher  <steven@gcc.gnu.org>

	* gengtype.c (adjust_field_type): Diagnose duplicate "length"
	option applications and option being applied to arrays of atomic
	types.
	(walk_type): Allow "atomic" option on strings too.
	* dwarf2out.h (struct dw_vec_struct): Use the "atomic" GTY option
	for the array field.
	* 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.
	* 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.

gcc/java:
2012-07-24  Laurynas Biveinis  <laurynas.biveinis@gmail.com>

	* jcf.h (CPool): Use the "atomic" GTY option for the tags field.
	(bootstrap_method): Likewise for the bootstrap_arguments field.

libcpp:
2012-07-24  Laurynas Biveinis  <laurynas.biveinis@gmail.com>

	* include/line-map.h (line_map_macro): Use the "atomic" GTY option
	for the macro_locations field.

Index: gcc/gengtype.c
===================================================================
--- gcc/gengtype.c	(revision 189893)
+++ gcc/gengtype.c	(working copy)
@@ -1256,7 +1256,17 @@ 
 
   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->kind == TYPE_STRING)
+	  {
+	    error_at_line (&lexer_line,
+			   "option `%s' may not be applied to "
+			   "arrays of atomic types", opt->name);
+	  }
+	length_p = 1;
+      }
     else if ((strcmp (opt->name, "param_is") == 0
 	      || (strncmp (opt->name, "param", 5) == 0
 		  && ISDIGIT (opt->name[5])
@@ -2495,7 +2505,7 @@ 
       return;
     }
 
-  if (atomic_p && (t->kind != TYPE_POINTER))
+  if (atomic_p && (t->kind != TYPE_POINTER) && (t->kind != TYPE_STRING))
     {
       error_at_line (d->line, "field `%s' has invalid option `atomic'\n", d->val);
       return;
Index: gcc/dwarf2out.h
===================================================================
--- gcc/dwarf2out.h	(revision 189893)
+++ gcc/dwarf2out.h	(working copy)
@@ -160,7 +160,7 @@ 
 /* Describe a floating point constant value, or a vector constant value.  */
 
 typedef struct GTY(()) dw_vec_struct {
-  unsigned char * GTY((length ("%h.length"))) array;
+  unsigned char * GTY((atomic)) array;
   unsigned length;
   unsigned elt_size;
 }
Index: gcc/vec.h
===================================================================
--- gcc/vec.h	(revision 189893)
+++ 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,
 
@@ -530,6 +531,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)						  \
@@ -904,6 +912,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 189893)
+++ 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/emit-rtl.c
===================================================================
--- gcc/emit-rtl.c	(revision 189893)
+++ 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/doc/gty.texi
===================================================================
--- gcc/doc/gty.texi	(revision 189893)
+++ 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 GC-allocated arrays and strings
+you should use @code{atomic}.
+
 @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
Index: libcpp/include/line-map.h
===================================================================
--- libcpp/include/line-map.h	(revision 189893)
+++ 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