[scalar-storage-order] Small tweaks

[scalar-storage-order] Small tweaks

[Eric Botcazou]

[-- Attachment #1: Type: text/plain, Size: 745 bytes --]

This adjusts the type adjustment code for array components in the C and C++ 
front-ends as per Joseph's remark, fixes a couple of documentation glitches 
and adds 3 more testcases following Richard's questions.

Joseph, is that what you had in mind?

Tested on x86_64-suse-linux.


	* doc/extend.texi (scalar_storage_order): Move around and fix glitches.
c-family/
	* c-common.c (c_common_attributes): Move scalar_storage_order around.
	(handle_scalar_storage_order_attribute): Move around.
c/
	* c-decl.c (finish_struct): Properly rewrite the type of array fields.
cp/
	* class.c (finish_struct_1): Likewise.
testsuite/
	* c-c++-common/sso-2.c: New test.
	* c-c++-common/sso-3.c: Likewise.
	* c-c++-common/sso-4.c: Likewise.


-- 
Eric Botcazou

[-- Attachment #2: p.diff --]
[-- Type: text/x-patch, Size: 16802 bytes --]

Index: doc/extend.texi
===================================================================
--- doc/extend.texi	(revision 224467)
+++ doc/extend.texi	(working copy)
@@ -6064,53 +6064,6 @@ S *p = (S *)malloc (sizeof(S) + 100);
 p->data[10] = 0; //OK
 @end smallexample
 
-@item scalar_storage_order
-@itemx scalar_storage_order ("@var{endianness}")
-@cindex @code{scalar_storage_order} type attribute
-When attached to a @code{union} or a @code{struct}, this attribute sets
-the storage order, aka endianness, of the scalar fields of the type, as
-well as the array fields whose component is scalar.  The supported
-endianness are @code{big-endian} and @code{little-endian}.  The attribute
-has no effects on fields which are themselves a @code{union}, a @code{struct}
-or an array whose component is a @code{union} or a @code{struct}, and it is
-possible to have fields with a different scalar storage order than the
-enclosing type.
-
-This attribute is supported only for targets that use a uniform default
-scalar storage order (fortunately, most of them), i.e. targets that store
-the scalars either all in big-endian or all in little-endian.
-
-Additional restrictions are enforced for types with the reverse scalar
-storage order with regard to the scalar storage order of the target:
-
-@itemize
-@item Taking the address of a scalar field of a @code{union} or a
-@code{struct} with reverse scalar storage order is illegal and will
-yield an error
-@item Taking the address of an array field, whose component is scalar, of
-a @code{union} or a @code{struct} with reverse scalar storage order is
-permitted but will yield a warning, unless @option{-Wno-scalar-storage-order}
-is specified
-@item Taking the address of a @code{union} or a @code{struct} with reverse
-scalar storage order is permitted
-@end itemize
-
-These restrictions exist because the storage order attribute is lost when
-the address of a scalar or the address of an array with scalar component
-is taken, so storing indirectly through this address will generally not work.
-The second case is nevertheless allowed to be able to perform a block copy
-from or to the array.
-
-@item unused
-@cindex @code{unused} type attribute
-When attached to a type (including a @code{union} or a @code{struct}),
-this attribute means that variables of that type are meant to appear
-possibly unused.  GCC does not produce a warning for any variables of
-that type, even if the variable appears to do nothing.  This is often
-the case with lock or thread classes, which are usually defined and then
-not referenced, but contain constructors and destructors that have
-nontrivial bookkeeping functions.
-
 @item deprecated
 @itemx deprecated (@var{msg})
 @cindex @code{deprecated} type attribute
@@ -6205,6 +6158,42 @@ of the structure or union is placed to m
 attached to an @code{enum} definition, it indicates that the smallest
 integral type should be used.
 
+@item scalar_storage_order ("@var{endianness}")
+@cindex @code{scalar_storage_order} type attribute
+When attached to a @code{union} or a @code{struct}, this attribute sets
+the storage order, aka endianness, of the scalar fields of the type, as
+well as the array fields whose component is scalar.  The supported
+endianness are @code{big-endian} and @code{little-endian}.  The attribute
+has no effects on fields which are themselves a @code{union}, a @code{struct}
+or an array whose component is a @code{union} or a @code{struct}, and it is
+possible to have fields with a different scalar storage order than the
+enclosing type.
+
+This attribute is supported only for targets that use a uniform default
+scalar storage order (fortunately, most of them), i.e. targets that store
+the scalars either all in big-endian or all in little-endian.
+
+Additional restrictions are enforced for types with the reverse scalar
+storage order with regard to the scalar storage order of the target:
+
+@itemize
+@item Taking the address of a scalar field of a @code{union} or a
+@code{struct} with reverse scalar storage order is not permitted and will
+yield an error
+@item Taking the address of an array field, whose component is scalar, of
+a @code{union} or a @code{struct} with reverse scalar storage order is
+permitted but will yield a warning, unless @option{-Wno-scalar-storage-order}
+is specified
+@item Taking the address of a @code{union} or a @code{struct} with reverse
+scalar storage order is permitted
+@end itemize
+
+These restrictions exist because the storage order attribute is lost when
+the address of a scalar or the address of an array with scalar component
+is taken, so storing indirectly through this address will generally not work.
+The second case is nevertheless allowed to be able to perform a block copy
+from or to the array.
+
 @item transparent_union
 @cindex @code{transparent_union} type attribute
 
Index: c-family/c-common.c
===================================================================
--- c-family/c-common.c	(revision 224467)
+++ c-family/c-common.c	(working copy)
@@ -690,10 +690,10 @@ const struct attribute_spec c_common_att
   /* The same comments as for noreturn attributes apply to const ones.  */
   { "const",                  0, 0, true,  false, false,
 			      handle_const_attribute, false },
-  { "transparent_union",      0, 0, false, false, false,
-			      handle_transparent_union_attribute, false },
   { "scalar_storage_order",   1, 1, false, false, false,
 			      handle_scalar_storage_order_attribute, false },
+  { "transparent_union",      0, 0, false, false, false,
+			      handle_transparent_union_attribute, false },
   { "constructor",            0, 1, true,  false, false,
 			      handle_constructor_attribute, false },
   { "destructor",             0, 1, true,  false, false,
@@ -7494,6 +7494,55 @@ handle_const_attribute (tree *node, tree
   return NULL_TREE;
 }
 
+/* Handle a "scalar_storage_order" attribute; arguments as in
+   struct attribute_spec.handler.  */
+
+static tree
+handle_scalar_storage_order_attribute (tree *node, tree name, tree args,
+				       int flags, bool *no_add_attrs)
+{
+  tree id = TREE_VALUE (args);
+  tree type;
+
+  *no_add_attrs = true;
+
+  if (TREE_CODE (*node) == TYPE_DECL
+      && ! (flags & ATTR_FLAG_CXX11))
+    node = &TREE_TYPE (*node);
+  type = *node;
+
+  if (BYTES_BIG_ENDIAN != WORDS_BIG_ENDIAN)
+    error ("scalar_storage_order is not supported");
+
+  if (RECORD_OR_UNION_TYPE_P (type))
+    {
+      if (!(flags & (int) ATTR_FLAG_TYPE_IN_PLACE))
+	goto ignored;
+
+      if (TREE_CODE (id) == STRING_CST
+	  && strcmp (TREE_STRING_POINTER (id), "big-endian") == 0)
+	{
+	  if (!BYTES_BIG_ENDIAN)
+	    TYPE_REVERSE_STORAGE_ORDER (type) = 1;
+	}
+      else if (TREE_CODE (id) == STRING_CST
+	       && strcmp (TREE_STRING_POINTER (id), "little-endian") == 0)
+	{
+	  if (BYTES_BIG_ENDIAN)
+	    TYPE_REVERSE_STORAGE_ORDER (type) = 1;
+	}
+      else
+	error ("scalar_storage_order argument must be one of \"big-endian\" "
+	       "or \"little-endian\"");
+
+      return NULL_TREE;
+    }
+
+ ignored:
+  warning (OPT_Wattributes, "%qE attribute ignored", name);
+  return NULL_TREE;
+}
+
 /* Handle a "transparent_union" attribute; arguments as in
    struct attribute_spec.handler.  */
 
@@ -7545,55 +7594,6 @@ handle_transparent_union_attribute (tree
       return NULL_TREE;
     }
 
- ignored:
-  warning (OPT_Wattributes, "%qE attribute ignored", name);
-  return NULL_TREE;
-}
-
-/* Handle a "scalar_storage_order" attribute; arguments as in
-   struct attribute_spec.handler.  */
-
-static tree
-handle_scalar_storage_order_attribute (tree *node, tree name, tree args,
-				       int flags, bool *no_add_attrs)
-{
-  tree id = TREE_VALUE (args);
-  tree type;
-
-  *no_add_attrs = true;
-
-  if (TREE_CODE (*node) == TYPE_DECL
-      && ! (flags & ATTR_FLAG_CXX11))
-    node = &TREE_TYPE (*node);
-  type = *node;
-
-  if (BYTES_BIG_ENDIAN != WORDS_BIG_ENDIAN)
-    error ("scalar_storage_order is not supported");
-
-  if (RECORD_OR_UNION_TYPE_P (type))
-    {
-      if (!(flags & (int) ATTR_FLAG_TYPE_IN_PLACE))
-	goto ignored;
-
-      if (TREE_CODE (id) == STRING_CST
-	  && strcmp (TREE_STRING_POINTER (id), "big-endian") == 0)
-	{
-	  if (!BYTES_BIG_ENDIAN)
-	    TYPE_REVERSE_STORAGE_ORDER (type) = 1;
-	}
-      else if (TREE_CODE (id) == STRING_CST
-	       && strcmp (TREE_STRING_POINTER (id), "little-endian") == 0)
-	{
-	  if (BYTES_BIG_ENDIAN)
-	    TYPE_REVERSE_STORAGE_ORDER (type) = 1;
-	}
-      else
-	error ("scalar_storage_order argument must be one of \"big-endian\" "
-	       "or \"little-endian\"");
-
-      return NULL_TREE;
-    }
-
  ignored:
   warning (OPT_Wattributes, "%qE attribute ignored", name);
   return NULL_TREE;
Index: c/c-decl.c
===================================================================
--- c/c-decl.c	(revision 224467)
+++ c/c-decl.c	(working copy)
@@ -7733,7 +7733,7 @@ finish_struct (location_t loc, tree t, t
     error ("type %qT is too large", t);
 
   /* Give bit-fields their proper types and rewrite the type of array fields
-     if the enclosing type has reverse storage order.  */
+     with scalar component if the enclosing type has reverse storage order.  */
   for (tree field = fieldlist; field; field = DECL_CHAIN (field))
     {
       if (TREE_CODE (field) == FIELD_DECL
@@ -7756,17 +7756,17 @@ finish_struct (location_t loc, tree t, t
 	       && TREE_CODE (TREE_TYPE (field)) == ARRAY_TYPE)
 	{
 	  tree ftype = TREE_TYPE (field);
-	  do
-	    ftype = TREE_TYPE (ftype);
-	  while (TREE_CODE (ftype) == ARRAY_TYPE);
-	  if (!RECORD_OR_UNION_TYPE_P (ftype))
+	  if (!RECORD_OR_UNION_TYPE_P (strip_array_types (ftype)))
 	    {
-	      tree *ftypep = &TREE_TYPE (field);
+	      tree fmain_type = TYPE_MAIN_VARIANT (ftype);
+	      tree *typep = &fmain_type;
 	      do {
-		*ftypep = build_distinct_type_copy (*ftypep);
-		TYPE_REVERSE_STORAGE_ORDER (*ftypep) = 1;
-		ftypep = &TREE_TYPE (*ftypep);
-	      } while (TREE_CODE (*ftypep) == ARRAY_TYPE);
+		*typep = build_distinct_type_copy (*typep);
+		TYPE_REVERSE_STORAGE_ORDER (*typep) = 1;
+		typep = &TREE_TYPE (*typep);
+	      } while (TREE_CODE (*typep) == ARRAY_TYPE);
+	      TREE_TYPE (field)
+		= c_build_qualified_type (fmain_type, TYPE_QUALS (ftype));
 	    }
 	}
     }
Index: testsuite/c-c++-common/sso-2.c
===================================================================
--- testsuite/c-c++-common/sso-2.c	(revision 0)
+++ testsuite/c-c++-common/sso-2.c	(revision 0)
@@ -0,0 +1,94 @@
+/* Test support of scalar_storage_order attribute */
+
+/* { dg-do compile } */
+
+struct S1
+{
+  int i;
+} __attribute__((scalar_storage_order("big-endian")));
+
+struct S2
+{
+  int i;
+} __attribute__((scalar_storage_order("little-endian")));
+
+struct S3 { int i; } __attribute__((scalar_storage_order("other"))); /* { dg-error "must be one of .big-endian. or .little-endian." } */
+
+void incompatible_assign (struct S1 *s1, struct S2 *s2)
+{
+  *s1 = *s2; /* { dg-error "(incompatible types|no match)" } */
+}
+
+int *addr1 (int which, struct S1 *s1, struct S2 *s2)
+{
+  return (which == 1 ? &s1->i : &s2->i); /* { dg-error "address of scalar with reverse storage order" } */
+}
+
+struct S4
+{
+  int a[4];
+  struct S2 s2;
+} __attribute__((scalar_storage_order("big-endian")));
+
+struct S5
+{
+  int a[4];
+  struct S1 s1;
+} __attribute__((scalar_storage_order("little-endian")));
+
+void *addr2 (int which, struct S4 *s4, struct S5 *s5)
+{
+  return (which == 1 ? (void *)s4->a : (void *)s5->a); /* { dg-warning "address of array with reverse scalar storage order" } */
+}
+
+void *addr3 (int which, struct S4 *s4, struct S5 *s5)
+{
+  return (which == 1 ? (void *)&s4->a : (void *)&s5->a); /* { dg-warning "address of array with reverse scalar storage order" } */
+}
+
+void *addr4 (int which, struct S4 *s4, struct S5 *s5)
+{
+  return (which == 1 ? (void *)&s4->a[0] : (void *)&s5->a[0]); /* { dg-error "address of scalar with reverse storage order" } */
+}
+
+void *addr5 (int which, struct S4 *s4, struct S5 *s5)
+{
+  return (which == 1 ? (void *)&s4->s2 : (void *) &s5->s1); /* ok */
+}
+
+struct S6
+{
+  int a[4][2];
+  struct S2 s2[2];
+}  __attribute__((scalar_storage_order("big-endian")));
+
+struct S7
+{
+  int a[4][2];
+  struct S1 s1[2];
+}  __attribute__((scalar_storage_order("little-endian")));
+
+void *addr6 (int which, struct S6 *s6, struct S7 *s7)
+{
+  return (which == 1 ? (void *)s6->a : (void *)s7->a); /* { dg-warning "address of array with reverse scalar storage order" } */
+}
+
+void *addr7 (int which, struct S6 *s6, struct S7 *s7)
+{
+  return (which == 1 ? (void *)&s6->a : (void *)&s7->a); /* { dg-warning "address of array with reverse scalar storage order" } */
+}
+
+void *addr8 (int which, struct S6 *s6, struct S7 *s7)
+{
+  return (which == 1 ? (void *)&s6->a[0] : (void *)&s7->a[0]); /* { dg-warning "address of array with reverse scalar storage order" } */
+}
+
+void *addr9 (int which, struct S6 *s6, struct S7 *s7)
+{
+  return (which == 1 ? (void *)&s6->a[0][0] : (void *)&s7->a[0][0]); /* { dg-error "address of scalar with reverse storage order" } */
+}
+
+void *addr10 (int which, struct S6 *s6, struct S7 *s7)
+{
+  return (which == 1 ? (void *)&s6->s2 : (void *)&s7->s1); /* ok */
+}
Index: testsuite/c-c++-common/sso-3.c
===================================================================
--- testsuite/c-c++-common/sso-3.c	(revision 0)
+++ testsuite/c-c++-common/sso-3.c	(revision 0)
@@ -0,0 +1,21 @@
+/* Test support of scalar_storage_order attribute */
+
+/* { dg-do compile } */
+
+struct S1
+{
+  int i;
+};
+
+typedef struct S1 __attribute__((scalar_storage_order("big-endian"))) NS1; /* { dg-warning "attribute ignored" } */
+
+struct S2
+{
+  int i;
+};
+
+typedef struct S2 __attribute__((scalar_storage_order("little-endian"))) NS2; /* { dg-warning "attribute ignored" } */
+
+struct S3 { int i; };
+
+typedef struct S3 __attribute__((scalar_storage_order("other"))) NS3; /* { dg-warning "attribute ignored" } */
Index: testsuite/c-c++-common/sso-4.c
===================================================================
--- testsuite/c-c++-common/sso-4.c	(revision 0)
+++ testsuite/c-c++-common/sso-4.c	(revision 0)
@@ -0,0 +1,24 @@
+/* Test support of scalar_storage_order attribute */
+
+/* { dg-do compile } */
+
+struct S3
+{
+  struct __attribute__((scalar_storage_order("big-endian"))) S1
+  {
+    int i;
+  } s1;
+};
+
+struct S4
+{
+  struct __attribute__((scalar_storage_order("little-endian"))) S2
+  {
+    int i;
+  } s2;
+};
+
+void incompatible_assign (struct S3 *s3, struct S4 *s4)
+{
+  s3->s1 = s4->s2; /* { dg-error "(incompatible types|no match)" } */
+}
Index: testsuite/c-c++-common/sso-1.c
===================================================================
--- testsuite/c-c++-common/sso-1.c	(revision 224463)
+++ testsuite/c-c++-common/sso-1.c	(working copy)
@@ -12,7 +12,7 @@ struct __attribute__((scalar_storage_ord
   int i;
 };
 
-struct __attribute__((scalar_storage_order("other"))) S3 {}; /* { dg-error "must be one of .big-endian. or .little-endian." } */
+struct __attribute__((scalar_storage_order("other"))) S3 { int i; }; /* { dg-error "must be one of .big-endian. or .little-endian." } */
 
 void incompatible_assign (struct S1 *s1, struct S2 *s2)
 {
Index: cp/class.c
===================================================================
--- cp/class.c	(revision 224467)
+++ cp/class.c	(working copy)
@@ -6658,7 +6658,8 @@ finish_struct_1 (tree t)
   set_method_tm_attributes (t);
 
   /* Complete the rtl for any static member objects of the type we're
-     working on.  */
+     working on and rewrite the type of array fields with scalar
+     component if the enclosing type has reverse storage order.  */
   for (x = TYPE_FIELDS (t); x; x = DECL_CHAIN (x))
     if (VAR_P (x) && TREE_STATIC (x)
         && TREE_TYPE (x) != error_mark_node
@@ -6669,17 +6670,17 @@ finish_struct_1 (tree t)
 	     && TREE_CODE (TREE_TYPE (x)) == ARRAY_TYPE)
       {
 	tree ftype = TREE_TYPE (x);
-	do
-	  ftype = TREE_TYPE (ftype);
-	while (TREE_CODE (ftype) == ARRAY_TYPE);
-	if (!RECORD_OR_UNION_TYPE_P (ftype))
+	if (!RECORD_OR_UNION_TYPE_P (strip_array_types (ftype)))
 	  {
-	    tree *ftypep = &TREE_TYPE (x);
+	    tree fmain_type = TYPE_MAIN_VARIANT (ftype);
+	    tree *typep = &fmain_type;
 	    do {
-	      *ftypep = build_distinct_type_copy (*ftypep);
-	      TYPE_REVERSE_STORAGE_ORDER (*ftypep) = 1;
-	      ftypep = &TREE_TYPE (*ftypep);
-	    } while (TREE_CODE (*ftypep) == ARRAY_TYPE);
+	      *typep = build_distinct_type_copy (*typep);
+	      TYPE_REVERSE_STORAGE_ORDER (*typep) = 1;
+	      typep = &TREE_TYPE (*typep);
+	    } while (TREE_CODE (*typep) == ARRAY_TYPE);
+	    TREE_TYPE (x)
+	      = cp_build_qualified_type (fmain_type, TYPE_QUALS (ftype));
 	  }
       }
 

Re: [scalar-storage-order] Small tweaks

[Joseph Myers]

On Fri, 19 Jun 2015, Eric Botcazou wrote:

> This adjusts the type adjustment code for array components in the C and C++ 
> front-ends as per Joseph's remark, fixes a couple of documentation glitches 
> and adds 3 more testcases following Richard's questions.
> 
> Joseph, is that what you had in mind?

Yes, something like that.

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: [scalar-storage-order] Small tweaks

[Eric Botcazou]

> Yes, something like that.

Thanks, installed on the branch.

-- 
Eric Botcazou