Richard Biener <rguent...@suse.de> writes:
> On Fri, 25 Sep 2020, Richard Sandiford wrote:
>
>> Richard Biener <rguent...@suse.de> writes:
>> >> What do we allow for non-boolean constructors. E.g. for:
>> >>
>> >> v2hi = 0xf001;
>> >>
>> >> do we allow the CONSTRUCTOR to be { 0xf001 }? Is the type of an
>> >> initialiser value allowed to be arbitrarily different from the type
>> >> of the elements being initialised?
>> >>
>> >> Or is there requirement that (say) each constructor element is either:
>> >>
>> >> - a scalar that initialises one element of the constructed vector
>> >> - a vector of N elements that initialises N elements of the constructed
>> >> vector
>> >>
>> >> ?
>> >>
>> >> Like you say, it mostly seems like guesswork how booleans would be
>> >> handled here, but personally I don't know the answer for non-booleans
>> >> either :-)
>> >
>> > There's extensive checking in tree-cfg.c for vector CTORs meanwhile.
>> > We only supporm uniform element CTORs with only trailing zeros elided.
>> > And the elements need to either have types of the vector component
>> > or be vectors with such component.
>>
>> Ah, great. So in that case, could we ditch bitsize altogether and
>> just use:
>>
>> unsigned int nelts = (VECTOR_TYPE_P (val_type)
>> ? TYPE_VECTOR_SUBPARTS (val_type).to_constant () : 1);
>>
>> or equivalent to work out the number of elements being initialised
>> by each constructor element?
>
> But
>
> store_constructor_field (target, bitsize, bitpos, 0,
> bitregion_end, value_mode,
> value, cleared, alias, reverse);
>
> still wants the bits to initialize (for the original testcase
> the vector<bool:1> had only the first 4 elements initialized,
> at wrong bit positions and sizes - QImode).
>
> But yes, I'm sure we can eventually simplify this further.
> FYI, the following passed bootstrap, regtest is still running
> (but as said, test coverage dropped to zero).
LGTM FWIW.
Thanks,
Richard
>
> Richard.
>
> commit d16b5975ca985cbe97698479fc38b6a636886978
> Author: Richard Biener <rguent...@suse.de>
> Date: Fri Sep 25 11:13:13 2020 +0200
>
> middle-end/96814 - fix VECTOR_BOOLEAN_TYPE_P CTOR RTL expansion
>
> The RTL expansion code for CTORs doesn't handle VECTOR_BOOLEAN_TYPE_P
> with bit-precision elements correctly as the testcase shows before
> the PR97085 fix. The following makes it do the correct thing
> (not 100% sure for CTOR of sub-vectors due to the lack of a testcase).
>
> The alternative would be to assert such CTORs do not happen (and also
> add IL verification for this).
>
> The GIMPLE FE needs a way to declare the VECTOR_BOOLEAN_TYPE_P vectors
> (thus the C FE needs that).
>
> 2020-09-25 Richard Biener <rguent...@suse.de>
>
> PR middle-end/96814
> * expr.c (store_constructor): Handle VECTOR_BOOLEAN_TYPE_P
> CTORs correctly.
>
> * gcc.target/i386/pr96814.c: New testcase.
>
> diff --git a/gcc/expr.c b/gcc/expr.c
> index 1a15f24b397..1c79518ee4d 100644
> --- a/gcc/expr.c
> +++ b/gcc/expr.c
> @@ -6922,7 +6922,7 @@ store_constructor (tree exp, rtx target, int cleared,
> poly_int64 size,
> insn_code icode = CODE_FOR_nothing;
> tree elt;
> tree elttype = TREE_TYPE (type);
> - int elt_size = tree_to_uhwi (TYPE_SIZE (elttype));
> + int elt_size = vector_element_bits (type);
> machine_mode eltmode = TYPE_MODE (elttype);
> HOST_WIDE_INT bitsize;
> HOST_WIDE_INT bitpos;
> @@ -6987,6 +6987,15 @@ store_constructor (tree exp, rtx target, int cleared,
> poly_int64 size,
> }
> }
>
> + /* Compute the size of the elements in the CTOR. It differs
> + from the size of the vector type elements only when the
> + CTOR elements are vectors themselves. */
> + tree val_type = TREE_TYPE (CONSTRUCTOR_ELT (exp, 0)->value);
> + if (VECTOR_TYPE_P (val_type))
> + bitsize = tree_to_uhwi (TYPE_SIZE (val_type));
> + else
> + bitsize = elt_size;
> +
> /* If the constructor has fewer elements than the vector,
> clear the whole array first. Similarly if this is static
> constructor of a non-BLKmode object. */
> @@ -7001,11 +7010,7 @@ store_constructor (tree exp, rtx target, int cleared,
> poly_int64 size,
>
> FOR_EACH_CONSTRUCTOR_VALUE (CONSTRUCTOR_ELTS (exp), idx, value)
> {
> - tree sz = TYPE_SIZE (TREE_TYPE (value));
> - int n_elts_here
> - = tree_to_uhwi (int_const_binop (TRUNC_DIV_EXPR, sz,
> - TYPE_SIZE (elttype)));
> -
> + int n_elts_here = bitsize / elt_size;
> count += n_elts_here;
> if (mostly_zeros_p (value))
> zero_count += n_elts_here;
> @@ -7045,7 +7050,6 @@ store_constructor (tree exp, rtx target, int cleared,
> poly_int64 size,
> HOST_WIDE_INT eltpos;
> tree value = ce->value;
>
> - bitsize = tree_to_uhwi (TYPE_SIZE (TREE_TYPE (value)));
> if (cleared && initializer_zerop (value))
> continue;
>
> diff --git a/gcc/testsuite/gcc.target/i386/pr96814.c
> b/gcc/testsuite/gcc.target/i386/pr96814.c
> new file mode 100644
> index 00000000000..b280c737130
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/i386/pr96814.c
> @@ -0,0 +1,19 @@
> +/* { dg-do run } */
> +/* { dg-options "-mavx512vl -mavx512bw" } */
> +/* { dg-require-effective-target avx512bw } */
> +/* { dg-require-effective-target avx512vl } */
> +
> +typedef unsigned char __attribute__ ((__vector_size__ (32))) V;
> +
> +void
> +test (void)
> +{
> + V x = ((V){8} > 0) == 0;
> + for (unsigned i = 0; i < sizeof (x); i++)
> + if (x[i] != (i ? 0xff : 0)) __builtin_abort();
> +}
> +
> +#define DO_TEST test
> +#define AVX512VL
> +#define AVX512BW
> +#include "avx512-check.h"
