On Fri, 25 Sep 2020, Richard Sandiford wrote:
> Richard Biener <rguent...@suse.de> writes:
> > 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), thus test coverage is quite limited (zero)
> > now and I didn't manage to convince GCC to create such CTOR for SVE
> > VnBImode vectors.
> >
> > Bootstrapped and tested on x86_64-unknown-linux-gnu.
> >
> > Does this look sensible?
> >
> > Thanks,
> > Richard.
> >
> > 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.
> > ---
> > gcc/expr.c | 28 ++++++++++++++++++-------
> > gcc/testsuite/gcc.target/i386/pr96814.c | 19 +++++++++++++++++
> > 2 files changed, 40 insertions(+), 7 deletions(-)
> > create mode 100644 gcc/testsuite/gcc.target/i386/pr96814.c
> >
> > diff --git a/gcc/expr.c b/gcc/expr.c
> > index 1a15f24b397..fb42e485089 100644
> > --- a/gcc/expr.c
> > +++ b/gcc/expr.c
> > @@ -6922,7 +6922,9 @@ 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_BOOLEAN_TYPE_P (type) ? TYPE_PRECISION (elttype)
> > + : tree_to_uhwi (TYPE_SIZE (elttype)));
>
> FWIW, we now have vector_element_bits for this.
ah, didn't know this
> > machine_mode eltmode = TYPE_MODE (elttype);
> > HOST_WIDE_INT bitsize;
> > HOST_WIDE_INT bitpos;
> > @@ -6987,6 +6989,23 @@ store_constructor (tree exp, rtx target, int
> > cleared, poly_int64 size,
> > }
> > }
> >
> > + /* Compute the size of the elements in the CTOR. */
> > + tree val_type = TREE_TYPE (CONSTRUCTOR_ELT (exp, 0)->value);
> > + if (VECTOR_BOOLEAN_TYPE_P (type))
> > + {
> > + if (VECTOR_TYPE_P (val_type))
> > + {
> > + /* ??? Never seen such beast, but it's not disallowed. */
> > + gcc_assert (VECTOR_BOOLEAN_TYPE_P (val_type));
> > + bitsize = (TYPE_PRECISION (TREE_TYPE (val_type))
> > + * TYPE_VECTOR_SUBPARTS (val_type).to_constant ());
but I wonder whether it is correct? Say, for AVX512 which uses
at least 'char' as type TYPE_SIZE of that will likely be 8 so for
a hyphotetical 4 element mask it would need two-bit elements
to work out but IIRC AVX512 mask registers always use 1 bit per lane.
The target hook currently does
static opt_machine_mode
ix86_get_mask_mode (machine_mode data_mode)
{
unsigned vector_size = GET_MODE_SIZE (data_mode);
unsigned nunits = GET_MODE_NUNITS (data_mode);
unsigned elem_size = vector_size / nunits;
/* Scalar mask case. */
if ((TARGET_AVX512F && vector_size == 64)
|| (TARGET_AVX512VL && (vector_size == 32 || vector_size == 16)))
{
if (elem_size == 4
|| elem_size == 8
|| (TARGET_AVX512BW && (elem_size == 1 || elem_size == 2)))
return smallest_int_mode_for_size (nunits);
}
and then build_truth_vector_type_for_mode will end up building
a vector<bool:2> with QImode I think. So it works but I now
wonder whether it works correctly ;)
I guess I will rework the above hunk to use the computed element
size for the non-vector element case and rely on TYPE_SIZE for
the vector element case since that's what vector_element_size does.
Simplifies the beast a bit.
> > + }
> > + else
> > + bitsize = TYPE_PRECISION (val_type);
> > + }
> > + else
> > + bitsize = tree_to_uhwi (TYPE_SIZE (val_type));
> > +
>
> 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.
Thanks,
Richard.
