Follow-up-fix 2 to "[PATCH] Move PR84877 fix elsewhere (PR bootstrap/88450)"

Hans-Peter Nilsson hans-peter.nilsson@axis.com
Mon May 13 04:01:00 GMT 2019


> Date: Tue, 30 Apr 2019 11:37:17 -0600
> From: Jeff Law <law@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.

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"!)

brgds, H-P



More information about the Gcc-patches mailing list