c: Add support for byte arrays in C2Y

Message ID 69cc6863d25717de71f4d95451f16bb3d769f983.camel@tugraz.at
State New
Headers
Series c: Add support for byte arrays in C2Y |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_gcc_build--master-arm success Build passed
linaro-tcwg-bot/tcwg_gcc_build--master-aarch64 success Build passed
linaro-tcwg-bot/tcwg_gcc_check--master-arm success Test passed
linaro-tcwg-bot/tcwg_gcc_check--master-aarch64 success Test passed

Commit Message

Martin Uecker June 29, 2024, 12:29 p.m. UTC
  This marks structures which include a byte array
as typeless storage.


Bootstrapped and regression tested on x86_64.



    c: Add support for byte arrays in C2Y
    
    To get correct aliasing behavior requires that structures and unions
    that contain a byte array, i.e. an array of non-atomic character
    type (N3254), are marked with TYPE_TYPELESS_STORAGE.
    
    gcc/c/
            * c-decl.cc (grokdeclarator, finish_struct): Set and
            propagate TYPE_TYPELESS_STORAGE.
    
    gcc/testsuite/
            * gcc.dg/c2y-byte-alias-1.c: New test.
            * gcc.dg/c2y-byte-alias-2.c: New test.
            * gcc.dg/c2y-byte-alias-3.c: New test.
  

Comments

Joseph Myers July 9, 2024, 5:28 p.m. UTC | #1
On Sat, 29 Jun 2024, Martin Uecker wrote:

> +		bool typeless = flag_isoc2y
> +				&& ((char_type_p (type)
> +				     && !(type_quals & TYPE_QUAL_ATOMIC))
> +				    || (AGGREGATE_TYPE_P (type)
> +					&& TYPE_TYPELESS_STORAGE (type)));

I'm not convinced this should be limited to C2Y mode, rather than applied 
also in older modes as an extension.  How likely is it that users of older 
modes would thereby see a performance regression in code that already 
worked reliably (as opposed to having code start to work more reliably 
because they were assuming things about aliasing that weren't actually 
guaranteed)?
  
Martin Uecker July 9, 2024, 5:44 p.m. UTC | #2
Am Dienstag, dem 09.07.2024 um 17:28 +0000 schrieb Joseph Myers:
> On Sat, 29 Jun 2024, Martin Uecker wrote:
> 
> > +		bool typeless = flag_isoc2y
> > +				&& ((char_type_p (type)
> > +				     && !(type_quals & TYPE_QUAL_ATOMIC))
> > +				    || (AGGREGATE_TYPE_P (type)
> > +					&& TYPE_TYPELESS_STORAGE (type)));
> 
> I'm not convinced this should be limited to C2Y mode, rather than applied 
> also in older modes as an extension.  How likely is it that users of older 
> modes would thereby see a performance regression in code that already 
> worked reliably (as opposed to having code start to work more reliably 
> because they were assuming things about aliasing that weren't actually 
> guaranteed)?

I agree that it is better to activate it in earlier modes too.
I believe both C++ and clang and for both C and C++ have this,
so the risk of making false assumptions is very high.

I do not have any benchmarks though (except my own projects
where it does not matter).  Should we simply try to activate
it and see what happens?

Martin


>
  

Patch

diff --git a/gcc/c/c-decl.cc b/gcc/c/c-decl.cc
index 0eac266471f..65561f3cbcc 100644
--- a/gcc/c/c-decl.cc
+++ b/gcc/c/c-decl.cc
@@ -7499,12 +7499,17 @@  grokdeclarator (const struct c_declarator *declarator,
 	       modify the shared type, so we gcc_assert (itype)
 	       below.  */
 	      {
+		bool typeless = flag_isoc2y
+				&& ((char_type_p (type)
+				     && !(type_quals & TYPE_QUAL_ATOMIC))
+				    || (AGGREGATE_TYPE_P (type)
+					&& TYPE_TYPELESS_STORAGE (type)));
+
 		addr_space_t as = DECODE_QUAL_ADDR_SPACE (type_quals);
 		if (!ADDR_SPACE_GENERIC_P (as) && as != TYPE_ADDR_SPACE (type))
 		  type = build_qualified_type (type,
 					       ENCODE_QUAL_ADDR_SPACE (as));
-
-		type = build_array_type (type, itype);
+		type = build_array_type (type, itype, typeless);
 	      }
 
 	    if (type != error_mark_node)
@@ -9656,6 +9661,10 @@  finish_struct (location_t loc, tree t, tree fieldlist, tree attributes,
       if (DECL_NAME (x)
 	  || RECORD_OR_UNION_TYPE_P (TREE_TYPE (x)))
 	saw_named_field = true;
+
+      if (AGGREGATE_TYPE_P (TREE_TYPE (x))
+	  && TYPE_TYPELESS_STORAGE (TREE_TYPE (x)))
+	TYPE_TYPELESS_STORAGE (t) = true;
     }
 
   detect_field_duplicates (fieldlist);
@@ -9856,6 +9865,7 @@  finish_struct (location_t loc, tree t, tree fieldlist, tree attributes,
       TYPE_FIELDS (x) = TYPE_FIELDS (t);
       TYPE_LANG_SPECIFIC (x) = TYPE_LANG_SPECIFIC (t);
       TYPE_TRANSPARENT_AGGR (x) = TYPE_TRANSPARENT_AGGR (t);
+      TYPE_TYPELESS_STORAGE (x) = TYPE_TYPELESS_STORAGE (t);
       C_TYPE_FIELDS_READONLY (x) = C_TYPE_FIELDS_READONLY (t);
       C_TYPE_FIELDS_VOLATILE (x) = C_TYPE_FIELDS_VOLATILE (t);
       C_TYPE_FIELDS_NON_CONSTEXPR (x) = C_TYPE_FIELDS_NON_CONSTEXPR (t);
diff --git a/gcc/testsuite/gcc.dg/c2y-byte-alias-1.c b/gcc/testsuite/gcc.dg/c2y-byte-alias-1.c
new file mode 100644
index 00000000000..30bc2c09c2f
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/c2y-byte-alias-1.c
@@ -0,0 +1,46 @@ 
+/* { dg-do run } */
+/* { dg-options "-std=c2y -O2" } */
+
+struct f { _Alignas(int) char buf[sizeof(int)]; };
+struct f2 { struct f x; };
+union g { _Alignas(int) char buf[sizeof(int)]; };
+
+[[gnu::noinline]]
+int foo(struct f *p, int *q)
+{
+	*q = 1;
+	*p = (struct f){ };
+	return *q;
+}
+
+[[gnu::noinline]]
+int foo2(struct f2 *p, int *q)
+{
+	*q = 1;
+	*p = (struct f2){ };
+	return *q;
+}
+
+[[gnu::noinline]]
+int bar(union g *p, int *q)
+{
+	*q = 1;
+	*p = (union g){ };
+	return *q;
+}
+
+
+int main()
+{
+	struct f p;
+	if (0 != foo(&p, (void*)&p.buf))
+		__builtin_abort();
+
+	struct f2 p2;
+	if (0 != foo2(&p2, (void*)&p2.x.buf))
+		__builtin_abort();
+
+	union g q;
+	if (0 != bar(&q, (void*)&q.buf))
+		__builtin_abort();
+}
diff --git a/gcc/testsuite/gcc.dg/c2y-byte-alias-2.c b/gcc/testsuite/gcc.dg/c2y-byte-alias-2.c
new file mode 100644
index 00000000000..9bd2d18b386
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/c2y-byte-alias-2.c
@@ -0,0 +1,43 @@ 
+/* { dg-do run } */
+/* { dg-options "-std=c2y -O2" } */
+
+struct f2 {
+	struct f {
+		_Alignas(int) char buf[sizeof(int)];
+	} x[2];
+	int i;
+};
+
+[[gnu::noinline]]
+int foo2(struct f2 *p, int *q)
+{
+	*q = 1;
+	*p = (struct f2){ };
+	return *q;
+}
+
+struct g2 {
+	union g {
+		_Alignas(int) char buf[sizeof(int)];
+	} x[2];
+	int i;
+};
+
+[[gnu::noinline]]
+int bar2(struct g2 *p, int *q)
+{
+	*q = 1;
+	*p = (struct g2){ };
+	return *q;
+}
+
+int main()
+{
+	struct f2 p2;
+	if (0 != foo2(&p2, (void*)&p2.x[0].buf))
+		__builtin_abort();
+
+	struct g2 q2;
+	if (0 != bar2(&q2, (void*)&q2.x[0].buf))
+		__builtin_abort();
+}
diff --git a/gcc/testsuite/gcc.dg/c2y-byte-alias-3.c b/gcc/testsuite/gcc.dg/c2y-byte-alias-3.c
new file mode 100644
index 00000000000..f88eab2e92f
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/c2y-byte-alias-3.c
@@ -0,0 +1,47 @@ 
+/* { dg-do run } */
+/* { dg-options "-std=c2y -O2" } */
+
+struct f { _Alignas(int) typeof(volatile char) buf[sizeof(int)]; };
+struct f2 { struct f x; };
+union g { _Alignas(int) volatile char buf[sizeof(int)]; };
+
+
+[[gnu::noinline]]
+int foo(struct f *p, int *q)
+{
+	*q = 1;
+	*p = (struct f){ };
+	return *q;
+}
+
+[[gnu::noinline]]
+int foo2(struct f2 *p, int *q)
+{
+	*q = 1;
+	*p = (struct f2){ };
+	return *q;
+}
+
+[[gnu::noinline]]
+int bar(union g *p, int *q)
+{
+	*q = 1;
+	*p = (union g){ };
+	return *q;
+}
+
+
+int main()
+{
+	struct f p;
+	if (0 != foo(&p, (void*)&p.buf))
+		__builtin_abort();
+
+	struct f2 p2;
+	if (0 != foo2(&p2, (void*)&p2.x.buf))
+		__builtin_abort();
+
+	union g q;
+	if (0 != bar(&q, (void*)&q.buf))
+		__builtin_abort();
+}