Re: [PATCH] gimple-match, gimple-fold: After PROP_gimple_lvec is set, punt for vector stmts that veclower would need to lower [PR98287]

[Richard Sandiford via Gcc-patches]

Jakub Jelinek via Gcc-patches <gcc-patches@gcc.gnu.org> writes:
> On Tue, Feb 02, 2021 at 09:38:09AM +0000, Richard Sandiford wrote:
>> > +    default:
>> > +      if (!VECTOR_MODE_P (mode))
>> > +  return true;
>> > +      op = optab_for_tree_code (code, type, optab_default);
>> > +      if (op == unknown_optab
>> > +    && code == NEGATE_EXPR
>> > +    && INTEGRAL_TYPE_P (TREE_TYPE (type)))
>> > +  op = optab_for_tree_code (MINUS_EXPR, type, optab_default);
>> > +      if (op && optab_handler (op, mode) != CODE_FOR_nothing)
>> > +  return false;
>> > +      return true;
>> 
>> This doesn't look right for things like SVE comparisons, where the
>> result is a VECTOR_BOOLEAN_TYPE_P with MODE_VECTOR_BOOL and where
>> the inputs are something else.  I guess it might also affect FP
>> comparisons on most targets.
>
> So how does that get through expand_vector_operations_1 ?
>
> I don't see there anything that would catch it until:
>   if (compute_type == NULL_TREE)
>     compute_type = get_compute_type (code, op, type);
>   if (compute_type == type)
>     return;
> and the above is an attempt to do what get_compute_type does.
>
> expand_vector_comparison indeed has one case:
>   /* As seen in PR95830, we should not expand comparisons that are only
>      feeding a VEC_COND_EXPR statement.  */
> ...
> and if expand_vector_comparison returns NULL, nothing is lowered.

Right.  But I think the main thing for SVE is the expand_vec_cmp_expr_p
test, which takes both the lhs and rhs types.  So…

> But we can't really look at immediate uses during the checking :(.
>
> So do you suggest for now letting all comparisons get through?

…I guess we could just use expand_vec_cmp_expr_p for the comparisons.

But in general, I think it would be good to avoid duplicating so
much of the tests.  At least the:

  if (CONVERT_EXPR_CODE_P (code)
      || code == FLOAT_EXPR
      || code == FIX_TRUNC_EXPR
      || code == VIEW_CONVERT_EXPR)
    return;

  /* The signedness is determined from input argument.  */
  if (code == VEC_UNPACK_FLOAT_HI_EXPR
      || code == VEC_UNPACK_FLOAT_LO_EXPR
      || code == VEC_PACK_FLOAT_EXPR)
    {
      /* We do not know how to scalarize those.  */
      return;
    }

  /* For widening/narrowing vector operations, the relevant type is of the
     arguments, not the widened result.  VEC_UNPACK_FLOAT_*_EXPR is
     calculated in the same way above.  */
  if (code == WIDEN_SUM_EXPR
      || code == VEC_WIDEN_PLUS_HI_EXPR
      || code == VEC_WIDEN_PLUS_LO_EXPR
      || code == VEC_WIDEN_MINUS_HI_EXPR
      || code == VEC_WIDEN_MINUS_LO_EXPR
      || code == VEC_WIDEN_MULT_HI_EXPR
      || code == VEC_WIDEN_MULT_LO_EXPR
      || code == VEC_WIDEN_MULT_EVEN_EXPR
      || code == VEC_WIDEN_MULT_ODD_EXPR
      || code == VEC_UNPACK_HI_EXPR
      || code == VEC_UNPACK_LO_EXPR
      || code == VEC_UNPACK_FIX_TRUNC_HI_EXPR
      || code == VEC_UNPACK_FIX_TRUNC_LO_EXPR
      || code == VEC_PACK_TRUNC_EXPR
      || code == VEC_PACK_SAT_EXPR
      || code == VEC_PACK_FIX_TRUNC_EXPR
      || code == VEC_WIDEN_LSHIFT_HI_EXPR
      || code == VEC_WIDEN_LSHIFT_LO_EXPR)
    {
      /* We do not know how to scalarize those.  */
      return;
    }

part seems like it could be split out into a predicate.

Maybe we could also split out the optab choice, e.g. returning the optab
if a split is possible and null otherwise.  Just a suggestion though.

Thanks,
Richard