[v1] Vect: Promote unsigned .SAT_ADD constant operand for vectorizable_call

Message ID 20240710092753.3810342-1-pan2.li@intel.com
State New
Headers
Series [v1] Vect: Promote unsigned .SAT_ADD constant operand for vectorizable_call |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_gcc_build--master-arm success Build passed
linaro-tcwg-bot/tcwg_gcc_build--master-aarch64 success Build passed
linaro-tcwg-bot/tcwg_gcc_check--master-arm success Test passed
linaro-tcwg-bot/tcwg_gcc_check--master-aarch64 success Test passed

Commit Message

Li, Pan2 July 10, 2024, 9:27 a.m. UTC
  From: Pan Li <pan2.li@intel.com>

The .SAT_ADD has 2 operand and one of the operand may be INTEGER_CST.
For example _1 = .SAT_ADD (_2, 9) comes from below sample code.

Form 3:
  #define DEF_VEC_SAT_U_ADD_IMM_FMT_3(T, IMM)                          \
  T __attribute__((noinline))                                          \
  vec_sat_u_add_imm##IMM##_##T##_fmt_3 (T *out, T *in, unsigned limit) \
  {                                                                    \
    unsigned i;                                                        \
    T ret;                                                             \
    for (i = 0; i < limit; i++)                                        \
      {                                                                \
        out[i] = __builtin_add_overflow (in[i], IMM, &ret) ? -1 : ret; \
      }                                                                \
  }

DEF_VEC_SAT_U_ADD_IMM_FMT_3(uint64_t, 9)

It will failure to vectorize as the vectorizable_call will check the
operands is type_compatiable but the imm will be treated as unsigned
SImode from the perspective of tree.  Aka

uint64_t _1;
uint64_t _2;

_1 = .SAT_ADD (_2, 9);

The _1 and _2 are unsigned DImode, which is different to imm 9 unsigned
SImode,  and then result in vectorizable_call fails.  This patch would
like to promote the imm operand to the operand type mode of _2 if and
only if there is no precision/data loss.  Aka convert the imm 9 to the
DImode for above example.

The below test suites are passed for this patch:
1. The rv64gcv fully regression tests.
2. The rv64gcv build with glibc.
3. The x86 bootstrap tests.
4. The x86 fully regression tests.

gcc/ChangeLog:

	* tree-vect-patterns.cc (vect_recog_promote_cst_to_unsigned): Add
	new func impl to promote the imm tree to target type.
	(vect_recog_sat_add_pattern): Peform the type promotion before
	generate .SAT_ADD call.

Signed-off-by: Pan Li <pan2.li@intel.com>
---
 gcc/tree-vect-patterns.cc | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)
  

Comments

Richard Biener July 10, 2024, 11:35 a.m. UTC | #1
On Wed, Jul 10, 2024 at 11:28 AM <pan2.li@intel.com> wrote:
>
> From: Pan Li <pan2.li@intel.com>
>
> The .SAT_ADD has 2 operand and one of the operand may be INTEGER_CST.
> For example _1 = .SAT_ADD (_2, 9) comes from below sample code.
>
> Form 3:
>   #define DEF_VEC_SAT_U_ADD_IMM_FMT_3(T, IMM)                          \
>   T __attribute__((noinline))                                          \
>   vec_sat_u_add_imm##IMM##_##T##_fmt_3 (T *out, T *in, unsigned limit) \
>   {                                                                    \
>     unsigned i;                                                        \
>     T ret;                                                             \
>     for (i = 0; i < limit; i++)                                        \
>       {                                                                \
>         out[i] = __builtin_add_overflow (in[i], IMM, &ret) ? -1 : ret; \
>       }                                                                \
>   }
>
> DEF_VEC_SAT_U_ADD_IMM_FMT_3(uint64_t, 9)
>
> It will failure to vectorize as the vectorizable_call will check the
> operands is type_compatiable but the imm will be treated as unsigned
> SImode from the perspective of tree.

I think that's a bug.  Do you say __builtin_add_overflow fails to promote
(constant) arguments?

>  Aka
>
> uint64_t _1;
> uint64_t _2;
>
> _1 = .SAT_ADD (_2, 9);
>
> The _1 and _2 are unsigned DImode, which is different to imm 9 unsigned
> SImode,  and then result in vectorizable_call fails.  This patch would
> like to promote the imm operand to the operand type mode of _2 if and
> only if there is no precision/data loss.  Aka convert the imm 9 to the
> DImode for above example.
>
> The below test suites are passed for this patch:
> 1. The rv64gcv fully regression tests.
> 2. The rv64gcv build with glibc.
> 3. The x86 bootstrap tests.
> 4. The x86 fully regression tests.
>
> gcc/ChangeLog:
>
>         * tree-vect-patterns.cc (vect_recog_promote_cst_to_unsigned): Add
>         new func impl to promote the imm tree to target type.
>         (vect_recog_sat_add_pattern): Peform the type promotion before
>         generate .SAT_ADD call.
>
> Signed-off-by: Pan Li <pan2.li@intel.com>
> ---
>  gcc/tree-vect-patterns.cc | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
>
> diff --git a/gcc/tree-vect-patterns.cc b/gcc/tree-vect-patterns.cc
> index 86e893a1c43..e1013222b12 100644
> --- a/gcc/tree-vect-patterns.cc
> +++ b/gcc/tree-vect-patterns.cc
> @@ -4527,6 +4527,20 @@ vect_recog_build_binary_gimple_stmt (vec_info *vinfo, stmt_vec_info stmt_info,
>    return NULL;
>  }
>
> +static void
> +vect_recog_promote_cst_to_unsigned (tree *op, tree type)
> +{
> +  if (TREE_CODE (*op) != INTEGER_CST || !TYPE_UNSIGNED (type))
> +    return;
> +
> +  unsigned precision = TYPE_PRECISION (type);
> +  wide_int type_max = wi::mask (precision, false, precision);
> +  wide_int op_cst_val = wi::to_wide (*op, precision);
> +
> +  if (wi::leu_p (op_cst_val, type_max))
> +    *op = wide_int_to_tree (type, op_cst_val);
> +}
> +
>  /*
>   * Try to detect saturation add pattern (SAT_ADD), aka below gimple:
>   *   _7 = _4 + _6;
> @@ -4553,6 +4567,9 @@ vect_recog_sat_add_pattern (vec_info *vinfo, stmt_vec_info stmt_vinfo,
>
>    if (gimple_unsigned_integer_sat_add (lhs, ops, NULL))
>      {
> +      vect_recog_promote_cst_to_unsigned (&ops[0], TREE_TYPE (ops[1]));
> +      vect_recog_promote_cst_to_unsigned (&ops[1], TREE_TYPE (ops[0]));
> +
>        gimple *stmt = vect_recog_build_binary_gimple_stmt (vinfo, stmt_vinfo,
>                                                           IFN_SAT_ADD, type_out,
>                                                           lhs, ops[0], ops[1]);
> --
> 2.34.1
>
  
Li, Pan2 July 10, 2024, 11:04 p.m. UTC | #2
> I think that's a bug.  Do you say __builtin_add_overflow fails to promote
> (constant) arguments?

Thanks Richard. Not very sure which part result in type mismatch, will take a look and keep you posted.

Pan

-----Original Message-----
From: Richard Biener <richard.guenther@gmail.com> 
Sent: Wednesday, July 10, 2024 7:36 PM
To: Li, Pan2 <pan2.li@intel.com>
Cc: gcc-patches@gcc.gnu.org; juzhe.zhong@rivai.ai; kito.cheng@gmail.com; tamar.christina@arm.com; jeffreyalaw@gmail.com; rdapp.gcc@gmail.com; Liu, Hongtao <hongtao.liu@intel.com>
Subject: Re: [PATCH v1] Vect: Promote unsigned .SAT_ADD constant operand for vectorizable_call

On Wed, Jul 10, 2024 at 11:28 AM <pan2.li@intel.com> wrote:
>
> From: Pan Li <pan2.li@intel.com>
>
> The .SAT_ADD has 2 operand and one of the operand may be INTEGER_CST.
> For example _1 = .SAT_ADD (_2, 9) comes from below sample code.
>
> Form 3:
>   #define DEF_VEC_SAT_U_ADD_IMM_FMT_3(T, IMM)                          \
>   T __attribute__((noinline))                                          \
>   vec_sat_u_add_imm##IMM##_##T##_fmt_3 (T *out, T *in, unsigned limit) \
>   {                                                                    \
>     unsigned i;                                                        \
>     T ret;                                                             \
>     for (i = 0; i < limit; i++)                                        \
>       {                                                                \
>         out[i] = __builtin_add_overflow (in[i], IMM, &ret) ? -1 : ret; \
>       }                                                                \
>   }
>
> DEF_VEC_SAT_U_ADD_IMM_FMT_3(uint64_t, 9)
>
> It will failure to vectorize as the vectorizable_call will check the
> operands is type_compatiable but the imm will be treated as unsigned
> SImode from the perspective of tree.

I think that's a bug.  Do you say __builtin_add_overflow fails to promote
(constant) arguments?

>  Aka
>
> uint64_t _1;
> uint64_t _2;
>
> _1 = .SAT_ADD (_2, 9);
>
> The _1 and _2 are unsigned DImode, which is different to imm 9 unsigned
> SImode,  and then result in vectorizable_call fails.  This patch would
> like to promote the imm operand to the operand type mode of _2 if and
> only if there is no precision/data loss.  Aka convert the imm 9 to the
> DImode for above example.
>
> The below test suites are passed for this patch:
> 1. The rv64gcv fully regression tests.
> 2. The rv64gcv build with glibc.
> 3. The x86 bootstrap tests.
> 4. The x86 fully regression tests.
>
> gcc/ChangeLog:
>
>         * tree-vect-patterns.cc (vect_recog_promote_cst_to_unsigned): Add
>         new func impl to promote the imm tree to target type.
>         (vect_recog_sat_add_pattern): Peform the type promotion before
>         generate .SAT_ADD call.
>
> Signed-off-by: Pan Li <pan2.li@intel.com>
> ---
>  gcc/tree-vect-patterns.cc | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
>
> diff --git a/gcc/tree-vect-patterns.cc b/gcc/tree-vect-patterns.cc
> index 86e893a1c43..e1013222b12 100644
> --- a/gcc/tree-vect-patterns.cc
> +++ b/gcc/tree-vect-patterns.cc
> @@ -4527,6 +4527,20 @@ vect_recog_build_binary_gimple_stmt (vec_info *vinfo, stmt_vec_info stmt_info,
>    return NULL;
>  }
>
> +static void
> +vect_recog_promote_cst_to_unsigned (tree *op, tree type)
> +{
> +  if (TREE_CODE (*op) != INTEGER_CST || !TYPE_UNSIGNED (type))
> +    return;
> +
> +  unsigned precision = TYPE_PRECISION (type);
> +  wide_int type_max = wi::mask (precision, false, precision);
> +  wide_int op_cst_val = wi::to_wide (*op, precision);
> +
> +  if (wi::leu_p (op_cst_val, type_max))
> +    *op = wide_int_to_tree (type, op_cst_val);
> +}
> +
>  /*
>   * Try to detect saturation add pattern (SAT_ADD), aka below gimple:
>   *   _7 = _4 + _6;
> @@ -4553,6 +4567,9 @@ vect_recog_sat_add_pattern (vec_info *vinfo, stmt_vec_info stmt_vinfo,
>
>    if (gimple_unsigned_integer_sat_add (lhs, ops, NULL))
>      {
> +      vect_recog_promote_cst_to_unsigned (&ops[0], TREE_TYPE (ops[1]));
> +      vect_recog_promote_cst_to_unsigned (&ops[1], TREE_TYPE (ops[0]));
> +
>        gimple *stmt = vect_recog_build_binary_gimple_stmt (vinfo, stmt_vinfo,
>                                                           IFN_SAT_ADD, type_out,
>                                                           lhs, ops[0], ops[1]);
> --
> 2.34.1
>
  

Patch

diff --git a/gcc/tree-vect-patterns.cc b/gcc/tree-vect-patterns.cc
index 86e893a1c43..e1013222b12 100644
--- a/gcc/tree-vect-patterns.cc
+++ b/gcc/tree-vect-patterns.cc
@@ -4527,6 +4527,20 @@  vect_recog_build_binary_gimple_stmt (vec_info *vinfo, stmt_vec_info stmt_info,
   return NULL;
 }
 
+static void
+vect_recog_promote_cst_to_unsigned (tree *op, tree type)
+{
+  if (TREE_CODE (*op) != INTEGER_CST || !TYPE_UNSIGNED (type))
+    return;
+
+  unsigned precision = TYPE_PRECISION (type);
+  wide_int type_max = wi::mask (precision, false, precision);
+  wide_int op_cst_val = wi::to_wide (*op, precision);
+
+  if (wi::leu_p (op_cst_val, type_max))
+    *op = wide_int_to_tree (type, op_cst_val);
+}
+
 /*
  * Try to detect saturation add pattern (SAT_ADD), aka below gimple:
  *   _7 = _4 + _6;
@@ -4553,6 +4567,9 @@  vect_recog_sat_add_pattern (vec_info *vinfo, stmt_vec_info stmt_vinfo,
 
   if (gimple_unsigned_integer_sat_add (lhs, ops, NULL))
     {
+      vect_recog_promote_cst_to_unsigned (&ops[0], TREE_TYPE (ops[1]));
+      vect_recog_promote_cst_to_unsigned (&ops[1], TREE_TYPE (ops[0]));
+
       gimple *stmt = vect_recog_build_binary_gimple_stmt (vinfo, stmt_vinfo,
 							  IFN_SAT_ADD, type_out,
 							  lhs, ops[0], ops[1]);