Patchwork Allow union variables to share stack slots wwith -fno-strict-aliasing (issue4444051)

login
register
mail settings
Submitter Easwaran Raman
Date April 20, 2011, 12:42 a.m.
Message ID <20110420004211.101FA220625@agni2.mtv.corp.google.com>
Download mbox | patch
Permalink /patch/92071/
State New
Headers show

Comments

Easwaran Raman - April 20, 2011, 12:42 a.m.
Hi,
 This patch allows variables whose type  transitively contains a union to share stack slots if -fno-strict-aliasing is used.

 Bootstraps on x86_64 with no test regressions. Also tested by changing flag_strict_aliasing to 0 by default. Bootstraps and no test regressions when compared to flag_strict_aliasing=0 without this patch.

OK for trunk?

-Easwaran

2011-04-19  Easwaran Raman  <eraman@google.com>

	* gcc/testsuite/gcc.dg/stack-layout-1.c: New
	* gcc/cfgexpand.c (add_alias_set_conflicts): Add conflicts
	with a variable containing union type only with
	-fstrict-aliasing.


--
This patch is available for review at http://codereview.appspot.com/4444051
Richard Guenther - April 20, 2011, 9:07 a.m.
On Wed, Apr 20, 2011 at 2:42 AM, Easwaran Raman <eraman@google.com> wrote:
> Hi,
>  This patch allows variables whose type  transitively contains a union to share stack slots if -fno-strict-aliasing is used.
>
>  Bootstraps on x86_64 with no test regressions. Also tested by changing flag_strict_aliasing to 0 by default. Bootstraps and no test regressions when compared to flag_strict_aliasing=0 without this patch.
>
> OK for trunk?

Ok.

Thanks,
Richard.

> -Easwaran
>
> 2011-04-19  Easwaran Raman  <eraman@google.com>
>
>        * gcc/testsuite/gcc.dg/stack-layout-1.c: New
>        * gcc/cfgexpand.c (add_alias_set_conflicts): Add conflicts
>        with a variable containing union type only with
>        -fstrict-aliasing.
>
> Index: gcc/testsuite/gcc.dg/stack-layout-1.c
> ===================================================================
> --- gcc/testsuite/gcc.dg/stack-layout-1.c       (revision 0)
> +++ gcc/testsuite/gcc.dg/stack-layout-1.c       (revision 0)
> @@ -0,0 +1,25 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -fno-strict-aliasing -fdump-rtl-expand" } */
> +union U {
> +  int a;
> +  float b;
> +};
> +struct A {
> +  union U u1;
> +  char a[100];
> +};
> +void bar (struct A *);
> +void foo ()
> +  {
> +    {
> +      struct A a;
> +      bar (&a);
> +    }
> +    {
> +      struct A a;
> +      bar (&a);
> +    }
> +  }
> +
> +/* { dg-final { scan-rtl-dump-times "Partition" 1 "expand" } } */
> +/* { dg-final { cleanup-rtl-dump "expand" } } */
> Index: gcc/cfgexpand.c
> ===================================================================
> --- gcc/cfgexpand.c     (revision 172657)
> +++ gcc/cfgexpand.c     (working copy)
> @@ -373,8 +373,9 @@ add_alias_set_conflicts (void)
>                 to elements will conflict.  In case of unions we have
>                 to be careful as type based aliasing rules may say
>                 access to the same memory does not conflict.  So play
> -                safe and add a conflict in this case.  */
> -             || contains_union)
> +                safe and add a conflict in this case when
> +                 -fstrict-aliasing is used.  */
> +              || (contains_union && flag_strict_aliasing))
>            add_stack_var_conflict (i, j);
>        }
>     }
>
> --
> This patch is available for review at http://codereview.appspot.com/4444051
>
Eric Botcazou - April 20, 2011, 9:12 a.m.
> 2011-04-19  Easwaran Raman  <eraman@google.com>
>
> 	* gcc/testsuite/gcc.dg/stack-layout-1.c: New
> 	* gcc/cfgexpand.c (add_alias_set_conflicts): Add conflicts
> 	with a variable containing union type only with
> 	-fstrict-aliasing.

You need an entry for each relevant ChangeLog, without prefixes:


2011-04-20  Easwaran Raman  <eraman@google.com>

	* cfgexpand.c (add_alias_set_conflicts): Add conflicts with a variable
	containing union type only with -fstrict-aliasing.


2011-04-20  Easwaran Raman  <eraman@google.com>

	* gcc.dg/stack-layout-1.c: New test.
Easwaran Raman - April 20, 2011, 6:36 p.m.
On Wed, Apr 20, 2011 at 2:12 AM, Eric Botcazou <ebotcazou@adacore.com> wrote:
>> 2011-04-19  Easwaran Raman  <eraman@google.com>
>>
>>       * gcc/testsuite/gcc.dg/stack-layout-1.c: New
>>       * gcc/cfgexpand.c (add_alias_set_conflicts): Add conflicts
>>       with a variable containing union type only with
>>       -fstrict-aliasing.
>
> You need an entry for each relevant ChangeLog, without prefixes:
>
>
> 2011-04-20  Easwaran Raman  <eraman@google.com>
>
>        * cfgexpand.c (add_alias_set_conflicts): Add conflicts with a variable
>        containing union type only with -fstrict-aliasing.
>
>
> 2011-04-20  Easwaran Raman  <eraman@google.com>
>
>        * gcc.dg/stack-layout-1.c: New test.
>
>
> --
> Eric Botcazou
>

Thanks. I've added them to the respective Changelog files and
committed the patch.

-Easwaran

Patch

Index: gcc/testsuite/gcc.dg/stack-layout-1.c
===================================================================
--- gcc/testsuite/gcc.dg/stack-layout-1.c	(revision 0)
+++ gcc/testsuite/gcc.dg/stack-layout-1.c	(revision 0)
@@ -0,0 +1,25 @@ 
+/* { dg-do compile } */
+/* { dg-options "-O2 -fno-strict-aliasing -fdump-rtl-expand" } */
+union U {
+  int a;
+  float b;
+};
+struct A {
+  union U u1;
+  char a[100];
+};
+void bar (struct A *);
+void foo ()
+  {
+    {
+      struct A a;
+      bar (&a);
+    }
+    {
+      struct A a;
+      bar (&a);
+    }
+  }
+
+/* { dg-final { scan-rtl-dump-times "Partition" 1 "expand" } } */
+/* { dg-final { cleanup-rtl-dump "expand" } } */
Index: gcc/cfgexpand.c
===================================================================
--- gcc/cfgexpand.c	(revision 172657)
+++ gcc/cfgexpand.c	(working copy)
@@ -373,8 +373,9 @@  add_alias_set_conflicts (void)
 		 to elements will conflict.  In case of unions we have
 		 to be careful as type based aliasing rules may say
 		 access to the same memory does not conflict.  So play
-		 safe and add a conflict in this case.  */
-	      || contains_union)
+		 safe and add a conflict in this case when
+                 -fstrict-aliasing is used.  */
+              || (contains_union && flag_strict_aliasing))
 	    add_stack_var_conflict (i, j);
 	}
     }