On 07/01/2016 10:34 AM, Dodji Seketeli wrote:

The patch below was bootstrapped and tested on x86_64-linux, without
issues,

The patch looks good to me, thanks.

Alright. I think we need a C frontend maintainer or maybe Jason to approve it. Attaching again.

but I'm not convinced this code is covered by any testcase.

Hmhm, looking at the test cases, accompanying the initial patch that
introduced that code, I agree.  I guess we should probably add a test case that
uses [[gnu::aligned (val)]], with 'val' being an alignment that is
greater than MAX (TYPE_ALIGN_UNIT (long_long_integer_type_node),
                  TYPE_ALIGN_UNIT (long_double_type_node))
in the g++.dg/cpp0x/gen-attrs-*.C series of tests?

I really don't know what this code is even doing, so I'll leave that up to you...


Bernd

	* c-common.c (cxx_fundamental_alignment_p): Use TYPE_ALIGN_UNIT,
	not TYPE_ALIGN.

Index: gcc/c-family/c-common.c
===================================================================
--- gcc/c-family/c-common.c	(revision 237797)
+++ gcc/c-family/c-common.c	(working copy)
@@ -7668,10 +7668,10 @@ fail:
   return NULL_TREE;
 }
 
-/* Check whether ALIGN is a valid user-specified alignment.  If so,
-   return its base-2 log; if not, output an error and return -1.  If
-   ALLOW_ZERO then 0 is valid and should result in a return of -1 with
-   no error.  */
+/* Check whether ALIGN is a valid user-specified alignment, specified
+   in bytes.  If so, return its base-2 log; if not, output an error
+   and return -1.  If ALLOW_ZERO then 0 is valid and should result in
+   a return of -1 with no error.  */
 int
 check_user_alignment (const_tree align, bool allow_zero)
 {
@@ -12700,9 +12700,9 @@ scalar_to_vector (location_t loc, enum t
   return stv_nothing;
 }
 
-/* Return true iff ALIGN is an integral constant that is a fundamental
-   alignment, as defined by [basic.align] in the c++-11
-   specifications.
+/* Return true iff ALIGN, which is specified in bytes, is an integral
+   constant that is a fundamental alignment, as defined by
+   [basic.align] in the c++-11 specifications.
 
    That is:
 
@@ -12712,10 +12712,11 @@ scalar_to_vector (location_t loc, enum t
         alignof(max_align_t)].  */
 
 bool
-cxx_fundamental_alignment_p  (unsigned align)
+cxx_fundamental_alignment_p (unsigned align)
 {
-  return (align <=  MAX (TYPE_ALIGN (long_long_integer_type_node),
-			 TYPE_ALIGN (long_double_type_node)));
+  unsigned limit = MAX (TYPE_ALIGN_UNIT (long_long_integer_type_node),
+			TYPE_ALIGN_UNIT (long_double_type_node));
+  return align <= limit;
 }
 
 /* Return true if T is a pointer to a zero-sized aggregate.  */

Reply via email to