[PATCH] Respect user align for local variable

[PATCH] Respect user align for local variable

[Kito Cheng]

gcc/ChangeLog

	* cfgexpand.c (align_local_variable): Check DECL_USER_ALIGN.
---
 gcc/cfgexpand.c | 20 +++++++++++++-------
 1 file changed, 13 insertions(+), 7 deletions(-)

diff --git a/gcc/cfgexpand.c b/gcc/cfgexpand.c
index a7ec77d5c85..19a020b4b97 100644
--- a/gcc/cfgexpand.c
+++ b/gcc/cfgexpand.c
@@ -369,13 +369,19 @@ align_local_variable (tree decl, bool really_expand)
     align = TYPE_ALIGN (TREE_TYPE (decl));
   else
     {
-      align = LOCAL_DECL_ALIGNMENT (decl);
-      /* Don't change DECL_ALIGN when called from estimated_stack_frame_size.
-	 That is done before IPA and could bump alignment based on host
-	 backend even for offloaded code which wants different
-	 LOCAL_DECL_ALIGNMENT.  */
-      if (really_expand)
-	SET_DECL_ALIGN (decl, align);
+      if (DECL_USER_ALIGN (decl))
+	align = DECL_ALIGN (decl);
+      else
+	{
+	  align = LOCAL_DECL_ALIGNMENT (decl);
+	  /* Don't change DECL_ALIGN when called from
+	     estimated_stack_frame_size.
+	     That is done before IPA and could bump alignment based on host
+	     backend even for offloaded code which wants different
+	     LOCAL_DECL_ALIGNMENT.  */
+	  if (really_expand)
+	    SET_DECL_ALIGN (decl, align);
+	}
     }
   return align / BITS_PER_UNIT;
 }
-- 
2.25.2

Re: [PATCH] Respect user align for local variable

[Jakub Jelinek]

On Mon, Mar 30, 2020 at 06:31:27PM +0800, Kito Cheng wrote:
> gcc/ChangeLog
> 
> 	* cfgexpand.c (align_local_variable): Check DECL_USER_ALIGN.

Why?  LOCAL_DECL_ALIGNMENT surely shouldn't decrease alignment of decls
with DECL_USER_ALIGN vars (but do you have evidence that it does),
but it is just fine to increase alignment of vars with explicit alignment
requirements if the compiler thinks it is beneficial.  Larger alignment
still satisfies the user requirement...

	Jakub

Re: [PATCH] Respect user align for local variable

[Kito Cheng]

Hi Jakub:

Thanks for your correction, I read the doc for the aligned attribute
again[1], it's minimum alignment not restricted alignment, I thought
it should honor to the alignment attribute, Richard Biener
suggested[2] should put an assertion here to make sure never decrease
alignment here, so I'll send another patch for add assertion.

[1] https://gcc.gnu.org/onlinedocs/gcc/Common-Type-Attributes.html#Common-Type-Attributes
[2] https://gcc.gnu.org/pipermail/gcc-patches/2020-March/542808.html

On Mon, Mar 30, 2020 at 7:21 PM Jakub Jelinek <jakub@redhat.com> wrote:
>
> On Mon, Mar 30, 2020 at 06:31:27PM +0800, Kito Cheng wrote:
> > gcc/ChangeLog
> >
> >       * cfgexpand.c (align_local_variable): Check DECL_USER_ALIGN.
>
> Why?  LOCAL_DECL_ALIGNMENT surely shouldn't decrease alignment of decls
> with DECL_USER_ALIGN vars (but do you have evidence that it does),
> but it is just fine to increase alignment of vars with explicit alignment
> requirements if the compiler thinks it is beneficial.  Larger alignment
> still satisfies the user requirement...
>
>         Jakub
>