[PATCH 1/2] c-family: Copy DECL_USER_ALIGN even if DECL_ALIGN is similar.

[Robin Dapp <rdapp@linux.ibm.com>]

[-- Attachment #1: Type: text/plain, Size: 2573 bytes --]

Hi,

on s390 a warning test fails:

inline int ATTR ((cold, aligned (8)))
finline_hot_noret_align (int);

inline int ATTR ((warn_unused_result))
finline_hot_noret_align (int);

inline int ATTR ((aligned (4)))
   finline_hot_noret_align (int);  /* { dg-warning "ignoring attribute 
.aligned \\(4\\). because it conflicts with attribute .aligned \\(8\\)."

This test actually uncovered two problems.  First, on s390 the default 
function alignment is 8 bytes.  When the second decl above is merged 
with the first one, DECL_USER_ALIGN is only copied if DECL_ALIGN (old) > 
DECL_ALIGN (new).  Subsequently, when merging the third decl, no warning 
is emitted since DECL_USER_ALIGN is unset.

This patch also copies DECL_USER_ALIGN if DECL_ALIGN (old) == DECL_ALIGN 
(new) && DECL_USER_ALIGN (olddecl) != DECL_USER_ALIGN (newdecl)).


Then, while going through the related files I also noticed that we emit 
a wrong warning for:

inline int ATTR ((aligned (32)))
finline_align (int);

inline int ATTR ((aligned (4)))
   finline_align (int);  /* { dg-warning "ignoring attribute .aligned 
\\(4\\). because it conflicts with attribute .aligned \\(32\\)." "" } */

What we emit is

warning: ignoring attribute ‘aligned (4)’ because it conflicts with 
attribute ‘aligned (8)’ [-Wattributes].

This is due to the short circuit evaluation in c-attribs.c:

            && ((curalign = DECL_ALIGN (decl)) > bitalign
                || ((lastalign = DECL_ALIGN (last_decl)) > bitalign)))

where lastalign is only initialized when curalign > bitalign.  On s390 
this is not the case and lastalign is used zero-initialized in the 
following code.

On top, the following condition
   else if (!warn_if_not_aligned_p
           && TREE_CODE (decl) == FUNCTION_DECL
           && DECL_ALIGN (decl) > bitalign)

seems to be fully covered by
     else if (TREE_CODE (decl) == FUNCTION_DECL
	   && ((curalign = DECL_ALIGN (decl)) > bitalign
	       || ((lastalign = DECL_ALIGN (last_decl)) > bitalign)))

and so is essentially dead. I therefore removed it as there does not 
seem to be a test anywhere for the error message ( "alignment for %q+D 
was previously specified as %d and may not be decreased") either.

Bootstrapped and regtested on s390, ppc64 and amd64.
Is it OK?

Regards
  Robin

--

gcc/c-family/ChangeLog:

         * c-attribs.c (common_handle_aligned_attribute): Re-init 
lastalign and remove case.

gcc/c/ChangeLog:

         * c-decl.c (merge_decls): Copy DECL_USER_ALIGN even for similar 
DECL_ALIGN.

gcc/cp/ChangeLog:

         * decl.c (duplicate_decls): Dito.

[-- Attachment #2: 0001-c-family-Copy-DECL_USER_ALIGN-even-if-DECL_ALIGN-is-.patch --]
[-- Type: text/x-patch, Size: 4030 bytes --]

From f3a9f8b9bd1e28c23f2efcdb061959852a91deb6 Mon Sep 17 00:00:00 2001
From: Robin Dapp <rdapp@linux.ibm.com>
Date: Tue, 20 Apr 2021 10:51:18 +0200
Subject: [PATCH 1/2] c-family: Copy DECL_USER_ALIGN even if DECL_ALIGN is
 similar.

When re-declaring a function with differing attributes DECL_USER_ALIGN
is usually not merged/copied when DECL_ALIGN is similar.  On s390 this
will cause a warning message not to be shown.  Similarly, a note is not
shown when we short-circuit an alignment initialization in
common_handle_aligned_attribute ().

Fix this by copying DECL_USER_ALIGN even if DECL_ALIGN is similar as
well as reinitializing the short-circuited initialization.
---
 gcc/c-family/c-attribs.c | 23 ++++-------------------
 gcc/c/c-decl.c           |  3 +++
 gcc/cp/decl.c            |  5 +++++
 3 files changed, 12 insertions(+), 19 deletions(-)

diff --git a/gcc/c-family/c-attribs.c b/gcc/c-family/c-attribs.c
index c1f652d1dc9..d132b6fd3b6 100644
--- a/gcc/c-family/c-attribs.c
+++ b/gcc/c-family/c-attribs.c
@@ -2317,6 +2317,10 @@ common_handle_aligned_attribute (tree *node, tree name, tree args, int flags,
 	   && ((curalign = DECL_ALIGN (decl)) > bitalign
 	       || ((lastalign = DECL_ALIGN (last_decl)) > bitalign)))
     {
+      /* Re-init lastalign in case we short-circuit the condition,
+	 i.e.  curalign > bitalign.  */
+      lastalign = DECL_ALIGN (last_decl);
+
       /* Either a prior attribute on the same declaration or one
 	 on a prior declaration of the same function specifies
 	 stricter alignment than this attribute.  */
@@ -2366,25 +2370,6 @@ common_handle_aligned_attribute (tree *node, tree name, tree args, int flags,
       This formally comes from the c++11 specification but we are
       doing it for the GNU attribute syntax as well.  */
     *no_add_attrs = true;
-  else if (!warn_if_not_aligned_p
-	   && TREE_CODE (decl) == FUNCTION_DECL
-	   && DECL_ALIGN (decl) > bitalign)
-    {
-      /* Don't warn for function alignment here if warn_if_not_aligned_p
-	 is true.  It will be warned about later.  */
-      if (DECL_USER_ALIGN (decl))
-	{
-	  /* Only reject attempts to relax/override an alignment
-	     explicitly specified previously and accept declarations
-	     that appear to relax the implicit function alignment for
-	     the target.  Both increasing and increasing the alignment
-	     set by -falign-functions setting is permitted.  */
-	  error ("alignment for %q+D was previously specified as %d "
-		 "and may not be decreased", decl,
-		 DECL_ALIGN (decl) / BITS_PER_UNIT);
-	  *no_add_attrs = true;
-	}
-    }
   else if (warn_if_not_aligned_p
 	   && TREE_CODE (decl) == FIELD_DECL
 	   && !DECL_C_BIT_FIELD (decl))
diff --git a/gcc/c/c-decl.c b/gcc/c/c-decl.c
index 3ea4708c507..2ea5051e9cd 100644
--- a/gcc/c/c-decl.c
+++ b/gcc/c/c-decl.c
@@ -2620,6 +2620,9 @@ merge_decls (tree newdecl, tree olddecl, tree newtype, tree oldtype)
 	  SET_DECL_ALIGN (newdecl, DECL_ALIGN (olddecl));
 	  DECL_USER_ALIGN (newdecl) |= DECL_USER_ALIGN (olddecl);
 	}
+      else if (DECL_ALIGN (olddecl) == DECL_ALIGN (newdecl)
+	       && DECL_USER_ALIGN (olddecl) != DECL_USER_ALIGN (newdecl))
+	DECL_USER_ALIGN (newdecl) |= DECL_USER_ALIGN (olddecl);
       if (DECL_WARN_IF_NOT_ALIGN (olddecl)
 	  > DECL_WARN_IF_NOT_ALIGN (newdecl))
 	SET_DECL_WARN_IF_NOT_ALIGN (newdecl,
diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c
index 60dc2bf182d..ad07b1a4e63 100644
--- a/gcc/cp/decl.c
+++ b/gcc/cp/decl.c
@@ -2795,6 +2795,11 @@ duplicate_decls (tree newdecl, tree olddecl, bool hiding, bool was_hidden)
       SET_DECL_ALIGN (newdecl, DECL_ALIGN (olddecl));
       DECL_USER_ALIGN (newdecl) |= DECL_USER_ALIGN (olddecl);
     }
+  else if (DECL_ALIGN (olddecl) == DECL_ALIGN (newdecl)
+      && DECL_USER_ALIGN (olddecl) != DECL_USER_ALIGN (newdecl))
+    {
+      DECL_USER_ALIGN (newdecl) |= DECL_USER_ALIGN (olddecl);
+    }
   DECL_USER_ALIGN (olddecl) = DECL_USER_ALIGN (newdecl);
   if (DECL_WARN_IF_NOT_ALIGN (olddecl)
       > DECL_WARN_IF_NOT_ALIGN (newdecl))
-- 
2.23.0