Re: [PATCH, vectorizer]: Vectorize int->double and double->int conversions

["Richard Guenther" <richard dot guenther at gmail dot com>]

On 4/24/07, Uros Bizjak <ubizjak@gmail.com> wrote:
> Hello!
>
> This patch vectorizes int->double and double->int conversions. As
> noticed by Dorit, the core change of the patch is in
> vectorizable_conversion() that now includes code to widen and narrow
> operands (the same as in vectorizable_type_promotion/demotion).
>
> Additionally, this patch implements supportable_narrowing_operation()
> function as a helper function to check if target supports certain
> narrowing optabs. While looking into this, I noticed that
> supportable_widening_operation() should handle CONVERT_EXPR in the
> same way as NOP_EXPR.
>
> I didn't remove vectorizable_type_promotion/demotion() functions, as
> the patch is already big enough, but this removal should be trivial.
> However, vectorizable_type_promotion() was changed to use
> supportable_narrowing_operation().
>
> Following that change, it should be relatively easy to add additional
> widening/narrowing operation (i.e. float from DImode to SFmode should
> be added as narrowing op using vec_pack_float optab because we pack
> 2x2 elements into 4 elements - in contrast to float from SImode to
> DFmode, where operation is implemented as widening op usign
> vec_unpacks_float_[hi|lo] optab). However, no target currently
> implements DImode->SFmode vector conversion, so the optab is omitted
> from the patch.

I wonder why

+    case VEC_UNPACK_FLOAT_HI_EXPR:
+      return TYPE_UNSIGNED (type) ?
+       vec_unpacku_float_hi_optab : vec_unpacks_float_hi_optab;
+
+    case VEC_UNPACK_FLOAT_LO_EXPR:
+      return TYPE_UNSIGNED (type) ?
+       vec_unpacku_float_lo_optab : vec_unpacks_float_lo_optab;
+
    case VEC_PACK_TRUNC_EXPR:
      return vec_pack_trunc_optab;

    case VEC_PACK_SAT_EXPR:
      return TYPE_UNSIGNED (type) ? vec_pack_usat_optab : vec_pack_ssat_optab;

+    case VEC_PACK_FIX_TRUNC_EXPR:
+      return TYPE_UNSIGNED (type) ?
+       vec_pack_ufix_trunc_optab : vec_pack_sfix_trunc_optab;
+

bot can be correct or are necessary.  It at least needs documenting that TYPE is
in one case the input type and in one case the output type (floating point types
obviously are not signed or unsigned).  For my taste this function is
the wrong thing
to handle tree-codes that have different output type than input type.

In fact, expand_expr_real_1 has type set to the output type (TREE_TYPE
(exp)) which
is wrong for (unsignedp is also taken from the float result type)

    case VEC_UNPACK_LO_EXPR:
+    case VEC_UNPACK_FLOAT_HI_EXPR:
+    case VEC_UNPACK_FLOAT_LO_EXPR:
      {
       op0 = expand_expr (TREE_OPERAND (exp, 0), NULL_RTX, VOIDmode, 0);
       this_optab = optab_for_tree_code (code, type);
       temp = expand_widen_pattern_expr (exp, op0, NULL_RTX, NULL_RTX,
                                         target, unsignedp);

but correct for

    case VEC_PACK_TRUNC_EXPR:
    case VEC_PACK_SAT_EXPR:
+    case VEC_PACK_FIX_TRUNC_EXPR:
      {
       mode = TYPE_MODE (TREE_TYPE (TREE_OPERAND (exp, 0)));
       goto binop;
...
binop:
 expand_operands (TREE_OPERAND (exp, 0), TREE_OPERAND (exp, 1),
                  subtarget, &op0, &op1, 0);
binop2:
 this_optab = optab_for_tree_code (code, type);


So, this part needs at least clarification (and comments).  I also
think I cannot approve
this patch, as it is certainly algorithmic.  Maybe Ian wants to take the ball.

Richard.