On Fri, Oct 24, 2014 at 4:57 PM, Jan Beulich <jbeul...@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 <jbeul...@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 <jbeul...@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));
> +}
>
>
>
