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

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

[Jan Beulich]

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

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.

[v3: Re-base to current trunk.]
[v2: Drop inclusion of hard register variables, as requested by
     Jakub and Richard.]

gcc/
2015-12-10  Jan Beulich  <jbeulich@suse.com>

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

gcc/testsuite/
2015-12-10  Jan Beulich  <jbeulich@suse.com>

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

--- 2015-12-09/gcc/cfgexpand.c
+++ 2015-12-09/gcc/cfgexpand.c
@@ -1544,12 +1544,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
@@ -1578,7 +1581,8 @@ expand_one_var (tree var, bool toplevel,
 	align = POINTER_SIZE;
     }
 
-  record_alignment_for_reg_var (align);
+  if (stack)
+    record_alignment_for_reg_var (align);
 
   if (TREE_CODE (origvar) == SSA_NAME)
     {
--- 2015-12-09/gcc/testsuite/gcc.c-torture/execute/stkalign.c
+++ 2015-12-09/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));
+}




[-- Attachment #2: gcc-trunk-stack-align-ignore-static.patch --]
[-- Type: text/plain, Size: 2493 bytes --]

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.

[v3: Re-base to current trunk.]
[v2: Drop inclusion of hard register variables, as requested by
     Jakub and Richard.]

gcc/
2015-12-10  Jan Beulich  <jbeulich@suse.com>

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

gcc/testsuite/
2015-12-10  Jan Beulich  <jbeulich@suse.com>

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

--- 2015-12-09/gcc/cfgexpand.c
+++ 2015-12-09/gcc/cfgexpand.c
@@ -1544,12 +1544,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
@@ -1578,7 +1581,8 @@ expand_one_var (tree var, bool toplevel,
 	align = POINTER_SIZE;
     }
 
-  record_alignment_for_reg_var (align);
+  if (stack)
+    record_alignment_for_reg_var (align);
 
   if (TREE_CODE (origvar) == SSA_NAME)
     {
--- 2015-12-09/gcc/testsuite/gcc.c-torture/execute/stkalign.c
+++ 2015-12-09/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));
+}

Re: [v3] avoid alignment of static variables affecting stack's

[Bernd Schmidt]

On 12/10/2015 01:38 PM, Jan Beulich wrote:
>
> 	* cfgexpand.c (expand_one_var): Exclude static and external
> 	variables when adjusting stack alignment related state.
>
> gcc/testsuite/
> 2015-12-10  Jan Beulich  <jbeulich@suse.com>
>
> 	* gcc.c-torture/execute/stkalign.c: New.
>
> --- 2015-12-09/gcc/cfgexpand.c
> +++ 2015-12-09/gcc/cfgexpand.c
> @@ -1544,12 +1544,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
> @@ -1578,7 +1581,8 @@ expand_one_var (tree var, bool toplevel,
>   	align = POINTER_SIZE;
>       }
>
> -  record_alignment_for_reg_var (align);
> +  if (stack)
> +    record_alignment_for_reg_var (align);

A bit further down we have

   else if (DECL_EXTERNAL (var))
     ;
   else if (DECL_HAS_VALUE_EXPR_P (var))
     ;
   else if (TREE_STATIC (var))
     ;
[....]
   return 0;

so I'm thinking the function doesn't do anything for DECL_EXTERNAL or 
TREE_STATIC vars. You're still computing alignment for them but not 
using it. I suggest just doing an early return for non-stack vars.


Bernd

Re: [v3] avoid alignment of static variables affecting stack's

[Jan Beulich]

>>> On 10.12.15 at 14:53, <bschmidt@redhat.com> wrote:
> On 12/10/2015 01:38 PM, Jan Beulich wrote:
>> --- 2015-12-09/gcc/cfgexpand.c
>> +++ 2015-12-09/gcc/cfgexpand.c
>> @@ -1544,12 +1544,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
>> @@ -1578,7 +1581,8 @@ expand_one_var (tree var, bool toplevel,
>>   	align = POINTER_SIZE;
>>       }
>>
>> -  record_alignment_for_reg_var (align);
>> +  if (stack)
>> +    record_alignment_for_reg_var (align);
> 
> A bit further down we have
> 
>    else if (DECL_EXTERNAL (var))
>      ;
>    else if (DECL_HAS_VALUE_EXPR_P (var))
>      ;
>    else if (TREE_STATIC (var))
>      ;
> [....]
>    return 0;
> 
> so I'm thinking the function doesn't do anything for DECL_EXTERNAL or 
> TREE_STATIC vars. You're still computing alignment for them but not 
> using it. I suggest just doing an early return for non-stack vars.

If not reaching

  if (TREE_CODE (origvar) == SSA_NAME)
    {
      gcc_assert (TREE_CODE (var) != VAR_DECL
		  || (!DECL_EXTERNAL (var)
		      && !DECL_HAS_VALUE_EXPR_P (var)
		      && !TREE_STATIC (var)
		      && TREE_TYPE (var) != error_mark_node
		      && !DECL_HARD_REGISTER (var)
		      && really_expand));
    }

in that case is not a problem, this would make for even simpler a
change. Please let me know.

Jan

Re: [v3] avoid alignment of static variables affecting stack's

[Bernd Schmidt]

On 12/10/2015 05:07 PM, Jan Beulich wrote:
> If not reaching
>
>    if (TREE_CODE (origvar) == SSA_NAME)
>      {
>        gcc_assert (TREE_CODE (var) != VAR_DECL
> 		  || (!DECL_EXTERNAL (var)
> 		      && !DECL_HAS_VALUE_EXPR_P (var)
> 		      && !TREE_STATIC (var)
> 		      && TREE_TYPE (var) != error_mark_node
> 		      && !DECL_HARD_REGISTER (var)
> 		      && really_expand));
>      }
>
> in that case is not a problem, this would make for even simpler a
> change. Please let me know.

I think that's fine. BTW there's a is_global_var predicate that 
{c,sh}ould be used for your test.


Bernd