Richard Biener <rguent...@suse.de> writes:
> On Fri, 11 Dec 2020, Richard Sandiford wrote:
>> > diff --git a/gcc/tree-vect-stmts.c b/gcc/tree-vect-stmts.c
>> > index a4980a931a9..d3ab8aa1c29 100644
>> > --- a/gcc/tree-vect-stmts.c
>> > +++ b/gcc/tree-vect-stmts.c
>> > @@ -5123,6 +5123,17 @@ vectorizable_assignment (vec_info *vinfo,
>> > GET_MODE_SIZE (TYPE_MODE (vectype_in)))))
>> > return false;
>> >
>> > + if (VECTOR_BOOLEAN_TYPE_P (vectype)
>> > + && !VECTOR_BOOLEAN_TYPE_P (vectype_in))
>> > + {
>> > + if (dump_enabled_p ())
>> > + dump_printf_loc (MSG_MISSED_OPTIMIZATION, vect_location,
>> > + "can't convert between boolean and non "
>> > + "boolean vectors %T\n", TREE_TYPE (op));
>> > +
>> > + return false;
>> > + }
>>
>> What do you think about my comment in the PR, about instead checking for:
>>
>> VECTOR_BOOLEAN_TYPE_P (vectype)
>> != VECTOR_BOOLEAN_TYPE_P (vectype_in)
>>
>> ? I'm not sure vectorizable_assignment can handle converting a vector
>> boolean type to a non-vector boolean type either, and checking for both
>> directions seems to match the dump message more closely.
>
> The condition matches that used in vectorizable_conversion, I'm not sure
> whether/why we allow the reverse but then if the precisions match
> and we do want to use the bool vector as "data" then why should a
> conversion fail? The condition is specifically to guard a missing
> "sign extension" which should be done via patterns and not conversions.
I think vectorizable_conversion should be able to handle the reverse case.
I'm just not sure vectorizable_assignment (with its VCE) necessarily can.
For vector bool -> vector int, are the upper bits of the vector bool
already supposed to be copies of the low bit (or perhaps instead supposed
to be zero)? I'm not sure we guarantee that for SVE. Only the low bit
of a vector bool element is really significant there.
And if the upper bits of the bool elements are sign copies, is that
necessarily what we want for bool -> int? For the C semantics I'd
have expected the result should be 0 or 1 instead.
> I've misinterpreted your comment to refer to the existing odd
> allowance in the test following this:
>
> /* We do not handle bit-precision changes. */
> if ((CONVERT_EXPR_CODE_P (code)
> || code == VIEW_CONVERT_EXPR)
> && INTEGRAL_TYPE_P (TREE_TYPE (scalar_dest))
> && (!type_has_mode_precision_p (TREE_TYPE (scalar_dest))
> || !type_has_mode_precision_p (TREE_TYPE (op)))
> /* But a conversion that does not change the bit-pattern is ok. */
> && !((TYPE_PRECISION (TREE_TYPE (scalar_dest))
> > TYPE_PRECISION (TREE_TYPE (op)))
> && TYPE_UNSIGNED (TREE_TYPE (op)))
> /* Conversion between boolean types of different sizes is
> a simple assignment in case their vectypes are same
> boolean vectors. */
> && (!VECTOR_BOOLEAN_TYPE_P (vectype)
> || !VECTOR_BOOLEAN_TYPE_P (vectype_in)))
> ^^^
>
> which I have since removed (and where I also shortly thought that
> a VECTOR_BOOLEAN_TYPE_P == VECTOR_BOOLEAN_TYPE_P was intended)
Yeah, initially I was thinking that changing that to != would be
enough to fix the bug. But it feels like the condition ought to be
symmetric wherever we put it.
Thanks,
Richard
