On 5/12/19 10:01 PM, Hans-Peter Nilsson wrote:
>> Date: Tue, 30 Apr 2019 11:37:17 -0600
>> From: Jeff Law <l...@redhat.com>
>
>> 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)
>
> That'd work for me, I think. Testing in progress. Thanks.
> Almost obvious, but: is this ok; what you meant?
>
> gcc:
> * function.c (assign_parm_setup_block): Raise alignment of
> stacked parameter only for STRICT_ALIGNMENT targets.
Yea. I threw it into my tester overnight and it hasn't caused any
problems. I think this is good for the trunk.
>
> Index: gcc/function.c
> ===================================================================
> --- gcc/function.c (revision 271111)
> +++ gcc/function.c (working copy)
> @@ -2912,7 +2912,11 @@ 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 parm_align
> + = (STRICT_ALIGNMENT
> + ? MAX (DECL_ALIGN (parm), BITS_PER_WORD) : DECL_ALIGN (parm));
> +
> + SET_DECL_ALIGN (parm, parm_align);
> if (DECL_ALIGN (parm) > MAX_SUPPORTED_STACK_ALIGNMENT)
> {
> rtx allocsize = gen_int_mode (size_stored, Pmode);
>
> PS. A preferable solution would IMHO involve hookifying
> parameter padding and alignment as separate entities. Maybe
> later, perhaps even after PR84877 is fixed, or that bug risk
> dying from perceived misprioritized attention. (Amazing how
> easy it is to "overalign" here, and hard to align "there"!)
Yea, but at this point I suspect folks are a bit reluctant to dig into
this code :(
jeff
