Richard Biener via Gcc-patches <gcc-patches@gcc.gnu.org> writes:
> On Fri, Jun 2, 2023 at 3:01 AM liuhongt via Gcc-patches
> <gcc-patches@gcc.gnu.org> wrote:
>>
>> We have already use intermidate type in case WIDEN, but not for NONE,
>> this patch extended that.
>>
>> I didn't do that in pattern recog since we need to know whether the
>> stmt belongs to any slp_node to decide the vectype, the related optabs
>> are checked according to vectype_in and vectype_out. For non-slp case,
>> vec_pack/unpack are always used when lhs has different size from rhs,
>> for slp case, sometimes vec_pack/unpack is used, somethings
>> direct conversion is used.
>>
>> Bootstrapped and regtested on x86_64-pc-linux-gnu{-m32,}.
>> Ok for trunk?
>>
>> gcc/ChangeLog:
>>
>> PR target/110018
>> * tree-vect-stmts.cc (vectorizable_conversion): Use
>> intermiediate integer type for float_expr/fix_trunc_expr when
>> direct optab is not existed.
>>
>> gcc/testsuite/ChangeLog:
>>
>> * gcc.target/i386/pr110018-1.c: New test.
>> ---
>> gcc/testsuite/gcc.target/i386/pr110018-1.c | 94 ++++++++++++++++++++++
>> gcc/tree-vect-stmts.cc | 56 ++++++++++++-
>> 2 files changed, 149 insertions(+), 1 deletion(-)
>> create mode 100644 gcc/testsuite/gcc.target/i386/pr110018-1.c
>>
>> diff --git a/gcc/testsuite/gcc.target/i386/pr110018-1.c
>> b/gcc/testsuite/gcc.target/i386/pr110018-1.c
>> new file mode 100644
>> index 00000000000..b1baffd7af1
>> --- /dev/null
>> +++ b/gcc/testsuite/gcc.target/i386/pr110018-1.c
>> @@ -0,0 +1,94 @@
>> +/* { dg-do compile } */
>> +/* { dg-options "-mavx512fp16 -mavx512vl -O2 -mavx512dq" } */
>> +/* { dg-final { scan-assembler-times {(?n)vcvttp[dsh]2[dqw]} 5 } } */
>> +/* { dg-final { scan-assembler-times {(?n)vcvt[dqw]*2p[dsh]} 5 } } */
>> +
>> +void
>> +foo (double* __restrict a, char* b)
>> +{
>> + a[0] = b[0];
>> + a[1] = b[1];
>> +}
>> +
>> +void
>> +foo1 (float* __restrict a, char* b)
>> +{
>> + a[0] = b[0];
>> + a[1] = b[1];
>> + a[2] = b[2];
>> + a[3] = b[3];
>> +}
>> +
>> +void
>> +foo2 (_Float16* __restrict a, char* b)
>> +{
>> + a[0] = b[0];
>> + a[1] = b[1];
>> + a[2] = b[2];
>> + a[3] = b[3];
>> + a[4] = b[4];
>> + a[5] = b[5];
>> + a[6] = b[6];
>> + a[7] = b[7];
>> +}
>> +
>> +void
>> +foo3 (double* __restrict a, short* b)
>> +{
>> + a[0] = b[0];
>> + a[1] = b[1];
>> +}
>> +
>> +void
>> +foo4 (float* __restrict a, char* b)
>> +{
>> + a[0] = b[0];
>> + a[1] = b[1];
>> + a[2] = b[2];
>> + a[3] = b[3];
>> +}
>> +
>> +void
>> +foo5 (double* __restrict b, char* a)
>> +{
>> + a[0] = b[0];
>> + a[1] = b[1];
>> +}
>> +
>> +void
>> +foo6 (float* __restrict b, char* a)
>> +{
>> + a[0] = b[0];
>> + a[1] = b[1];
>> + a[2] = b[2];
>> + a[3] = b[3];
>> +}
>> +
>> +void
>> +foo7 (_Float16* __restrict b, char* a)
>> +{
>> + a[0] = b[0];
>> + a[1] = b[1];
>> + a[2] = b[2];
>> + a[3] = b[3];
>> + a[4] = b[4];
>> + a[5] = b[5];
>> + a[6] = b[6];
>> + a[7] = b[7];
>> +}
>> +
>> +void
>> +foo8 (double* __restrict b, short* a)
>> +{
>> + a[0] = b[0];
>> + a[1] = b[1];
>> +}
>> +
>> +void
>> +foo9 (float* __restrict b, char* a)
>> +{
>> + a[0] = b[0];
>> + a[1] = b[1];
>> + a[2] = b[2];
>> + a[3] = b[3];
>> +}
>> diff --git a/gcc/tree-vect-stmts.cc b/gcc/tree-vect-stmts.cc
>> index bd3b07a3aa1..1118c89686d 100644
>> --- a/gcc/tree-vect-stmts.cc
>> +++ b/gcc/tree-vect-stmts.cc
>> @@ -5162,6 +5162,49 @@ vectorizable_conversion (vec_info *vinfo,
>> return false;
>> if (supportable_convert_operation (code, vectype_out, vectype_in,
>> &code1))
>> break;
>
> A comment would be nice here. Like
>
> /* For conversions between float and smaller integer types try whether we
> can
> use intermediate signed integer types to support the conversion. */
>
>> + if ((code == FLOAT_EXPR
>> + && GET_MODE_SIZE (lhs_mode) > GET_MODE_SIZE (rhs_mode))
>> + || (code == FIX_TRUNC_EXPR
>> + && GET_MODE_SIZE (rhs_mode) > GET_MODE_SIZE (lhs_mode)))
Is the FIX_TRUNC_EXPR case safe without some flag?
#include <stdint.h>
int32_t x = (int32_t)0x1.0p32;
int32_t y = (int32_t)(int64_t)0x1.0p32;
sets x to 2147483647 and y to 0.
Thanks,
Richard
>> + {
>> + bool float_expr_p = code == FLOAT_EXPR;
>> + scalar_mode imode = float_expr_p ? rhs_mode : lhs_mode;
>> + fltsz = GET_MODE_SIZE (float_expr_p ? lhs_mode : rhs_mode);
>> + code1 = float_expr_p ? code : NOP_EXPR;
>> + codecvt1 = float_expr_p ? NOP_EXPR : code;
>> + FOR_EACH_2XWIDER_MODE (rhs_mode_iter, imode)
>> + {
>> + imode = rhs_mode_iter.require ();
>> + if (GET_MODE_SIZE (imode) > fltsz)
>> + break;
>> +
>> + cvt_type
>> + = build_nonstandard_integer_type (GET_MODE_BITSIZE (imode),
>> + 0);
>> + cvt_type = get_vectype_for_scalar_type (vinfo, cvt_type,
>> + slp_node);
>> + /* This should only happened for SLP as long as loop vectorizer
>> + only supports same-sized vector. */
>> + if (cvt_type == NULL_TREE
>> + || maybe_ne (TYPE_VECTOR_SUBPARTS (cvt_type), nunits_in)
>> + || !supportable_convert_operation (code1, vectype_out,
>> + cvt_type, &code1)
>> + || !supportable_convert_operation (codecvt1, cvt_type,
>> + vectype_in, &codecvt1))
>> + continue;
>> +
>> + found_mode = true;
>> + break;
>> + }
>> +
>> + if (found_mode)
>> + {
>> + multi_step_cvt++;
>> + interm_types.safe_push (cvt_type);
>> + cvt_type = NULL_TREE;
>> + break;
>> + }
>> + }
>> /* FALLTHRU */
>> unsupported:
>> if (dump_enabled_p ())
>> @@ -5381,7 +5424,18 @@ vectorizable_conversion (vec_info *vinfo,
>> {
>> /* Arguments are ready, create the new vector stmt. */
>> gcc_assert (TREE_CODE_LENGTH (code1) == unary_op);
>> - gassign *new_stmt = gimple_build_assign (vec_dest, code1, vop0);
>> + gassign* new_stmt;
>> + if (multi_step_cvt)
>> + {
>> + gcc_assert (multi_step_cvt == 1);
>> + new_stmt = gimple_build_assign (vec_dest, codecvt1, vop0);
>> + new_temp = make_ssa_name (vec_dest, new_stmt);
>
> I wonder how you get away with using vec_dest both for the final and the
> intermediate conversion and not involve interm_types[0]?
>
> Otherwise looks good.
>
> Thanks,
> Richard.
>
>> + gimple_assign_set_lhs (new_stmt, new_temp);
>> + vect_finish_stmt_generation (vinfo, stmt_info, new_stmt, gsi);
>> + vop0 = new_temp;
>> + vec_dest = vec_dsts[0];
>> + }
>> + new_stmt = gimple_build_assign (vec_dest, code1, vop0);
>> new_temp = make_ssa_name (vec_dest, new_stmt);
>> gimple_assign_set_lhs (new_stmt, new_temp);
>> vect_finish_stmt_generation (vinfo, stmt_info, new_stmt, gsi);
>> --
>> 2.39.1.388.g2fc9e9ca3c
>>
