Re: [10/n] Make less use of get_same_sized_vectype

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

Richard Biener <richard.guenther@gmail.com> writes:
> On Fri, Oct 25, 2019 at 2:41 PM Richard Sandiford
> <richard.sandiford@arm.com> wrote:
>>
>> Some callers of get_same_sized_vectype were dealing with operands that
>> are constant or defined externally, and so have no STMT_VINFO_VECTYPE
>> available.  Under the current model, using get_same_sized_vectype for
>> that case is equivalent to using get_vectype_for_scalar_type, since
>> get_vectype_for_scalar_type always returns vectors of the same size,
>> once a size is fixed.
>>
>> Using get_vectype_for_scalar_type is arguably more obvious though:
>> if we're using the same scalar type as we would for internal
>> definitions, we should use the same vector type too.  (Constant and
>> external definitions sometimes let us change the original scalar type
>> to a "nicer" scalar type, but that isn't what's happening here.)
>>
>> This is a prerequisite to supporting multiple vector sizes in the same
>> vec_info.
>
> This might change the actual type we get back, IIRC we mass-changed
> it in the opposite direction from your change in the past, because it's
> more obvious to relate the type used to another vector type on the
> stmt.  So isn't it better to use the new related_vector_type thing here?

I guess this is a downside of the patch order.  Hopefully this looks
like a more sensible decision after 16/n, where we also pass the
group size to get_vectype_for_scalar_type.  If not: :-)

At the moment, there can only ever be one vector type for a given scalar
type within a vector loop.  We don't e.g. allow one loop vector stmt to
use V2SI and another to V4SI, because all vectorised SIs have to be
compatible in the same way that the original scalar SIs were.  So once
we have a loop_vec_info and once we have a scalar type, there's only one
valid choice of vector type.  I think trying to circumvent that by using
get_related_vectype_for_scalar_type instead of get_vectype_for_scalar_type
would run the risk of introducing accidental mismatches.  I.e. if we do
it right, calling get_related_vectype_for_scalar_type would give the same
result as calling get_vectype_for_scalar_type (and so we might as well
just call get_vectype_for_scalar_type).  If we do it wrong we can end up
with a different type from the one that other statements were expecting.

BB vectorisation currently works the same way.  But after 16/n, there
is instead one vector type for each (bb_vinfo, scalar_type, group_size)
triple.  So that patch makes it possible for different SLP instances to
have different vector types for the same scalar type, but the choice is
still fixed within an SLP instance, in a similar way to loop
vectorisation.

So I think using anything other than get_vectype_for_scalar_type
would give a false sense of freedom.  Calling it directly is mostly
useful for temporaries, e.g. in epilogue reduction handling.

Thanks,
Richard