Re: [Boolean Vector, patch 1/5] Introduce boolean vector to be used as a vector comparison type

[Ilya Enkovich <enkovich.gnu@gmail.com>]

2015-10-13 16:17 GMT+03:00 Richard Biener <richard.guenther@gmail.com>:
> On Fri, Oct 9, 2015 at 10:43 PM, Jeff Law <law@redhat.com> wrote:
>> On 10/02/2015 07:59 AM, Ilya Enkovich wrote:
>>>
>>> 2015-10-02  Ilya Enkovich  <enkovich.gnu@gmail.com>
>>>
>>>         * doc/tm.texi: Regenerated.
>>>         * doc/tm.texi.in (TARGET_VECTORIZE_GET_MASK_MODE): New.
>>>         * stor-layout.c (layout_type): Use mode to get vector mask size.
>>>         * target.def (get_mask_mode): New.
>>>         * targhooks.c (default_get_mask_mode): New.
>>>         * targhooks.h (default_get_mask_mode): New.
>>>         * gcc/tree-vect-stmts.c (get_same_sized_vectype): Add special case
>>>         for boolean vector.
>>>         * tree.c (MAX_BOOL_CACHED_PREC): New.
>>>         (nonstandard_boolean_type_cache): New.
>>>         (build_nonstandard_boolean_type): New.
>>>         (make_vector_type): Vector mask has no canonical type.
>>>         (build_truth_vector_type): New.
>>>         (build_same_sized_truth_vector_type): New.
>>>         (truth_type_for): Support vector masks.
>>>         * tree.h (VECTOR_BOOLEAN_TYPE_P): New.
>>>         (build_truth_vector_type): New.
>>>         (build_same_sized_truth_vector_type): New.
>>>         (build_nonstandard_boolean_type): New.
>>>
>>>
>>> diff --git a/gcc/doc/tm.texi b/gcc/doc/tm.texi
>>> index eb495a8..098213e 100644
>>> --- a/gcc/doc/tm.texi
>>> +++ b/gcc/doc/tm.texi
>>> @@ -5688,6 +5688,11 @@ mode returned by
>>> @code{TARGET_VECTORIZE_PREFERRED_SIMD_MODE}.
>>>   The default is zero which means to not iterate over other vector sizes.
>>>   @end deftypefn
>>>
>>> +@deftypefn {Target Hook} machine_mode TARGET_VECTORIZE_GET_MASK_MODE
>>> (unsigned @var{nunits}, unsigned @var{length})
>>> +This hook returns mode to be used for a mask to be used for a vector
>>> +of specified @var{length} with @var{nunits} elements.
>>> +@end deftypefn
>>
>> Does it make sense to indicate the default used if the target does not
>> provide a definition for this hook?
>>
>>
>>
>>
>>> diff --git a/gcc/stor-layout.c b/gcc/stor-layout.c
>>> index 938e54b..58ecd7b 100644
>>> --- a/gcc/stor-layout.c
>>> +++ b/gcc/stor-layout.c
>>> @@ -2184,10 +2184,16 @@ layout_type (tree type)
>>>
>>>         TYPE_SATURATING (type) = TYPE_SATURATING (TREE_TYPE (type));
>>>           TYPE_UNSIGNED (type) = TYPE_UNSIGNED (TREE_TYPE (type));
>>> -       TYPE_SIZE_UNIT (type) = int_const_binop (MULT_EXPR,
>>> -                                                TYPE_SIZE_UNIT
>>> (innertype),
>>> -                                                size_int (nunits));
>>> -       TYPE_SIZE (type) = int_const_binop (MULT_EXPR, TYPE_SIZE
>>> (innertype),
>>> +       /* Several boolean vector elements may fit in a single unit.  */
>>> +       if (VECTOR_BOOLEAN_TYPE_P (type))
>>> +         TYPE_SIZE_UNIT (type)
>>> +           = size_int (GET_MODE_SIZE (type->type_common.mode));
>>
>> Shouldn't this be TYPE_MODE rather than accessing the internals of the tree
>> node directly?
>
> Probably not because of TYPE_MODE interfering for vector types.

Seems I need to roll it back then. I don't think I want scalar mode to
be used for cases when proper integer vector mode is unsupported by
target but returned by default get_mask_mode hook. Such cases just
should be lowered into scalars.

>
> But...
>
> +/* Builds a boolean type of precision PRECISION.
> +   Used for boolean vectors to choose proper vector element size.  */
> +tree
> +build_nonstandard_boolean_type (unsigned HOST_WIDE_INT precision)
> +{
> +  tree type;
> +
> +  if (precision <= MAX_BOOL_CACHED_PREC)
> +    {
> +      type = nonstandard_boolean_type_cache[precision];
> +      if (type)
> +       return type;
> +    }
> +
> +  type = make_node (BOOLEAN_TYPE);
> +  TYPE_PRECISION (type) = precision;
> +  fixup_unsigned_type (type);
>
> do we really need differing _precision_ boolean types?  I think we only
> need differing size (aka mode) boolean types, no?  Thus, keep precision == 1
> but "only" adjust the mode (possibly by simply setting precision to 1 after
> fixup_unsigned_type ...)?

The reason for that was -1 value of a proper size which may be used as
vector element value. I'm not sure if something breaks in the compiler
if I set 1 precision for all created boolean types, but I assumed it's
reasonable to match precision and actually stored values.

Ilya

>
> Richard.
>
>>
>>> diff --git a/gcc/tree.c b/gcc/tree.c
>>> index 84fd34d..0cb8361 100644
>>> --- a/gcc/tree.c
>>> +++ b/gcc/tree.c
>>> @@ -11067,9 +11130,10 @@ truth_type_for (tree type)
>>>   {
>>>     if (TREE_CODE (type) == VECTOR_TYPE)
>>>       {
>>> -      tree elem = lang_hooks.types.type_for_size
>>> -        (GET_MODE_BITSIZE (TYPE_MODE (TREE_TYPE (type))), 0);
>>> -      return build_opaque_vector_type (elem, TYPE_VECTOR_SUBPARTS
>>> (type));
>>> +      if (VECTOR_BOOLEAN_TYPE_P (type))
>>> +       return type;
>>> +      return build_truth_vector_type (TYPE_VECTOR_SUBPARTS (type),
>>> +                                     GET_MODE_SIZE (TYPE_MODE (type)));
>>
>> Presumably you're not building an opaque type anymore because you want
>> warnings if somethings tries to do a conversion?  I'm going to assume this
>> was intentional.
>>
>>
>> With the doc update and the fix to use TYPE_MODE (assuming there's not a
>> good reason to be looking at the underlying type directly) this is OK.
>>
>> jeff