This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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


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));
> +}
>
>
>


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]