On Fri, Oct 23, 2015 at 12:23 PM, Ilya Enkovich <enkovich....@gmail.com> wrote:
> 2015-10-23 12:59 GMT+03:00 Richard Biener <richard.guent...@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.

I put it there to make sure it is because otherwise the use of boolean_type_node
below needs adjustment (boolean_true_node as well).  TREE_TYPE (cond)
would work and constant_boolean_node (true, TREE_TYPE (cond)) for
boolean_type_node.

Yes, you are right.

>>    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

No, we'd get

  mask_1 = bool != 1;

and the 'mask' variable should have been simplified to 'bool'
(yes, we'd insert a dead stmt).  gimple_build simplifies
stmts via the match-and-simplify machinery and match.pd
knows how to invert conditions.

> 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)

Reply via email to