Re: Add VEC_DUPLICATE_{CST,EXPR} and associated optab

[Richard Sandiford <richard dot sandiford at linaro dot org>]

Richard Biener <richard.guenther@gmail.com> writes:
> On Mon, Sep 25, 2017 at 1:08 PM, Richard Sandiford
> <richard.sandiford@linaro.org> wrote:
>> SVE needs a way of broadcasting a scalar to a variable-length vector.
>> This patch adds VEC_DUPLICATE_CST for when VECTOR_CST would be used for
>> fixed-length vectors and VEC_DUPLICATE_EXPR for when CONSTRUCTOR would
>> be used for fixed-length vectors.  VEC_DUPLICATE_EXPR is the tree
>> equivalent of the existing rtl code VEC_DUPLICATE.
>>
>> Originally we had a single VEC_DUPLICATE_EXPR and used TREE_CONSTANT
>> to mark constant nodes, but in response to last year's RFC, Richard B.
>> suggested it would be better to have separate codes for the constant
>> and non-constant cases.  This allows VEC_DUPLICATE_EXPR to be treated
>> as a normal unary operation and avoids the previous need for treating
>> it as a GIMPLE_SINGLE_RHS.
>>
>> It might make sense to use VEC_DUPLICATE_CST for all duplicated
>> vector constants, since it's a bit more compact than VECTOR_CST
>> in that case, and is potentially more efficient to process.  I don't
>> have any specific plans to do that though.  We'll need to keep both
>> types of constant around whatever happens.
>
> I think VEC_DUPLICATE_EXPR is a good thing to have.  Looking at the
> changelog you didn't patch build_vector_from_val to make use of either
> new tree code?  That would get you (quite) some testing coverage.

I didn't want to change the use of VECTOR_CST and CONSTRUCTOR for
fixed-length vectors since that would be another invasive change,
and wouldn't remove the need for supporting VECTOR_CST and CONSTRUCTOR
in all the places that currently handle it.  I think it would make sense
to do it only after the variable-length support has settled.

The SVE patches do make build_vector_from_val use these codes
for variable-length vectors:

  if (!TYPE_VECTOR_SUBPARTS (vectype).is_constant (&nunits))
    {
      if (CONSTANT_CLASS_P (sc))
       return build_vec_duplicate_cst (vectype, sc);
      return fold_build1 (VEC_DUPLICATE_EXPR, vectype, sc);
    }

> Currently we require all elements of a VECTOR_CST to be present -- how
> difficult would it be to declare that iff and if only the first
> element is present then all following elements are the same as the
> last one?  That said, I'm looking for a loop-hole to not add the extra
> VEC_DUPLICATE_CST code ... eventually we can simply allow scalars in
> contexts where vectors are valid?  Like we do for shifts?  OTOH that'd
> be "implicitely typed constants" (depends on context) like CONST_INT,
> so probably not the way to go?

I don't think it would be as bad as CONST_INT, because at least it would
still have a type.  But the vast majority of code that sees an INTEGER_CST
is going to expect it to be a scalar integer.  I don't think trying to
reuse it for vectors would make things cleaner.

Note also that we need a VEC_SERIES_CST and VEC_SERIES_EXPR for linear
series.  Unlike VEC_DUPLICATE_CST, that's restricted to integer types,
to avoid awkward rounding questions with floats.

> The ugly thing about the new codes is that we go from 3 cases when folding
> vector CONSTRUCTOR and VECTOR_CST we now have 24 to cover...
> (if I didn't miscount).  This now really asks for some common iterator over
> elements of a vector (and VEC_DUPLICATE_{EXPR,CST} would just return the first
> elt all the time).  Note that using scalars instead of vectors reduces the
> combinatorical explosion a bit (scalar and scalar const can be handled
> the same).

One of the advantages of restricting the new codes to variable-length
vectors is that you never get combinations of the old and new codes.
So at the moment this adds only a single case for each fold.
With VEC_SERIES_CST we get 4 new cases for PLUS and MINUS, but
not for much else.

If we did extend the new codes to fixed-length vectors, I think we want
to hide it behind a common accessor that gives the value of element
number X, rather than operating directly on TREE_CODE, VECTOR_CST_ELT, etc.

Thanks,
Richard

>
> So ... I'd rather not have those if we can avoid it but I haven't
> fully thought out
> things as you can see from above.
>
> Richard.