[Bug target/39146] New: Unnecessary stack alignment

[Bug target/39146] New: Unnecessary stack alignment

[hjl dot tools at gmail dot com]

[hjl@gnu-6 avx-abi]$ cat x5.i
typedef long long __m256i __attribute__ ((__vector_size__ (32),
__may_alias__));


__m256i
bar (__m256i x)
{
  return x;
}
[hjl@gnu-6 avx-abi]$ /export/build/gnu/gcc-avx/build-x86_64-linux/gcc/xgcc
-B/export/build/gnu/gcc-avx/build-x86_64-linux/gcc/ -mavx 
-fno-asynchronous-unwind-tables -O2 x5.i -S -o x5.s
[hjl@gnu-6 avx-abi]$ cat x5.s
        .file   "x5.i"
        .text
        .p2align 4,,15
.globl bar
        .type   bar, @function
bar:
        pushq   %rbp
        movq    %rsp, %rbp
        andq    $-32, %rsp
        leave
        ret
        .size   bar, .-bar

Is that possible to avoid stack alignment here?


-- 
           Summary: Unnecessary stack alignment
           Product: gcc
           Version: 4.4.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: target
        AssignedTo: unassigned at gcc dot gnu dot org
        ReportedBy: hjl dot tools at gmail dot com


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=39146

[Bug target/39146] Unnecessary stack alignment

[Joey dot ye at intel dot com]

------- Comment #1 from Joey dot ye at intel dot com  2009-02-10 05:35 -------
Argument need 32 bytes alignment, No way to guarantee the argument won't be
spilled. That's why stack adjustment is there. 


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=39146

[Bug target/39146] Unnecessary stack alignment

[hjl dot tools at gmail dot com]

------- Comment #2 from hjl dot tools at gmail dot com  2009-02-11 15:15 -------
This is caused by assign_parms in function.c:

      /* Estimate stack alignment from parameter alignment.  */
      if (SUPPORTS_STACK_ALIGNMENT)
        {
          unsigned int align = FUNCTION_ARG_BOUNDARY (data.promoted_mode,
                                                      data.passed_type);
          if (TYPE_ALIGN (data.nominal_type) > align)
            align = TYPE_ALIGN (data.passed_type);
          if (crtl->stack_alignment_estimated < align)
            {
              gcc_assert (!crtl->stack_realign_processed);
              crtl->stack_alignment_estimated = align;
            }
        }


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=39146

[Bug target/39146] Unnecessary stack alignment

[hjl dot tools at gmail dot com]

------- Comment #3 from hjl dot tools at gmail dot com  2009-02-11 16:53 -------
Also expand_one_var in cfgexpand.c has

      if (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;
        }    


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=39146

[Bug target/39146] Unnecessary stack alignment

[hjl dot tools at gmail dot com]

------- Comment #4 from hjl dot tools at gmail dot com  2009-02-11 23:59 -------
The dynamic stack alignment may not be easy to optimize out.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=39146

[Bug target/39146] Unnecessary stack alignment

[Joey dot ye at intel dot com]

------- Comment #5 from Joey dot ye at intel dot com  2009-02-12 01:45 -------
Stack realign is finalized by
stack_realign = (incoming_stack_boundary
                                < (current_function_is_leaf
                                   ? crtl->max_used_stack_slot_alignment
                                   : crtl->stack_alignment_needed));
since bar is leaf function, it checks max_used_stack_slot_alignment.

According to it's definition, max_used_stack_slot_alignment is   /* The largest
alignment of slot allocated on the stack.  */. Parameter x isn't allocated on
local stack, so max_used_stack_slot_alignment shouldn't be set to 256 bits.

In locate_and_pad_parm,
  if (crtl->max_used_stack_slot_alignment < crtl->stack_alignment_needed)
    crtl->max_used_stack_slot_alignment = crtl->stack_alignment_needed;
sets max_used_stack_slot_alignment to 256 bits, which seems shouldn't happen
all the time.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=39146

[Bug target/39146] Unnecessary stack alignment

[hjl dot tools at gmail dot com]

------- Comment #6 from hjl dot tools at gmail dot com  2009-02-12 01:58 -------
locate_and_pad_parm in function.c has

  /* Remember if the outgoing parameter requires extra alignment on the
     calling function side.  */
  if (crtl->stack_alignment_needed < boundary)
    crtl->stack_alignment_needed = boundary;
  if (crtl->max_used_stack_slot_alignment < crtl->stack_alignment_needed)
    crtl->max_used_stack_slot_alignment = crtl->stack_alignment_needed;
  if (crtl->preferred_stack_boundary < boundary)
    crtl->preferred_stack_boundary = boundary;


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=39146

[Bug target/39146] Unnecessary stack alignment

[Joey dot ye at intel dot com]

------- Comment #7 from Joey dot ye at intel dot com  2009-02-12 02:26 -------
Created an attachment (id=17283)
 --> (http://gcc.gnu.org/bugzilla/attachment.cgi?id=17283&action=view)
A patch to fix this problem

Impact to other test unknown. Test undergoing.

HJ, can you also help to verify and test this patch?


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=39146

[Bug target/39146] Unnecessary stack alignment

[hjl dot tools at gmail dot com]

------- Comment #8 from hjl dot tools at gmail dot com  2009-02-12 02:33 -------
(In reply to comment #7)
> Created an attachment (id=17283)
 --> (http://gcc.gnu.org/bugzilla/attachment.cgi?id=17283&action=view) [edit]
> A patch to fix this problem
> 
> Impact to other test unknown. Test undergoing.
> 
> HJ, can you also help to verify and test this patch?
> 

With your patch, I got

.globl bar
        .type   bar, @function
bar:
        pushq   %rbp
        movq    %rsp, %rbp
        leave
        ret

We still have push and mov. I guess it may be the best we can do.
But please run full 32 and 64bit testsuite with your patch as well
as under emx-avx-sim.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=39146

[Bug target/39146] Unnecessary stack alignment

[Joey dot ye at intel dot com]

------- Comment #9 from Joey dot ye at intel dot com  2009-02-12 02:40 -------
(In reply to comment #8)
> We still have push and mov. I guess it may be the best we can do.
I believe so too.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=39146

[Bug target/39146] Unnecessary stack alignment

[Joey dot ye at intel dot com]

------- Comment #10 from Joey dot ye at intel dot com  2009-02-12 15:20 -------
(In reply to comment #8)
> We still have push and mov. I guess it may be the best we can do.
> But please run full 32 and 64bit testsuite with your patch as well
> as under emx-avx-sim.
full 32/64 bit test pass with no regression {-m32, -m32 -mstackrealign
-mpreferred-stack-boundary=4, -m64}. Haven't tested emx-avx-sim test yet.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=39146

[Bug target/39146] Unnecessary stack alignment

[hjl dot tools at gmail dot com]

------- Comment #11 from hjl dot tools at gmail dot com  2009-02-12 15:42 -------
I think we can consider it for 4.5.


-- 

hjl dot tools at gmail dot com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |4.5.0


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=39146

[Bug target/39146] Unnecessary stack alignment

[Joey dot ye at intel dot com]

------- Comment #12 from Joey dot ye at intel dot com  2009-02-16 08:49 -------
Created an attachment (id=17305)
 --> (http://gcc.gnu.org/bugzilla/attachment.cgi?id=17305&action=view)
New patch attached

Test finished. No regression with emx_avx_sim. Wait to checkin to 4.5


-- 

Joey dot ye at intel dot com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #17283|0                           |1
        is obsolete|                            |


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=39146

[Bug target/39146] Unnecessary stack alignment

[hjl at gcc dot gnu dot org]

------- Comment #13 from hjl at gcc dot gnu dot org  2009-02-21 00:01 -------
Subject: Bug 39146

Author: hjl
Date: Sat Feb 21 00:00:49 2009
New Revision: 144338

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=144338
Log:
2009-02-20  Joey Ye  <joey.ye@intel.com>

        PR middle-end/39146
        * cfgexpand.c (get_decl_align_unit): Update
        max_used_stack_slot_alignment with align instead of
        stack_alignment_needed.

        * function.c (assign_stack_local_1): Update
        max_used_stack_slot_alignment with alignment_in_bits instead
        of stack_alignment_needed.
        (locate_and_pad_parm): Don't update max_used_stack_slot_alignment
        here.

Modified:
    branches/stack/gcc/ChangeLog.stackalign
    branches/stack/gcc/cfgexpand.c
    branches/stack/gcc/function.c


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=39146

[Bug target/39146] Unnecessary stack alignment

[hjl dot tools at gmail dot com]

------- Comment #14 from hjl dot tools at gmail dot com  2009-02-28 18:51 -------
(In reply to comment #12)
> Created an attachment (id=17305)
 --> (http://gcc.gnu.org/bugzilla/attachment.cgi?id=17305&action=view) [edit]
> New patch attached
> 
> Test finished. No regression with emx_avx_sim. Wait to checkin to 4.5
> 

Joey, please submit it for review, targeting gcc 4.5.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=39146

[Bug target/39146] Unnecessary stack alignment

[hjl dot tools at gmail dot com]

------- Comment #15 from hjl dot tools at gmail dot com  2009-02-28 21:43 -------
(In reply to comment #14)
> (In reply to comment #12)
> > Created an attachment (id=17305)
 --> (http://gcc.gnu.org/bugzilla/attachment.cgi?id=17305&action=view) [edit]
> > New patch attached
> > 
> > Test finished. No regression with emx_avx_sim. Wait to checkin to 4.5
> > 
> 
> Joey, please submit it for review, targeting gcc 4.5.
> 

Joey, please also include a testcase.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=39146

[Bug target/39146] Unnecessary stack alignment

[hjl at gcc dot gnu dot org]

------- Comment #16 from hjl at gcc dot gnu dot org  2009-03-03 17:14 -------
Subject: Bug 39146

Author: hjl
Date: Tue Mar  3 17:14:04 2009
New Revision: 144577

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=144577
Log:
2009-03-03  Joey Ye  <joey.ye@intel.com>
            H.J. Lu  <hongjiu.lu@intel.com>

        PR middle-end/39146
        * gcc.target/i386/stackalign/pr39146.c: New.

Added:
    branches/stack/gcc/testsuite/gcc.target/i386/stackalign/pr39146.c
Modified:
    branches/stack/gcc/testsuite/ChangeLog.stackalign


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=39146

[Bug target/39146] Unnecessary stack alignment

[hjl dot tools at gmail dot com]

------- Comment #17 from hjl dot tools at gmail dot com  2009-03-03 17:36 -------
A patch is posted at

http://gcc.gnu.org/ml/gcc-patches/2009-03/msg00159.html


-- 

hjl dot tools at gmail dot com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                URL|                            |http://gcc.gnu.org/ml/gcc-
                   |                            |patches/2009-
                   |                            |03/msg00159.html


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=39146

[Bug target/39146] Unnecessary stack alignment

[pinskia at gcc dot gnu dot org]

------- Comment #18 from pinskia at gcc dot gnu dot org  2009-04-16 18:33 -------
Stop setting the target milestone unless it is a regression.


-- 

pinskia at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|4.5.0                       |---


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=39146

[Bug target/39146] Unnecessary stack alignment

[hjl at gcc dot gnu dot org]

------- Comment #19 from hjl at gcc dot gnu dot org  2009-06-12 12:54 -------
Subject: Bug 39146

Author: hjl
Date: Fri Jun 12 12:54:30 2009
New Revision: 148428

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=148428
Log:
gcc/

2009-06-12  Joey Ye  <joey.ye@intel.com>

        PR middle-end/39146
        * cfgexpand.c (get_decl_align_unit): Update
        max_used_stack_slot_alignment with align instead of
        stack_alignment_needed.

        * function.c (assign_stack_local_1): Update
        max_used_stack_slot_alignment with alignment_in_bits instead
        of stack_alignment_needed.
        (locate_and_pad_parm): Don't update max_used_stack_slot_alignment
        here.

gcc/testsuite/

2009-06-12  Joey Ye  <joey.ye@intel.com>
            H.J. Lu  <hongjiu.lu@intel.com>

        PR middle-end/39146
        * gcc.target/i386/stackalign/pr39146.c: New.

Added:
    trunk/gcc/testsuite/gcc.target/i386/stackalign/pr39146.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/cfgexpand.c
    trunk/gcc/function.c
    trunk/gcc/testsuite/ChangeLog


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=39146

[Bug target/39146] Unnecessary stack alignment

[hjl dot tools at gmail dot com]

------- Comment #20 from hjl dot tools at gmail dot com  2009-06-13 00:10 -------
Fixed.


-- 

hjl dot tools at gmail dot com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |RESOLVED
         Resolution|                            |FIXED


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=39146