Re: c: ignore initializers for elements of variable-size types [PR93577]

[Richard Sandiford <richard.sandiford@arm.com>]

Christophe Lyon via Gcc-patches <gcc-patches@gcc.gnu.org> writes:
> On Wed, 11 Mar 2020 at 00:37, Joseph Myers <joseph@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.

Tested on aarch64-linux-gnu and committed as obvious.

Richard


2020-03-16  Richard Sandiford  <richard.sandiford@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.  */