On Mon, 16 Mar 2020 at 19:59, Richard Sandiford
<richard.sandif...@arm.com> wrote:
>
> Christophe Lyon via Gcc-patches <gcc-patches@gcc.gnu.org> writes:
> > On Wed, 11 Mar 2020 at 00:37, Joseph Myers <jos...@codesourcery.com> wrote:
> >>
> >> On Tue, 10 Mar 2020, Christophe Lyon wrote:
> >>
> >> > sizeless-1.c and sizeless-2.c have the same code, but the latter is
> >> > compiled with -msve-vector-bits=256 and expects different
> >> > warnings/errors.
> >> > For line 33:
> >> > svint8_t *invalid_sve_sc_ptr = &(svint8_t) { *global_sve_sc_ptr };
> >> > we now have:
> >> > sizeless-1.c:33:44: error: empty scalar initializer
> >> > sizeless-1.c:33:44: note: (near initialization for '(anonymous)')
> >> > and
> >> > sizeless-2.c:33:44: error: initializer element is not constant
> >> > sizeless-2.c:33:44: note: (near initialization for 'invalid_sve_sc_ptr')
> >> > sizeless-2.c:33:44: error: SVE type 'svint8_t' does not have a fixed size
> >> > so I think the error comes from the compound literal being treated
> >> > differently with -msve-vector-bits=256
> >>
> >> I think the sizeless-2.c diagnostics are correct while there's a problem
> >> in the sizeless-1.c case (the initializer is not empty, so it should not
> >> be diagnosed as such).
> >>
> >> Does the process_init_element code
> >>
> >> /* Ignore elements of an initializer for a variable-size type.
> >> Those are diagnosed in digest_init. */
> >> if (COMPLETE_TYPE_P (constructor_type)
> >> && TREE_CODE (TYPE_SIZE (constructor_type)) != INTEGER_CST)
> >> return;
> >>
> >> fire for the sizeless-1.c case? If so, maybe it needs to be restricted in
> >> some way to apply only to variable size structs / unions / arrays rather
> >> than whatever kind of variable-size type the SVE types are.
> >
> > It does. Type_size has POLY_INT_CST type.
> >
> > The attached small patch fixes the problem (tested on arm and aarch64).
> > OK?
>
> Thanks for doing this. I'd wondered whether it should be based on
> this or on VECTOR_TYPE_P, but FWIW, I agree basing it on this is best.
>
> > diff --git a/gcc/c/c-typeck.c b/gcc/c/c-typeck.c
> > index d8025de..1302486 100644
> > --- a/gcc/c/c-typeck.c
> > +++ b/gcc/c/c-typeck.c
> > @@ -9968,7 +9968,8 @@ process_init_element (location_t loc, struct
> > c_expr value, bool implicit,
> > /* Ignore elements of an initializer for a variable-size type.
> > Those are diagnosed in digest_init. */
> > if (COMPLETE_TYPE_P (constructor_type)
> > - && TREE_CODE (TYPE_SIZE (constructor_type)) != INTEGER_CST)
> > + && TREE_CODE (TYPE_SIZE (constructor_type)) != INTEGER_CST
> > + && TREE_CODE (TYPE_SIZE (constructor_type)) != POLY_INT_CST)
>
> Not worth respinning over, but since it hasn't been applied yet:
> a shorter alternative is to replace the != INTEGER_CST test with
> !poly_int_tree_p.
>
> > diff --git
> > a/gcc/testsuite/gcc.target/aarch64/sve/acle/general-c/sizeless-1.c
> > b/gcc/testsuite/gcc.target/aarch64/sve/acle/general-c/sizeless-1.c
> > index ec892a3..e53b871 100644
> > --- a/gcc/testsuite/gcc.target/aarch64/sve/acle/general-c/sizeless-1.c
> > +++ b/gcc/testsuite/gcc.target/aarch64/sve/acle/general-c/sizeless-1.c
> > @@ -83,7 +83,6 @@ statements (int n)
> > svint8_t array[2]; /* { dg-error {array elements cannot have SVE
> > type 'svint8_t'} } */
> > svint8_t zero_length_array[0]; /* { dg-error {array elements cannot
> > have SVE type 'svint8_t'} } */
> > svint8_t empty_init_array[] = {}; /* { dg-error {array elements
> > cannot have SVE type 'svint8_t'} } */
> > - /* { dg-error {empty scalar
> > initializer} "" { target *-*-* } .-1 } */
> > typedef svint8_t vla_type[n]; /* { dg-error {array elements cannot
> > have SVE type 'svint8_t'} } */
> >
> > /* Assignment. */
> > diff --git
> > a/gcc/testsuite/gcc.target/aarch64/sve/acle/general-c/sizeless-2.c
> > b/gcc/testsuite/gcc.target/aarch64/sve/acle/general-c/sizeless-2.c
> > index 7174393..9986d27 100644
> > --- a/gcc/testsuite/gcc.target/aarch64/sve/acle/general-c/sizeless-2.c
> > +++ b/gcc/testsuite/gcc.target/aarch64/sve/acle/general-c/sizeless-2.c
> > @@ -83,7 +83,6 @@ statements (int n)
> > svint8_t array[2]; /* { dg-error {array elements cannot have SVE
> > type 'svint8_t'} } */
> > svint8_t zero_length_array[0]; /* { dg-error {array elements cannot
> > have SVE type 'svint8_t'} } */
> > svint8_t empty_init_array[] = {}; /* { dg-error {array elements
> > cannot have SVE type 'svint8_t'} } */
> > - /* { dg-error {empty scalar
> > initializer} "" { target *-*-* } .-1 } */
> > typedef svint8_t vla_type[n]; /* { dg-error {array elements cannot
> > have SVE type 'svint8_t'} } */
> >
> > /* Assignment. */
>
> Jeff pointed out off-list that using:
>
> N: ... /* { dg-error {...} } */
> N+1: /* { dg-error {...} "" { target *-*-* } .-1 } */
>
> led to two identical test names for line N. This was causing the
> results to fluctuate when using contrib/compare_tests (which I admit
> I don't do, so hadn't noticed). Your patch fixes the cases that
> mattered, but for future-proofing reasons, this patch adds proper
> test names for the remaining instances.
>
Thanks.
Just checked, there are many more testcases with duplicate "names"
(266 under gcc.target/aarch64 only) :-(
Sigh...
> Tested on aarch64-linux-gnu and committed as obvious.
>
> Richard
>
>
> 2020-03-16 Richard Sandiford <richard.sandif...@arm.com>
>
> gcc/testsuite/
> * gcc.target/aarch64/sve/acle/general-c/sizeless-1.c: Add a test
> name to .-1 dg-error tests.
> * gcc.target/aarch64/sve/acle/general-c/sizeless-2.c: Likewise.
> ---
> .../gcc.target/aarch64/sve/acle/general-c/sizeless-1.c | 2 +-
> .../gcc.target/aarch64/sve/acle/general-c/sizeless-2.c | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/gcc/testsuite/gcc.target/aarch64/sve/acle/general-c/sizeless-1.c
> b/gcc/testsuite/gcc.target/aarch64/sve/acle/general-c/sizeless-1.c
> index ec892a3fc83..045963d5c76 100644
> --- a/gcc/testsuite/gcc.target/aarch64/sve/acle/general-c/sizeless-1.c
> +++ b/gcc/testsuite/gcc.target/aarch64/sve/acle/general-c/sizeless-1.c
> @@ -31,7 +31,7 @@ union union1 {
>
> svint8_t *global_sve_sc_ptr;
> svint8_t *invalid_sve_sc_ptr = &(svint8_t) { *global_sve_sc_ptr }; /* {
> dg-error {initializer element is not constant} } */
> - /* { dg-error {SVE type 'svint8_t' does not have a fixed size} "" { target
> *-*-* } .-1 } */
> + /* { dg-error {SVE type 'svint8_t' does not have a fixed size} "2nd line"
> { target *-*-* } .-1 } */
>
> /* Sizeless arguments and return values. */
>
> diff --git a/gcc/testsuite/gcc.target/aarch64/sve/acle/general-c/sizeless-2.c
> b/gcc/testsuite/gcc.target/aarch64/sve/acle/general-c/sizeless-2.c
> index 71743930098..c7282faba46 100644
> --- a/gcc/testsuite/gcc.target/aarch64/sve/acle/general-c/sizeless-2.c
> +++ b/gcc/testsuite/gcc.target/aarch64/sve/acle/general-c/sizeless-2.c
> @@ -31,7 +31,7 @@ union union1 {
>
> svint8_t *global_sve_sc_ptr;
> svint8_t *invalid_sve_sc_ptr = &(svint8_t) { *global_sve_sc_ptr }; /* {
> dg-error {initializer element is not constant} } */
> - /* { dg-error {SVE type 'svint8_t' does not have a fixed size} "" { target
> *-*-* } .-1 } */
> + /* { dg-error {SVE type 'svint8_t' does not have a fixed size} "2nd line"
> { target *-*-* } .-1 } */
>
> /* Sizeless arguments and return values. */
>
