Re: [mask-load, patch 1/2] Use boolean predicate for masked loads and store

[Ilya Enkovich <enkovich dot gnu at gmail dot com>]

2015-10-23 12:59 GMT+03:00 Richard Biener <richard.guenther@gmail.com>:
>
> ICK.  So what does the above do?  It basically preserves the boolean condition
> as "mask" unless ... we ought to swap it (formerly easy, just swap arguments
> of the cond_expr, now a bit harder, we need to invert the condition).  But I
> don't understand the 'negate' dance.  It looks like you want to have mask
> not be bool != 0 or bool == 1 but just bool in this case.  I suggest you
> rework this to do sth like

That's right, I want to avoid ==,!= comparisons with 0 and 1 by either
using compared SSA_NAME
or SSA_NAME != 0 (negate case).

>
>    gimple_seq stmts = NULL;
>    gcc_assert (types_compatible_p (TREE_TYPE (cond), boolean_type_node));

Is it really valid assert? Compiling fortran test with LTO I may have
logical(kind=4) [aka 32bit boolean]
type for cond and single bit _Bool for boolean_type_node.

>    if (TREE_CODE (cond) == SSA_NAME)
>      ;
>    else if (COMPARISON_CLASS_P (cond))
>      mask = gimple_build (&stmts, TREE_CODE (cond), boolean_type_node,
> TREE_OPERAND (cond, 0), TREE_OPERAND (cond, 1));
>    else
>       gcc_unreachable ();
>    if (swap)
>      mask = gimple_build (&stmts, BIT_XOR_EXPR, boolean_type_node,
> mask, boolean_true_node);
>    gsi_insert_seq_before (&gsi, stmts, GSI_SAME_STMT);
>
> which should do all of the above.

Thus we would get smth like

mask_1 = bool != 1
mask_2 = mask_1 XOR 1
_ifc_ = mask_2

instead of

_ifc_ = bool

Note that cond is built to be used as a condition in COND_EXPR
prepared for vectorization, i.e. it is always a comparison, thus
comparison with 0 and 1 is quite a common case.

Thanks,
Ilya

>
> The rest of the changes look good to me.
>
> Thanks,
> Richard.
>
>>                 /* Save mask and its size for further use.  */
>>                 vect_sizes.safe_push (bitsize);
>>                 vect_masks.safe_push (mask);
>> diff --git a/gcc/tree-vect-stmts.c b/gcc/tree-vect-stmts.c
>> index 337ea7b..d6819ec 100644
>> --- a/gcc/tree-vect-stmts.c
>> +++ b/gcc/tree-vect-stmts.c
>> @@ -1800,6 +1800,7 @@ vectorizable_mask_load_store (gimple *stmt, gimple_stmt_iterator *gsi,
>>    bool nested_in_vect_loop = nested_in_vect_loop_p (loop, stmt);
>>    struct data_reference *dr = STMT_VINFO_DATA_REF (stmt_info);
>>    tree vectype = STMT_VINFO_VECTYPE (stmt_info);
>> +  tree mask_vectype;
>>    tree elem_type;
>>    gimple *new_stmt;
>>    tree dummy;
>> @@ -1827,8 +1828,8 @@ vectorizable_mask_load_store (gimple *stmt, gimple_stmt_iterator *gsi,
>>
>>    is_store = gimple_call_internal_fn (stmt) == IFN_MASK_STORE;
>>    mask = gimple_call_arg (stmt, 2);
>> -  if (TYPE_PRECISION (TREE_TYPE (mask))
>> -      != GET_MODE_BITSIZE (TYPE_MODE (TREE_TYPE (vectype))))
>> +
>> +  if (TREE_CODE (TREE_TYPE (mask)) != BOOLEAN_TYPE)
>>      return false;
>>
>>    /* FORNOW. This restriction should be relaxed.  */
>> @@ -1857,6 +1858,19 @@ vectorizable_mask_load_store (gimple *stmt, gimple_stmt_iterator *gsi,
>>    if (STMT_VINFO_STRIDED_P (stmt_info))
>>      return false;
>>
>> +  if (TREE_CODE (mask) != SSA_NAME)
>> +    return false;
>> +
>> +  if (!vect_is_simple_use_1 (mask, stmt, loop_vinfo, NULL,
>> +                            &def_stmt, &def, &dt, &mask_vectype))
>> +    return false;
>> +
>> +  if (!mask_vectype)
>> +    mask_vectype = get_mask_type_for_scalar_type (TREE_TYPE (vectype));
>> +
>> +  if (!mask_vectype)
>> +    return false;
>> +
>>    if (STMT_VINFO_GATHER_SCATTER_P (stmt_info))
>>      {
>>        gimple *def_stmt;
>> @@ -1890,14 +1904,9 @@ vectorizable_mask_load_store (gimple *stmt, gimple_stmt_iterator *gsi,
>>                                  : DR_STEP (dr), size_zero_node) <= 0)
>>      return false;
>>    else if (!VECTOR_MODE_P (TYPE_MODE (vectype))
>> -          || !can_vec_mask_load_store_p (TYPE_MODE (vectype), !is_store))
>> -    return false;
>> -
>> -  if (TREE_CODE (mask) != SSA_NAME)
>> -    return false;
>> -
>> -  if (!vect_is_simple_use (mask, stmt, loop_vinfo, NULL,
>> -                          &def_stmt, &def, &dt))
>> +          || !can_vec_mask_load_store_p (TYPE_MODE (vectype),
>> +                                         TYPE_MODE (mask_vectype),
>> +                                         !is_store))
>>      return false;
>>
>>    if (is_store)