| Message ID | 201902110109.x1B19UWh021718@ignucius.se.axis.com |
|---|---|
| State | New |
| Headers | show |
| Series | Follow-up-fix 2 to "[PATCH] Move PR84877 fix elsewhere (PR bootstrap/88450)" | expand |
On February 11, 2019 2:09:30 AM GMT+01:00, Hans-Peter Nilsson <hans-peter.nilsson@axis.com> wrote: >Here's the follow-up, getting rid of the observed >alignment-padding in execute/930126-1.c: the x parameter in f >spuriously being runtime-aligned to BITS_PER_WORD. I separated >this change because this is an older issue, a change introduced >in r94104 where BITS_PER_WORD was chosen perhaps because we >expect register-sized writes into this area. Here, we instead >align to a minimum of PREFERRED_STACK_BOUNDARY, but of course >gated on ! STRICT_ALIGNMENT. > >Regtested cris-elf and x86_64-pc-linux-gnu. > >Ok to commit? > >gcc: > * function.c (assign_parm_setup_block): If not STRICT_ALIGNMENT, > instead of always BITS_PER_WORD, align the stacked > parameter to a minimum PREFERRED_STACK_BOUNDARY. > >--- function.c.orig2 Sat Feb 9 00:53:17 2019 >+++ function.c Sat Feb 9 23:21:35 2019 >@@ -2912,7 +2912,10 @@ assign_parm_setup_block (struct assign_p > size_stored = CEIL_ROUND (size, UNITS_PER_WORD); > if (stack_parm == 0) > { >- SET_DECL_ALIGN (parm, MAX (DECL_ALIGN (parm), BITS_PER_WORD)); >+ HOST_WIDE_INT min_parm_align >+ = STRICT_ALIGNMENT ? BITS_PER_WORD : PREFERRED_STACK_BOUNDARY; Shouldn't it be MIN (...) of BOTH? >+ >+ SET_DECL_ALIGN (parm, MAX (DECL_ALIGN (parm), min_parm_align)); > if (DECL_ALIGN (parm) > MAX_SUPPORTED_STACK_ALIGNMENT) > { > rtx allocsize = gen_int_mode (size_stored, Pmode); > >brgds, H-P
On Mon, Feb 11, 2019 at 10:00:03AM +0100, Hans-Peter Nilsson wrote: > > Date: Mon, 11 Feb 2019 07:38:14 +0100 > > From: Richard Biener <richard.guenther@gmail.com> > > > >+ HOST_WIDE_INT min_parm_align > > >+ = STRICT_ALIGNMENT ? BITS_PER_WORD : PREFERRED_STACK_BOUNDARY; > > > > Shouldn't it be MIN (...) of BOTH? > > That *does* seem logical... Take 2 as follows, in testing as > before. > > Ok to commit, if testing passes? Is PREFERRED_STACK_BOUNDARY what we want here? Shouldn't that be STACK_BOUNDARY, or PARM_BOUNDARY? Though, PARM_BOUNDARY on cris is 32... > gcc: > * function.c (assign_parm_setup_block): Align the > parameter to a minimum of PREFERRED_STACK_BOUNDARY and > BITS_PER_WORD instead of always BITS_PER_WORD. > > --- gcc/function.c.orig2 Sat Feb 9 00:53:17 2019 > +++ gcc/function.c Mon Feb 11 09:37:28 2019 > @@ -2912,7 +2912,10 @@ assign_parm_setup_block (struct assign_p > size_stored = CEIL_ROUND (size, UNITS_PER_WORD); > if (stack_parm == 0) > { > - SET_DECL_ALIGN (parm, MAX (DECL_ALIGN (parm), BITS_PER_WORD)); > + HOST_WIDE_INT min_parm_align > + = MIN (BITS_PER_WORD, PREFERRED_STACK_BOUNDARY); > + > + SET_DECL_ALIGN (parm, MAX (DECL_ALIGN (parm), min_parm_align)); > if (DECL_ALIGN (parm) > MAX_SUPPORTED_STACK_ALIGNMENT) > { > rtx allocsize = gen_int_mode (size_stored, Pmode); Jakub
On 2/10/19 6:09 PM, Hans-Peter Nilsson wrote: > Here's the follow-up, getting rid of the observed > alignment-padding in execute/930126-1.c: the x parameter in f > spuriously being runtime-aligned to BITS_PER_WORD. I separated > this change because this is an older issue, a change introduced > in r94104 where BITS_PER_WORD was chosen perhaps because we > expect register-sized writes into this area. Here, we instead > align to a minimum of PREFERRED_STACK_BOUNDARY, but of course > gated on ! STRICT_ALIGNMENT. > > Regtested cris-elf and x86_64-pc-linux-gnu. > > Ok to commit? > > gcc: > * function.c (assign_parm_setup_block): If not STRICT_ALIGNMENT, > instead of always BITS_PER_WORD, align the stacked > parameter to a minimum PREFERRED_STACK_BOUNDARY. Interestingly enough in the thread from 2005 Richard S suggests that he could have made increasing the alignment conditional on STRICT_ALIGNMENT but thought that with the size already being rounded up it wasn't worth it and that we could take advantage of the increased alignment elsewhere. I wonder if we could just go back to that idea. Leave the alignment as DECL_ALIGN for !STRICT_ALIGNMENT targets and bump it up for STRICT_ALIGNMENT targets? So something like align = STRICT_ALIGNMENT ? MAX (DECL_ALIGN (parm), BITS_PER_WORD) : DECL_ALIGN (parm) Jeff
--- function.c.orig2 Sat Feb 9 00:53:17 2019 +++ function.c Sat Feb 9 23:21:35 2019 @@ -2912,7 +2912,10 @@ assign_parm_setup_block (struct assign_p size_stored = CEIL_ROUND (size, UNITS_PER_WORD); if (stack_parm == 0) { - SET_DECL_ALIGN (parm, MAX (DECL_ALIGN (parm), BITS_PER_WORD)); + HOST_WIDE_INT min_parm_align + = STRICT_ALIGNMENT ? BITS_PER_WORD : PREFERRED_STACK_BOUNDARY; + + SET_DECL_ALIGN (parm, MAX (DECL_ALIGN (parm), min_parm_align)); if (DECL_ALIGN (parm) > MAX_SUPPORTED_STACK_ALIGNMENT) { rtx allocsize = gen_int_mode (size_stored, Pmode);