[v2] avoid alignment of static variables affecting stack's

Function (or more narrow) scope static variables (as well as others not
placed on the stack) should also not have any effect on the stack
alignment. I noticed the issue first with Linux'es dynamic_pr_debug()
construct using an 8-byte aligned sub-file-scope local variable.

According to my checking bad behavior started with 4.6.x (4.5.3 was
still okay), but generated code got quite a bit worse as of 4.9.0.

[v2: Drop inclusion of hard register variables, as requested by
     Jakub and Richard.]

gcc/
2014-10-24  Jan Beulich  <jbeulich@suse.com>

	* cfgexpand.c (expand_one_var): Exclude static and external
	variables when adjusting stack alignment related state.

gcc/testsuite/
2014-10-24  Jan Beulich  <jbeulich@suse.com>

	* gcc.c-torture/execute/stkalign.c: New.
avoid alignment of static variables affecting stack's

Function (or more narrow) scope static variables (as well as others not
placed on the stack) should also not have any effect on the stack
alignment. I noticed the issue first with Linux'es dynamic_pr_debug()
construct using an 8-byte aligned sub-file-scope local variable.

According to my checking bad behavior started with 4.6.x (4.5.3 was
still okay), but generated code got quite a bit worse as of 4.9.0.

[v2: Drop inclusion of hard register variables, as requested by
     Jakub and Richard.]

gcc/
2014-10-24  Jan Beulich  <jbeulich@suse.com>

	* cfgexpand.c (expand_one_var): Exclude static and external
	variables when adjusting stack alignment related state.

gcc/testsuite/
2014-10-24  Jan Beulich  <jbeulich@suse.com>

	* gcc.c-torture/execute/stkalign.c: New.

--- a/gcc/cfgexpand.c
+++ b/gcc/cfgexpand.c
@@ -1233,12 +1233,15 @@ static HOST_WIDE_INT
 expand_one_var (tree var, bool toplevel, bool really_expand)
 {
   unsigned int align = BITS_PER_UNIT;
+  bool stack = true;
   tree origvar = var;
 
   var = SSAVAR (var);
 
   if (TREE_TYPE (var) != error_mark_node && TREE_CODE (var) == VAR_DECL)
     {
+      stack = !TREE_STATIC (var) && !DECL_EXTERNAL (var);
+
       /* Because we don't know if VAR will be in register or on stack,
 	 we conservatively assume it will be on stack even if VAR is
 	 eventually put into register after RA pass.  For non-automatic
@@ -1267,22 +1270,25 @@ expand_one_var (tree var, bool toplevel,
 	align = POINTER_SIZE;
     }
 
-  if (SUPPORTS_STACK_ALIGNMENT
-      && crtl->stack_alignment_estimated < align)
+  if (stack)
     {
-      /* stack_alignment_estimated shouldn't change after stack
-         realign decision made */
-      gcc_assert (!crtl->stack_realign_processed);
-      crtl->stack_alignment_estimated = align;
+      if (SUPPORTS_STACK_ALIGNMENT
+	  && crtl->stack_alignment_estimated < align)
+	{
+	  /* stack_alignment_estimated shouldn't change after stack
+	     realign decision made */
+	  gcc_assert (!crtl->stack_realign_processed);
+	  crtl->stack_alignment_estimated = align;
+	}
+
+      /* stack_alignment_needed > PREFERRED_STACK_BOUNDARY is permitted.
+	 So here we only make sure stack_alignment_needed >= align.  */
+      if (crtl->stack_alignment_needed < align)
+	crtl->stack_alignment_needed = align;
+      if (crtl->max_used_stack_slot_alignment < align)
+	crtl->max_used_stack_slot_alignment = align;
     }
 
-  /* stack_alignment_needed > PREFERRED_STACK_BOUNDARY is permitted.
-     So here we only make sure stack_alignment_needed >= align.  */
-  if (crtl->stack_alignment_needed < align)
-    crtl->stack_alignment_needed = align;
-  if (crtl->max_used_stack_slot_alignment < align)
-    crtl->max_used_stack_slot_alignment = align;
-
   if (TREE_CODE (origvar) == SSA_NAME)
     {
       gcc_assert (TREE_CODE (var) != VAR_DECL
--- a/gcc/testsuite/gcc.c-torture/execute/stkalign.c
+++ b/gcc/testsuite/gcc.c-torture/execute/stkalign.c
@@ -0,0 +1,26 @@
+/* { dg-options "-fno-inline" } */
+
+#include <assert.h>
+
+#define ALIGNMENT 64
+
+unsigned test(unsigned n, unsigned p)
+{
+  static struct { char __attribute__((__aligned__(ALIGNMENT))) c; } s;
+  unsigned x;
+
+  assert(__alignof__(s) == ALIGNMENT);
+  asm ("" : "=g" (x), "+m" (s) : "0" (&x));
+
+  return n ? test(n - 1, x) : (x ^ p);
+}
+
+int main (int argc, char *argv[] __attribute__((unused)))
+{
+  unsigned int x = test(argc, 0);
+
+  x |= test(argc + 1, 0);
+  x |= test(argc + 2, 0);
+
+  return !(x & (ALIGNMENT - 1));
+}

On Fri, Oct 24, 2014 at 4:57 PM, Jan Beulich <JBeulich@suse.com> wrote:
> Function (or more narrow) scope static variables (as well as others not
> placed on the stack) should also not have any effect on the stack
> alignment. I noticed the issue first with Linux'es dynamic_pr_debug()
> construct using an 8-byte aligned sub-file-scope local variable.
>
> According to my checking bad behavior started with 4.6.x (4.5.3 was
> still okay), but generated code got quite a bit worse as of 4.9.0.
>
> [v2: Drop inclusion of hard register variables, as requested by
>      Jakub and Richard.]

I wonder if the spilling argument also holds true for those though,
as probably nothing guarantees that we don't re-load from the global
(the global may be more expensive to access than the stack).

Otherwise this looks ok, but maybe Vlad can chime in here.

Thanks,
Richard.

> gcc/
> 2014-10-24  Jan Beulich  <jbeulich@suse.com>
>
>         * cfgexpand.c (expand_one_var): Exclude static and external
>         variables when adjusting stack alignment related state.
>
> gcc/testsuite/
> 2014-10-24  Jan Beulich  <jbeulich@suse.com>
>
>         * gcc.c-torture/execute/stkalign.c: New.
>
> --- a/gcc/cfgexpand.c
> +++ b/gcc/cfgexpand.c
> @@ -1233,12 +1233,15 @@ static HOST_WIDE_INT
>  expand_one_var (tree var, bool toplevel, bool really_expand)
>  {
>    unsigned int align = BITS_PER_UNIT;
> +  bool stack = true;
>    tree origvar = var;
>
>    var = SSAVAR (var);
>
>    if (TREE_TYPE (var) != error_mark_node && TREE_CODE (var) == VAR_DECL)
>      {
> +      stack = !TREE_STATIC (var) && !DECL_EXTERNAL (var);
> +
>        /* Because we don't know if VAR will be in register or on stack,
>          we conservatively assume it will be on stack even if VAR is
>          eventually put into register after RA pass.  For non-automatic
> @@ -1267,22 +1270,25 @@ expand_one_var (tree var, bool toplevel,
>         align = POINTER_SIZE;
>      }
>
> -  if (SUPPORTS_STACK_ALIGNMENT
> -      && crtl->stack_alignment_estimated < align)
> +  if (stack)
>      {
> -      /* stack_alignment_estimated shouldn't change after stack
> -         realign decision made */
> -      gcc_assert (!crtl->stack_realign_processed);
> -      crtl->stack_alignment_estimated = align;
> +      if (SUPPORTS_STACK_ALIGNMENT
> +         && crtl->stack_alignment_estimated < align)
> +       {
> +         /* stack_alignment_estimated shouldn't change after stack
> +            realign decision made */
> +         gcc_assert (!crtl->stack_realign_processed);
> +         crtl->stack_alignment_estimated = align;
> +       }
> +
> +      /* stack_alignment_needed > PREFERRED_STACK_BOUNDARY is permitted.
> +        So here we only make sure stack_alignment_needed >= align.  */
> +      if (crtl->stack_alignment_needed < align)
> +       crtl->stack_alignment_needed = align;
> +      if (crtl->max_used_stack_slot_alignment < align)
> +       crtl->max_used_stack_slot_alignment = align;
>      }
>
> -  /* stack_alignment_needed > PREFERRED_STACK_BOUNDARY is permitted.
> -     So here we only make sure stack_alignment_needed >= align.  */
> -  if (crtl->stack_alignment_needed < align)
> -    crtl->stack_alignment_needed = align;
> -  if (crtl->max_used_stack_slot_alignment < align)
> -    crtl->max_used_stack_slot_alignment = align;
> -
>    if (TREE_CODE (origvar) == SSA_NAME)
>      {
>        gcc_assert (TREE_CODE (var) != VAR_DECL
> --- a/gcc/testsuite/gcc.c-torture/execute/stkalign.c
> +++ b/gcc/testsuite/gcc.c-torture/execute/stkalign.c
> @@ -0,0 +1,26 @@
> +/* { dg-options "-fno-inline" } */
> +
> +#include <assert.h>
> +
> +#define ALIGNMENT 64
> +
> +unsigned test(unsigned n, unsigned p)
> +{
> +  static struct { char __attribute__((__aligned__(ALIGNMENT))) c; } s;
> +  unsigned x;
> +
> +  assert(__alignof__(s) == ALIGNMENT);
> +  asm ("" : "=g" (x), "+m" (s) : "0" (&x));
> +
> +  return n ? test(n - 1, x) : (x ^ p);
> +}
> +
> +int main (int argc, char *argv[] __attribute__((unused)))
> +{
> +  unsigned int x = test(argc, 0);
> +
> +  x |= test(argc + 1, 0);
> +  x |= test(argc + 2, 0);
> +
> +  return !(x & (ALIGNMENT - 1));
> +}
>
>
>