* [C PATCH] Don't reject valid code with _Alignas (PR c/61053)
@ 2014-05-05 20:27 Marek Polacek
2014-05-05 20:31 ` Marek Polacek
` (3 more replies)
0 siblings, 4 replies; 13+ messages in thread
From: Marek Polacek @ 2014-05-05 20:27 UTC (permalink / raw)
To: GCC Patches; +Cc: Joseph S. Myers
In this PR the issue is that we reject (valid) code such as
_Alignas (long long) long long foo;
with -m32, because we trip this condition:
alignas_align = 1U << declspecs->align_log;
if (alignas_align < TYPE_ALIGN_UNIT (type))
{
if (name)
error_at (loc, "%<_Alignas%> specifiers cannot reduce "
"alignment of %qE", name);
and error later on, since alignas_align is 4 (correct, see PR52023 for
why), but TYPE_ALIGN_UNIT of long long is 8. I think TYPE_ALIGN_UNIT
is wrong here as that won't give us minimal alignment required.
In c_sizeof_or_alignof_type we already have the code to compute such
minimal alignment so I just moved the code to a separate function
and used that instead of TYPE_ALIGN_UNIT.
Note that the test is run only on i?86 and x86_64, because we can't (?)
easily determine which target requires what alignment.
Regtested/bootstrapped on x86_64-unknown-linux-gnu and
powerpc64-unknown-linux-gnu, ok for trunk?
2014-05-05 Marek Polacek <polacek@redhat.com>
PR c/61053
c-family/
* c-common.c (min_align_of_type): New function factored out from...
(c_sizeof_or_alignof_type): ...here.
* c-common.h (min_align_of_type): Declare.
c/
* c-decl.c (grokdeclarator): Use min_align_of_type instead of
TYPE_ALIGN_UNIT.
testsuite/
* gcc.dg/pr61053.c: New test.
diff --git gcc/c-family/c-common.c gcc/c-family/c-common.c
index 0ad955d..6d4440e 100644
--- gcc/c-family/c-common.c
+++ gcc/c-family/c-common.c
@@ -4931,6 +4931,26 @@ c_common_get_alias_set (tree t)
return -1;
}
\f
+/* Return the least alignment required for type TYPE. */
+
+unsigned int
+min_align_of_type (tree type)
+{
+ unsigned int align = TYPE_ALIGN (type);
+ align = MIN (align, BIGGEST_ALIGNMENT);
+#ifdef BIGGEST_FIELD_ALIGNMENT
+ align = MIN (align, BIGGEST_FIELD_ALIGNMENT);
+#endif
+ unsigned int field_align = align;
+#ifdef ADJUST_FIELD_ALIGN
+ tree field = build_decl (UNKNOWN_LOCATION, FIELD_DECL, NULL_TREE,
+ type);
+ field_align = ADJUST_FIELD_ALIGN (field, field_align);
+#endif
+ align = MIN (align, field_align);
+ return align / BITS_PER_UNIT;
+}
+
/* Compute the value of 'sizeof (TYPE)' or '__alignof__ (TYPE)', where
the IS_SIZEOF parameter indicates which operator is being applied.
The COMPLAIN flag controls whether we should diagnose possibly
@@ -5009,21 +5029,7 @@ c_sizeof_or_alignof_type (location_t loc,
size_int (TYPE_PRECISION (char_type_node)
/ BITS_PER_UNIT));
else if (min_alignof)
- {
- unsigned int align = TYPE_ALIGN (type);
- align = MIN (align, BIGGEST_ALIGNMENT);
-#ifdef BIGGEST_FIELD_ALIGNMENT
- align = MIN (align, BIGGEST_FIELD_ALIGNMENT);
-#endif
- unsigned int field_align = align;
-#ifdef ADJUST_FIELD_ALIGN
- tree field = build_decl (UNKNOWN_LOCATION, FIELD_DECL, NULL_TREE,
- type);
- field_align = ADJUST_FIELD_ALIGN (field, field_align);
-#endif
- align = MIN (align, field_align);
- value = size_int (align / BITS_PER_UNIT);
- }
+ value = size_int (min_align_of_type (type));
else
value = size_int (TYPE_ALIGN_UNIT (type));
}
diff --git gcc/c-family/c-common.h gcc/c-family/c-common.h
index 57b7dce..d34d2bb 100644
--- gcc/c-family/c-common.h
+++ gcc/c-family/c-common.h
@@ -758,6 +758,7 @@ extern tree c_wrap_maybe_const (tree, bool);
extern tree c_save_expr (tree);
extern tree c_common_truthvalue_conversion (location_t, tree);
extern void c_apply_type_quals_to_decl (int, tree);
+extern unsigned int min_align_of_type (tree);
extern tree c_sizeof_or_alignof_type (location_t, tree, bool, bool, int);
extern tree c_alignof_expr (location_t, tree);
/* Print an error message for invalid operands to arith operation CODE.
diff --git gcc/c/c-decl.c gcc/c/c-decl.c
index 6e7c589..9f7419a 100644
--- gcc/c/c-decl.c
+++ gcc/c/c-decl.c
@@ -5931,7 +5931,7 @@ grokdeclarator (const struct c_declarator *declarator,
else if (declspecs->align_log != -1)
{
alignas_align = 1U << declspecs->align_log;
- if (alignas_align < TYPE_ALIGN_UNIT (type))
+ if (alignas_align < min_align_of_type (type))
{
if (name)
error_at (loc, "%<_Alignas%> specifiers cannot reduce "
diff --git gcc/testsuite/gcc.dg/pr61053.c gcc/testsuite/gcc.dg/pr61053.c
index e69de29..e3db534 100644
--- gcc/testsuite/gcc.dg/pr61053.c
+++ gcc/testsuite/gcc.dg/pr61053.c
@@ -0,0 +1,75 @@
+/* PR c/61053 */
+/* { dg-do compile { target i?86-*-* x86_64-*-* } } */
+/* { dg-options "-std=c11 -pedantic-errors" } */
+
+_Alignas (char) char cc;
+_Alignas (short int) char cs;
+_Alignas (int) char ci;
+_Alignas (long int) char cl;
+_Alignas (long long int) char cll;
+_Alignas (float) char cf;
+_Alignas (double) char cd;
+_Alignas (long double) char cld;
+
+_Alignas (char) short int sc; /* { dg-error "cannot reduce alignment" } */
+_Alignas (short int) short int ss;
+_Alignas (int) short int si;
+_Alignas (long int) short int sl;
+_Alignas (long long int) short int sll;
+_Alignas (float) short int sf;
+_Alignas (double) short int sd;
+_Alignas (long double) short int sld;
+
+_Alignas (char) int ic; /* { dg-error "cannot reduce alignment" } */
+_Alignas (short int) int is; /* { dg-error "cannot reduce alignment" } */
+_Alignas (int) int ii;
+_Alignas (long int) int il;
+_Alignas (long long int) int ill;
+_Alignas (float) int if_;
+_Alignas (double) int id;
+_Alignas (long double) int ild;
+
+_Alignas (char) long int lic; /* { dg-error "cannot reduce alignment" } */
+_Alignas (short int) long int lis; /* { dg-error "cannot reduce alignment" } */
+_Alignas (int) long int lii; /* { dg-error "cannot reduce alignment" "" { target { lp64 } } } */
+_Alignas (long int) long int lil;
+_Alignas (long long int) long int lill;
+_Alignas (float) long int lif; /* { dg-error "cannot reduce alignment" "" { target { lp64 } } } */
+_Alignas (double) long int lid;
+_Alignas (long double) long int lild;
+
+_Alignas (char) long long int llic; /* { dg-error "cannot reduce alignment" } */
+_Alignas (short int) long long int llis; /* { dg-error "cannot reduce alignment" } */
+_Alignas (int) long long int llii; /* { dg-error "cannot reduce alignment" "" { target { lp64 } } } */
+_Alignas (long int) long long int llil;
+_Alignas (long long int) long long int llill;
+_Alignas (float) long long int llif; /* { dg-error "cannot reduce alignment" "" { target { lp64 } } } */
+_Alignas (double) long long int llid;
+_Alignas (long double) long long int llild;
+
+_Alignas (char) float fc; /* { dg-error "cannot reduce alignment" } */
+_Alignas (short int) float fs; /* { dg-error "cannot reduce alignment" } */
+_Alignas (int) float fi;
+_Alignas (long int) float fl;
+_Alignas (long long int) float fll;
+_Alignas (float) float ff;
+_Alignas (double) float fd;
+_Alignas (long double) float fld;
+
+_Alignas (char) double dc; /* { dg-error "cannot reduce alignment" } */
+_Alignas (short int) double ds; /* { dg-error "cannot reduce alignment" } */
+_Alignas (int) double di; /* { dg-error "cannot reduce alignment" "" { target { lp64 } } } */
+_Alignas (long int) double dl;
+_Alignas (long long int) double dll;
+_Alignas (float) double df; /* { dg-error "cannot reduce alignment" "" { target { lp64 } } } */
+_Alignas (double) double dd;
+_Alignas (long double) double dld;
+
+_Alignas (char) long double ldc; /* { dg-error "cannot reduce alignment" } */
+_Alignas (short int) long double lds; /* { dg-error "cannot reduce alignment" } */
+_Alignas (int) long double ldi; /* { dg-error "cannot reduce alignment" "" { target { lp64 } } } */
+_Alignas (long int) long double ldl; /* { dg-error "cannot reduce alignment" "" { target { lp64 } } } */
+_Alignas (long long int) long double ldll; /* { dg-error "cannot reduce alignment" "" { target { lp64 } } } */
+_Alignas (float) long double ldf; /* { dg-error "cannot reduce alignment" "" { target { lp64 } } } */
+_Alignas (double) long double ldd; /* { dg-error "cannot reduce alignment" "" { target { lp64 } } } */
+_Alignas (long double) long double ldld;
Marek
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [C PATCH] Don't reject valid code with _Alignas (PR c/61053)
2014-05-05 20:27 [C PATCH] Don't reject valid code with _Alignas (PR c/61053) Marek Polacek
@ 2014-05-05 20:31 ` Marek Polacek
2014-05-06 8:04 ` Richard Biener
` (2 subsequent siblings)
3 siblings, 0 replies; 13+ messages in thread
From: Marek Polacek @ 2014-05-05 20:31 UTC (permalink / raw)
To: GCC Patches; +Cc: Joseph S. Myers
On Mon, May 05, 2014 at 10:27:03PM +0200, Marek Polacek wrote:
> In this PR the issue is that we reject (valid) code such as
> _Alignas (long long) long long foo;
> with -m32, because we trip this condition:
>
> alignas_align = 1U << declspecs->align_log;
> if (alignas_align < TYPE_ALIGN_UNIT (type))
> {
> if (name)
> error_at (loc, "%<_Alignas%> specifiers cannot reduce "
> "alignment of %qE", name);
>
> and error later on, since alignas_align is 4 (correct, see PR52023 for
> why), but TYPE_ALIGN_UNIT of long long is 8. I think TYPE_ALIGN_UNIT
> is wrong here as that won't give us minimal alignment required.
> In c_sizeof_or_alignof_type we already have the code to compute such
> minimal alignment so I just moved the code to a separate function
> and used that instead of TYPE_ALIGN_UNIT.
>
> Note that the test is run only on i?86 and x86_64, because we can't (?)
> easily determine which target requires what alignment.
>
> Regtested/bootstrapped on x86_64-unknown-linux-gnu and
> powerpc64-unknown-linux-gnu, ok for trunk?
I should add that I checked what clang does on my testcase, and with this
patch both compilers accept and reject the same sort of testcases,
both with -m32 and -m64.
Marek
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [C PATCH] Don't reject valid code with _Alignas (PR c/61053)
2014-05-05 20:27 [C PATCH] Don't reject valid code with _Alignas (PR c/61053) Marek Polacek
2014-05-05 20:31 ` Marek Polacek
@ 2014-05-06 8:04 ` Richard Biener
2014-05-06 8:14 ` Jakub Jelinek
2014-05-07 17:15 ` Joseph S. Myers
2014-05-25 9:31 ` Marek Polacek
3 siblings, 1 reply; 13+ messages in thread
From: Richard Biener @ 2014-05-06 8:04 UTC (permalink / raw)
To: Marek Polacek; +Cc: GCC Patches, Joseph S. Myers
On Mon, May 5, 2014 at 10:27 PM, Marek Polacek <polacek@redhat.com> wrote:
> In this PR the issue is that we reject (valid) code such as
> _Alignas (long long) long long foo;
> with -m32, because we trip this condition:
>
> alignas_align = 1U << declspecs->align_log;
> if (alignas_align < TYPE_ALIGN_UNIT (type))
> {
> if (name)
> error_at (loc, "%<_Alignas%> specifiers cannot reduce "
> "alignment of %qE", name);
>
> and error later on, since alignas_align is 4 (correct, see PR52023 for
> why), but TYPE_ALIGN_UNIT of long long is 8. I think TYPE_ALIGN_UNIT
> is wrong here as that won't give us minimal alignment required.
> In c_sizeof_or_alignof_type we already have the code to compute such
> minimal alignment so I just moved the code to a separate function
> and used that instead of TYPE_ALIGN_UNIT.
Hmm, but isn't TYPE_ALIGN_UNIT wrong then?
Richard.
>
> Note that the test is run only on i?86 and x86_64, because we can't (?)
> easily determine which target requires what alignment.
>
> Regtested/bootstrapped on x86_64-unknown-linux-gnu and
> powerpc64-unknown-linux-gnu, ok for trunk?
>
> 2014-05-05 Marek Polacek <polacek@redhat.com>
>
> PR c/61053
> c-family/
> * c-common.c (min_align_of_type): New function factored out from...
> (c_sizeof_or_alignof_type): ...here.
> * c-common.h (min_align_of_type): Declare.
> c/
> * c-decl.c (grokdeclarator): Use min_align_of_type instead of
> TYPE_ALIGN_UNIT.
> testsuite/
> * gcc.dg/pr61053.c: New test.
>
> diff --git gcc/c-family/c-common.c gcc/c-family/c-common.c
> index 0ad955d..6d4440e 100644
> --- gcc/c-family/c-common.c
> +++ gcc/c-family/c-common.c
> @@ -4931,6 +4931,26 @@ c_common_get_alias_set (tree t)
> return -1;
> }
>
> +/* Return the least alignment required for type TYPE. */
> +
> +unsigned int
> +min_align_of_type (tree type)
> +{
> + unsigned int align = TYPE_ALIGN (type);
> + align = MIN (align, BIGGEST_ALIGNMENT);
> +#ifdef BIGGEST_FIELD_ALIGNMENT
> + align = MIN (align, BIGGEST_FIELD_ALIGNMENT);
> +#endif
> + unsigned int field_align = align;
> +#ifdef ADJUST_FIELD_ALIGN
> + tree field = build_decl (UNKNOWN_LOCATION, FIELD_DECL, NULL_TREE,
> + type);
> + field_align = ADJUST_FIELD_ALIGN (field, field_align);
> +#endif
> + align = MIN (align, field_align);
> + return align / BITS_PER_UNIT;
> +}
> +
> /* Compute the value of 'sizeof (TYPE)' or '__alignof__ (TYPE)', where
> the IS_SIZEOF parameter indicates which operator is being applied.
> The COMPLAIN flag controls whether we should diagnose possibly
> @@ -5009,21 +5029,7 @@ c_sizeof_or_alignof_type (location_t loc,
> size_int (TYPE_PRECISION (char_type_node)
> / BITS_PER_UNIT));
> else if (min_alignof)
> - {
> - unsigned int align = TYPE_ALIGN (type);
> - align = MIN (align, BIGGEST_ALIGNMENT);
> -#ifdef BIGGEST_FIELD_ALIGNMENT
> - align = MIN (align, BIGGEST_FIELD_ALIGNMENT);
> -#endif
> - unsigned int field_align = align;
> -#ifdef ADJUST_FIELD_ALIGN
> - tree field = build_decl (UNKNOWN_LOCATION, FIELD_DECL, NULL_TREE,
> - type);
> - field_align = ADJUST_FIELD_ALIGN (field, field_align);
> -#endif
> - align = MIN (align, field_align);
> - value = size_int (align / BITS_PER_UNIT);
> - }
> + value = size_int (min_align_of_type (type));
> else
> value = size_int (TYPE_ALIGN_UNIT (type));
> }
> diff --git gcc/c-family/c-common.h gcc/c-family/c-common.h
> index 57b7dce..d34d2bb 100644
> --- gcc/c-family/c-common.h
> +++ gcc/c-family/c-common.h
> @@ -758,6 +758,7 @@ extern tree c_wrap_maybe_const (tree, bool);
> extern tree c_save_expr (tree);
> extern tree c_common_truthvalue_conversion (location_t, tree);
> extern void c_apply_type_quals_to_decl (int, tree);
> +extern unsigned int min_align_of_type (tree);
> extern tree c_sizeof_or_alignof_type (location_t, tree, bool, bool, int);
> extern tree c_alignof_expr (location_t, tree);
> /* Print an error message for invalid operands to arith operation CODE.
> diff --git gcc/c/c-decl.c gcc/c/c-decl.c
> index 6e7c589..9f7419a 100644
> --- gcc/c/c-decl.c
> +++ gcc/c/c-decl.c
> @@ -5931,7 +5931,7 @@ grokdeclarator (const struct c_declarator *declarator,
> else if (declspecs->align_log != -1)
> {
> alignas_align = 1U << declspecs->align_log;
> - if (alignas_align < TYPE_ALIGN_UNIT (type))
> + if (alignas_align < min_align_of_type (type))
> {
> if (name)
> error_at (loc, "%<_Alignas%> specifiers cannot reduce "
> diff --git gcc/testsuite/gcc.dg/pr61053.c gcc/testsuite/gcc.dg/pr61053.c
> index e69de29..e3db534 100644
> --- gcc/testsuite/gcc.dg/pr61053.c
> +++ gcc/testsuite/gcc.dg/pr61053.c
> @@ -0,0 +1,75 @@
> +/* PR c/61053 */
> +/* { dg-do compile { target i?86-*-* x86_64-*-* } } */
> +/* { dg-options "-std=c11 -pedantic-errors" } */
> +
> +_Alignas (char) char cc;
> +_Alignas (short int) char cs;
> +_Alignas (int) char ci;
> +_Alignas (long int) char cl;
> +_Alignas (long long int) char cll;
> +_Alignas (float) char cf;
> +_Alignas (double) char cd;
> +_Alignas (long double) char cld;
> +
> +_Alignas (char) short int sc; /* { dg-error "cannot reduce alignment" } */
> +_Alignas (short int) short int ss;
> +_Alignas (int) short int si;
> +_Alignas (long int) short int sl;
> +_Alignas (long long int) short int sll;
> +_Alignas (float) short int sf;
> +_Alignas (double) short int sd;
> +_Alignas (long double) short int sld;
> +
> +_Alignas (char) int ic; /* { dg-error "cannot reduce alignment" } */
> +_Alignas (short int) int is; /* { dg-error "cannot reduce alignment" } */
> +_Alignas (int) int ii;
> +_Alignas (long int) int il;
> +_Alignas (long long int) int ill;
> +_Alignas (float) int if_;
> +_Alignas (double) int id;
> +_Alignas (long double) int ild;
> +
> +_Alignas (char) long int lic; /* { dg-error "cannot reduce alignment" } */
> +_Alignas (short int) long int lis; /* { dg-error "cannot reduce alignment" } */
> +_Alignas (int) long int lii; /* { dg-error "cannot reduce alignment" "" { target { lp64 } } } */
> +_Alignas (long int) long int lil;
> +_Alignas (long long int) long int lill;
> +_Alignas (float) long int lif; /* { dg-error "cannot reduce alignment" "" { target { lp64 } } } */
> +_Alignas (double) long int lid;
> +_Alignas (long double) long int lild;
> +
> +_Alignas (char) long long int llic; /* { dg-error "cannot reduce alignment" } */
> +_Alignas (short int) long long int llis; /* { dg-error "cannot reduce alignment" } */
> +_Alignas (int) long long int llii; /* { dg-error "cannot reduce alignment" "" { target { lp64 } } } */
> +_Alignas (long int) long long int llil;
> +_Alignas (long long int) long long int llill;
> +_Alignas (float) long long int llif; /* { dg-error "cannot reduce alignment" "" { target { lp64 } } } */
> +_Alignas (double) long long int llid;
> +_Alignas (long double) long long int llild;
> +
> +_Alignas (char) float fc; /* { dg-error "cannot reduce alignment" } */
> +_Alignas (short int) float fs; /* { dg-error "cannot reduce alignment" } */
> +_Alignas (int) float fi;
> +_Alignas (long int) float fl;
> +_Alignas (long long int) float fll;
> +_Alignas (float) float ff;
> +_Alignas (double) float fd;
> +_Alignas (long double) float fld;
> +
> +_Alignas (char) double dc; /* { dg-error "cannot reduce alignment" } */
> +_Alignas (short int) double ds; /* { dg-error "cannot reduce alignment" } */
> +_Alignas (int) double di; /* { dg-error "cannot reduce alignment" "" { target { lp64 } } } */
> +_Alignas (long int) double dl;
> +_Alignas (long long int) double dll;
> +_Alignas (float) double df; /* { dg-error "cannot reduce alignment" "" { target { lp64 } } } */
> +_Alignas (double) double dd;
> +_Alignas (long double) double dld;
> +
> +_Alignas (char) long double ldc; /* { dg-error "cannot reduce alignment" } */
> +_Alignas (short int) long double lds; /* { dg-error "cannot reduce alignment" } */
> +_Alignas (int) long double ldi; /* { dg-error "cannot reduce alignment" "" { target { lp64 } } } */
> +_Alignas (long int) long double ldl; /* { dg-error "cannot reduce alignment" "" { target { lp64 } } } */
> +_Alignas (long long int) long double ldll; /* { dg-error "cannot reduce alignment" "" { target { lp64 } } } */
> +_Alignas (float) long double ldf; /* { dg-error "cannot reduce alignment" "" { target { lp64 } } } */
> +_Alignas (double) long double ldd; /* { dg-error "cannot reduce alignment" "" { target { lp64 } } } */
> +_Alignas (long double) long double ldld;
>
> Marek
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [C PATCH] Don't reject valid code with _Alignas (PR c/61053)
2014-05-06 8:04 ` Richard Biener
@ 2014-05-06 8:14 ` Jakub Jelinek
2014-05-06 8:58 ` Marek Polacek
0 siblings, 1 reply; 13+ messages in thread
From: Jakub Jelinek @ 2014-05-06 8:14 UTC (permalink / raw)
To: Richard Biener; +Cc: Marek Polacek, GCC Patches, Joseph S. Myers
On Tue, May 06, 2014 at 10:04:33AM +0200, Richard Biener wrote:
> On Mon, May 5, 2014 at 10:27 PM, Marek Polacek <polacek@redhat.com> wrote:
> > In this PR the issue is that we reject (valid) code such as
> > _Alignas (long long) long long foo;
> > with -m32, because we trip this condition:
> >
> > alignas_align = 1U << declspecs->align_log;
> > if (alignas_align < TYPE_ALIGN_UNIT (type))
> > {
> > if (name)
> > error_at (loc, "%<_Alignas%> specifiers cannot reduce "
> > "alignment of %qE", name);
> >
> > and error later on, since alignas_align is 4 (correct, see PR52023 for
> > why), but TYPE_ALIGN_UNIT of long long is 8. I think TYPE_ALIGN_UNIT
> > is wrong here as that won't give us minimal alignment required.
> > In c_sizeof_or_alignof_type we already have the code to compute such
> > minimal alignment so I just moved the code to a separate function
> > and used that instead of TYPE_ALIGN_UNIT.
>
> Hmm, but isn't TYPE_ALIGN_UNIT wrong then?
No, after all, you don't want to change __alignof__ (long long), that is
pretty essential part of ABI.
Jakub
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [C PATCH] Don't reject valid code with _Alignas (PR c/61053)
2014-05-06 8:14 ` Jakub Jelinek
@ 2014-05-06 8:58 ` Marek Polacek
0 siblings, 0 replies; 13+ messages in thread
From: Marek Polacek @ 2014-05-06 8:58 UTC (permalink / raw)
To: Jakub Jelinek; +Cc: Richard Biener, GCC Patches, Joseph S. Myers
On Tue, May 06, 2014 at 10:14:42AM +0200, Jakub Jelinek wrote:
> > Hmm, but isn't TYPE_ALIGN_UNIT wrong then?
>
> No, after all, you don't want to change __alignof__ (long long), that is
> pretty essential part of ABI.
Yeah, as I understand things, for x86_64 __alignof__ (T) returns
*preferred* alignment which is specified for each type in the ABI
(Figure 3.1: Scalar Types), and is the same as sizeof (T). But
_Alignof (T) (thus also _Alignas (T)) is the *least* alignment for
a type -- and they can differ if the type is in a struct/union.
Please correct me if I'm mistaken.
Marek
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [C PATCH] Don't reject valid code with _Alignas (PR c/61053)
2014-05-05 20:27 [C PATCH] Don't reject valid code with _Alignas (PR c/61053) Marek Polacek
2014-05-05 20:31 ` Marek Polacek
2014-05-06 8:04 ` Richard Biener
@ 2014-05-07 17:15 ` Joseph S. Myers
2014-05-07 18:31 ` H.J. Lu
2014-05-25 9:31 ` Marek Polacek
3 siblings, 1 reply; 13+ messages in thread
From: Joseph S. Myers @ 2014-05-07 17:15 UTC (permalink / raw)
To: Marek Polacek; +Cc: GCC Patches
On Mon, 5 May 2014, Marek Polacek wrote:
> In this PR the issue is that we reject (valid) code such as
> _Alignas (long long) long long foo;
> with -m32, because we trip this condition:
>
> alignas_align = 1U << declspecs->align_log;
> if (alignas_align < TYPE_ALIGN_UNIT (type))
> {
> if (name)
> error_at (loc, "%<_Alignas%> specifiers cannot reduce "
> "alignment of %qE", name);
>
> and error later on, since alignas_align is 4 (correct, see PR52023 for
> why), but TYPE_ALIGN_UNIT of long long is 8. I think TYPE_ALIGN_UNIT
> is wrong here as that won't give us minimal alignment required.
> In c_sizeof_or_alignof_type we already have the code to compute such
> minimal alignment so I just moved the code to a separate function
> and used that instead of TYPE_ALIGN_UNIT.
>
> Note that the test is run only on i?86 and x86_64, because we can't (?)
> easily determine which target requires what alignment.
>
> Regtested/bootstrapped on x86_64-unknown-linux-gnu and
> powerpc64-unknown-linux-gnu, ok for trunk?
OK, though I'm not sure if the "lp64" conditions are right in the testcase
(i.e. if x32 has the same peculiarity as -m32 here, which is what's
implied by the use of "lp64").
--
Joseph S. Myers
joseph@codesourcery.com
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [C PATCH] Don't reject valid code with _Alignas (PR c/61053)
2014-05-07 17:15 ` Joseph S. Myers
@ 2014-05-07 18:31 ` H.J. Lu
2014-05-08 18:19 ` Marek Polacek
0 siblings, 1 reply; 13+ messages in thread
From: H.J. Lu @ 2014-05-07 18:31 UTC (permalink / raw)
To: Joseph S. Myers; +Cc: Marek Polacek, GCC Patches
On Wed, May 7, 2014 at 10:15 AM, Joseph S. Myers
<joseph@codesourcery.com> wrote:
> On Mon, 5 May 2014, Marek Polacek wrote:
>
>> In this PR the issue is that we reject (valid) code such as
>> _Alignas (long long) long long foo;
>> with -m32, because we trip this condition:
>>
>> alignas_align = 1U << declspecs->align_log;
>> if (alignas_align < TYPE_ALIGN_UNIT (type))
>> {
>> if (name)
>> error_at (loc, "%<_Alignas%> specifiers cannot reduce "
>> "alignment of %qE", name);
>>
>> and error later on, since alignas_align is 4 (correct, see PR52023 for
>> why), but TYPE_ALIGN_UNIT of long long is 8. I think TYPE_ALIGN_UNIT
>> is wrong here as that won't give us minimal alignment required.
>> In c_sizeof_or_alignof_type we already have the code to compute such
>> minimal alignment so I just moved the code to a separate function
>> and used that instead of TYPE_ALIGN_UNIT.
>>
>> Note that the test is run only on i?86 and x86_64, because we can't (?)
>> easily determine which target requires what alignment.
>>
>> Regtested/bootstrapped on x86_64-unknown-linux-gnu and
>> powerpc64-unknown-linux-gnu, ok for trunk?
>
> OK, though I'm not sure if the "lp64" conditions are right in the testcase
It should be !ia32 instead of lp64.
> (i.e. if x32 has the same peculiarity as -m32 here, which is what's
> implied by the use of "lp64").
>
Alignments of long long and long double on x32 are the same as x86-64.
--
H.J.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [C PATCH] Don't reject valid code with _Alignas (PR c/61053)
2014-05-07 18:31 ` H.J. Lu
@ 2014-05-08 18:19 ` Marek Polacek
2014-09-02 22:29 ` H.J. Lu
0 siblings, 1 reply; 13+ messages in thread
From: Marek Polacek @ 2014-05-08 18:19 UTC (permalink / raw)
To: H.J. Lu; +Cc: Joseph S. Myers, GCC Patches
On Wed, May 07, 2014 at 11:31:38AM -0700, H.J. Lu wrote:
> > OK, though I'm not sure if the "lp64" conditions are right in the testcase
>
> It should be !ia32 instead of lp64.
Ok, I changed lp64 to ! { ia32 } and committed the patch now.
Marek
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [C PATCH] Don't reject valid code with _Alignas (PR c/61053)
2014-05-05 20:27 [C PATCH] Don't reject valid code with _Alignas (PR c/61053) Marek Polacek
` (2 preceding siblings ...)
2014-05-07 17:15 ` Joseph S. Myers
@ 2014-05-25 9:31 ` Marek Polacek
2014-06-02 6:53 ` Marek Polacek
3 siblings, 1 reply; 13+ messages in thread
From: Marek Polacek @ 2014-05-25 9:31 UTC (permalink / raw)
To: GCC Patches; +Cc: Joseph S. Myers, Jakub Jelinek, Richard Biener
On Mon, May 05, 2014 at 10:27:03PM +0200, Marek Polacek wrote:
> In this PR the issue is that we reject (valid) code such as
> _Alignas (long long) long long foo;
> with -m32, because we trip this condition:
>
> alignas_align = 1U << declspecs->align_log;
> if (alignas_align < TYPE_ALIGN_UNIT (type))
> {
> if (name)
> error_at (loc, "%<_Alignas%> specifiers cannot reduce "
> "alignment of %qE", name);
>
> and error later on, since alignas_align is 4 (correct, see PR52023 for
> why), but TYPE_ALIGN_UNIT of long long is 8. I think TYPE_ALIGN_UNIT
> is wrong here as that won't give us minimal alignment required.
> In c_sizeof_or_alignof_type we already have the code to compute such
> minimal alignment so I just moved the code to a separate function
> and used that instead of TYPE_ALIGN_UNIT.
>
> Note that the test is run only on i?86 and x86_64, because we can't (?)
> easily determine which target requires what alignment.
>
> Regtested/bootstrapped on x86_64-unknown-linux-gnu and
> powerpc64-unknown-linux-gnu, ok for trunk?
Can I backport this one to 4.9?
Marek
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [C PATCH] Don't reject valid code with _Alignas (PR c/61053)
2014-05-25 9:31 ` Marek Polacek
@ 2014-06-02 6:53 ` Marek Polacek
2014-06-04 7:17 ` Jeff Law
0 siblings, 1 reply; 13+ messages in thread
From: Marek Polacek @ 2014-06-02 6:53 UTC (permalink / raw)
To: GCC Patches; +Cc: Joseph S. Myers, Jakub Jelinek, Richard Biener
Ping.
On Sun, May 25, 2014 at 11:30:51AM +0200, Marek Polacek wrote:
> On Mon, May 05, 2014 at 10:27:03PM +0200, Marek Polacek wrote:
> > In this PR the issue is that we reject (valid) code such as
> > _Alignas (long long) long long foo;
> > with -m32, because we trip this condition:
> >
> > alignas_align = 1U << declspecs->align_log;
> > if (alignas_align < TYPE_ALIGN_UNIT (type))
> > {
> > if (name)
> > error_at (loc, "%<_Alignas%> specifiers cannot reduce "
> > "alignment of %qE", name);
> >
> > and error later on, since alignas_align is 4 (correct, see PR52023 for
> > why), but TYPE_ALIGN_UNIT of long long is 8. I think TYPE_ALIGN_UNIT
> > is wrong here as that won't give us minimal alignment required.
> > In c_sizeof_or_alignof_type we already have the code to compute such
> > minimal alignment so I just moved the code to a separate function
> > and used that instead of TYPE_ALIGN_UNIT.
> >
> > Note that the test is run only on i?86 and x86_64, because we can't (?)
> > easily determine which target requires what alignment.
> >
> > Regtested/bootstrapped on x86_64-unknown-linux-gnu and
> > powerpc64-unknown-linux-gnu, ok for trunk?
>
> Can I backport this one to 4.9?
Marek
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [C PATCH] Don't reject valid code with _Alignas (PR c/61053)
2014-06-02 6:53 ` Marek Polacek
@ 2014-06-04 7:17 ` Jeff Law
0 siblings, 0 replies; 13+ messages in thread
From: Jeff Law @ 2014-06-04 7:17 UTC (permalink / raw)
To: Marek Polacek, GCC Patches; +Cc: Joseph S. Myers, Jakub Jelinek, Richard Biener
On 06/02/14 00:52, Marek Polacek wrote:
> Ping.
>
> On Sun, May 25, 2014 at 11:30:51AM +0200, Marek Polacek wrote:
>> On Mon, May 05, 2014 at 10:27:03PM +0200, Marek Polacek wrote:
>>> In this PR the issue is that we reject (valid) code such as
>>> _Alignas (long long) long long foo;
>>> with -m32, because we trip this condition:
>>>
>>> alignas_align = 1U << declspecs->align_log;
>>> if (alignas_align < TYPE_ALIGN_UNIT (type))
>>> {
>>> if (name)
>>> error_at (loc, "%<_Alignas%> specifiers cannot reduce "
>>> "alignment of %qE", name);
>>>
>>> and error later on, since alignas_align is 4 (correct, see PR52023 for
>>> why), but TYPE_ALIGN_UNIT of long long is 8. I think TYPE_ALIGN_UNIT
>>> is wrong here as that won't give us minimal alignment required.
>>> In c_sizeof_or_alignof_type we already have the code to compute such
>>> minimal alignment so I just moved the code to a separate function
>>> and used that instead of TYPE_ALIGN_UNIT.
>>>
>>> Note that the test is run only on i?86 and x86_64, because we can't (?)
>>> easily determine which target requires what alignment.
>>>
>>> Regtested/bootstrapped on x86_64-unknown-linux-gnu and
>>> powerpc64-unknown-linux-gnu, ok for trunk?
>>
>> Can I backport this one to 4.9?
Yes, backporting to 4.9 is fine.
jeff
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [C PATCH] Don't reject valid code with _Alignas (PR c/61053)
2014-05-08 18:19 ` Marek Polacek
@ 2014-09-02 22:29 ` H.J. Lu
2014-09-02 22:39 ` H.J. Lu
0 siblings, 1 reply; 13+ messages in thread
From: H.J. Lu @ 2014-09-02 22:29 UTC (permalink / raw)
To: Marek Polacek; +Cc: Joseph S. Myers, GCC Patches
On Thu, May 8, 2014 at 11:19 AM, Marek Polacek <polacek@redhat.com> wrote:
> On Wed, May 07, 2014 at 11:31:38AM -0700, H.J. Lu wrote:
>> > OK, though I'm not sure if the "lp64" conditions are right in the testcase
>>
>> It should be !ia32 instead of lp64.
>
> Ok, I changed lp64 to ! { ia32 } and committed the patch now.
>
> Marek
The change is insufficient for x32, which has the same alignments
for floating point types and the integer types with the same size as
x86-64. This patch is needed for x32. OK for trunk and 4.8 branch?
--
H.J.
---
2014-09-02 H.J. Lu <hongjiu.lu@intel.com>
* gcc.dg/pr61053.c: Updated for x32.
diff --git a/gcc/testsuite/gcc.dg/pr61053.c b/gcc/testsuite/gcc.dg/pr61053.c
index 4fd5319..5557784 100644
--- a/gcc/testsuite/gcc.dg/pr61053.c
+++ b/gcc/testsuite/gcc.dg/pr61053.c
@@ -31,17 +31,17 @@ _Alignas (long double) int ild;
_Alignas (char) long int lic; /* { dg-error "cannot reduce alignment" } */
_Alignas (short int) long int lis; /* { dg-error "cannot reduce alignment" } */
-_Alignas (int) long int lii; /* { dg-error "cannot reduce alignment"
"" { target { ! { ia32 } } } } */
+_Alignas (int) long int lii; /* { dg-error "cannot reduce alignment"
"" { target { ! { ia32 || x32 } } } } */
_Alignas (long int) long int lil;
_Alignas (long long int) long int lill;
-_Alignas (float) long int lif; /* { dg-error "cannot reduce
alignment" "" { target { ! { ia32 } } } } */
+_Alignas (float) long int lif; /* { dg-error "cannot reduce
alignment" "" { target { ! { ia32 || x32 } } } } */
_Alignas (double) long int lid;
_Alignas (long double) long int lild;
_Alignas (char) long long int llic; /* { dg-error "cannot reduce
alignment" } */
_Alignas (short int) long long int llis; /* { dg-error "cannot reduce
alignment" } */
_Alignas (int) long long int llii; /* { dg-error "cannot reduce
alignment" "" { target { ! { ia32 } } } } */
-_Alignas (long int) long long int llil;
+_Alignas (long int) long long int llil; /* { dg-error "cannot reduce
alignment" "" { target { x32 } } } */
_Alignas (long long int) long long int llill;
_Alignas (float) long long int llif; /* { dg-error "cannot reduce
alignment" "" { target { ! { ia32 } } } } */
_Alignas (double) long long int llid;
@@ -59,7 +59,7 @@ _Alignas (long double) float fld;
_Alignas (char) double dc; /* { dg-error "cannot reduce alignment" } */
_Alignas (short int) double ds; /* { dg-error "cannot reduce alignment" } */
_Alignas (int) double di; /* { dg-error "cannot reduce alignment" ""
{ target { ! { ia32 } } } } */
-_Alignas (long int) double dl;
+_Alignas (long int) double dl; /* { dg-error "cannot reduce
alignment" "" { target { x32 } } } */
_Alignas (long long int) double dll;
_Alignas (float) double df; /* { dg-error "cannot reduce alignment"
"" { target { ! { ia32 } } } } */
_Alignas (double) double dd;
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [C PATCH] Don't reject valid code with _Alignas (PR c/61053)
2014-09-02 22:29 ` H.J. Lu
@ 2014-09-02 22:39 ` H.J. Lu
0 siblings, 0 replies; 13+ messages in thread
From: H.J. Lu @ 2014-09-02 22:39 UTC (permalink / raw)
To: Marek Polacek; +Cc: Joseph S. Myers, GCC Patches
[-- Attachment #1: Type: text/plain, Size: 790 bytes --]
On Tue, Sep 2, 2014 at 3:29 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Thu, May 8, 2014 at 11:19 AM, Marek Polacek <polacek@redhat.com> wrote:
>> On Wed, May 07, 2014 at 11:31:38AM -0700, H.J. Lu wrote:
>>> > OK, though I'm not sure if the "lp64" conditions are right in the testcase
>>>
>>> It should be !ia32 instead of lp64.
>>
>> Ok, I changed lp64 to ! { ia32 } and committed the patch now.
>>
>> Marek
>
> The change is insufficient for x32, which has the same alignments
> for floating point types and the integer types with the same size as
> x86-64. This patch is needed for x32. OK for trunk and 4.8 branch?
>
>
> --
> H.J.
> ---
> 2014-09-02 H.J. Lu <hongjiu.lu@intel.com>
>
> * gcc.dg/pr61053.c: Updated for x32.
>
Here is the patch as an attachment.
--
H.J.
[-- Attachment #2: align.patch --]
[-- Type: text/x-patch, Size: 2292 bytes --]
2014-09-02 H.J. Lu <hongjiu.lu@intel.com>
* gcc.dg/pr61053.c: Updated for x32.
diff --git a/gcc/testsuite/gcc.dg/pr61053.c b/gcc/testsuite/gcc.dg/pr61053.c
index 4fd5319..5557784 100644
--- a/gcc/testsuite/gcc.dg/pr61053.c
+++ b/gcc/testsuite/gcc.dg/pr61053.c
@@ -31,17 +31,17 @@ _Alignas (long double) int ild;
_Alignas (char) long int lic; /* { dg-error "cannot reduce alignment" } */
_Alignas (short int) long int lis; /* { dg-error "cannot reduce alignment" } */
-_Alignas (int) long int lii; /* { dg-error "cannot reduce alignment" "" { target { ! { ia32 } } } } */
+_Alignas (int) long int lii; /* { dg-error "cannot reduce alignment" "" { target { ! { ia32 || x32 } } } } */
_Alignas (long int) long int lil;
_Alignas (long long int) long int lill;
-_Alignas (float) long int lif; /* { dg-error "cannot reduce alignment" "" { target { ! { ia32 } } } } */
+_Alignas (float) long int lif; /* { dg-error "cannot reduce alignment" "" { target { ! { ia32 || x32 } } } } */
_Alignas (double) long int lid;
_Alignas (long double) long int lild;
_Alignas (char) long long int llic; /* { dg-error "cannot reduce alignment" } */
_Alignas (short int) long long int llis; /* { dg-error "cannot reduce alignment" } */
_Alignas (int) long long int llii; /* { dg-error "cannot reduce alignment" "" { target { ! { ia32 } } } } */
-_Alignas (long int) long long int llil;
+_Alignas (long int) long long int llil; /* { dg-error "cannot reduce alignment" "" { target { x32 } } } */
_Alignas (long long int) long long int llill;
_Alignas (float) long long int llif; /* { dg-error "cannot reduce alignment" "" { target { ! { ia32 } } } } */
_Alignas (double) long long int llid;
@@ -59,7 +59,7 @@ _Alignas (long double) float fld;
_Alignas (char) double dc; /* { dg-error "cannot reduce alignment" } */
_Alignas (short int) double ds; /* { dg-error "cannot reduce alignment" } */
_Alignas (int) double di; /* { dg-error "cannot reduce alignment" "" { target { ! { ia32 } } } } */
-_Alignas (long int) double dl;
+_Alignas (long int) double dl; /* { dg-error "cannot reduce alignment" "" { target { x32 } } } */
_Alignas (long long int) double dll;
_Alignas (float) double df; /* { dg-error "cannot reduce alignment" "" { target { ! { ia32 } } } } */
_Alignas (double) double dd;
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2014-09-02 22:39 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-05-05 20:27 [C PATCH] Don't reject valid code with _Alignas (PR c/61053) Marek Polacek
2014-05-05 20:31 ` Marek Polacek
2014-05-06 8:04 ` Richard Biener
2014-05-06 8:14 ` Jakub Jelinek
2014-05-06 8:58 ` Marek Polacek
2014-05-07 17:15 ` Joseph S. Myers
2014-05-07 18:31 ` H.J. Lu
2014-05-08 18:19 ` Marek Polacek
2014-09-02 22:29 ` H.J. Lu
2014-09-02 22:39 ` H.J. Lu
2014-05-25 9:31 ` Marek Polacek
2014-06-02 6:53 ` Marek Polacek
2014-06-04 7:17 ` Jeff Law
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).