[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

[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  

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  
H.J. Lu  

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

[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 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

[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  
H.J. Lu  

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 #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 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 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  

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

[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 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 #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

[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

[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 #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 #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 #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 #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

[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 #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

[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