> I think that's a bug. Do you say __builtin_add_overflow fails to promote
> (constant) arguments?
I double checked the 022t.ssa pass for the __builtin_add_overflow operands tree
type. It looks like that
the 2 operands of .ADD_OVERFLOW has different tree types when one of them is
constant.
One is unsigned DI, and the other is int.
(gdb) call debug_gimple_stmt(stmt)
_14 = .ADD_OVERFLOW (_4, 129);
(gdb) call debug_tree (gimple_call_arg(stmt, 0))
<ssa_name 0x7ffff6a0ddc8
type <integer_type 0x7ffff6a437e0 long unsigned int sizes-gimplified public
unsigned DI
size <integer_cst 0x7ffff6a391b0 constant 64>
unit-size <integer_cst 0x7ffff6a391c8 constant 8>
align:64 warn_if_not_align:0 symtab:0 alias-set -1 canonical-type
0x7ffff6a437e0 precision:64 min <integer_cst 0x7ffff6a39480 0> max <integer_cst
0x7ffff6a03660 18446744073709551615>
pointer_to_this <pointer_type 0x7ffff6a522a0>>
visited
def_stmt _4 = *_3;
version:4>
(gdb) call debug_tree (gimple_call_arg(stmt, 1))
<integer_cst 0x7ffff6beac78 type <integer_type 0x7ffff6a435e8 int> constant
129>
(gdb)
Then we go to the vect pass, we can also see that the ops of .ADD_OVERFLOW has
different tree types.
As my understanding, here we should have unsigned DI for constant operands
(gdb) layout src
(gdb) list
506 if
(gimple_call_num_args (_c4) == 2)
507 {
508
tree _q40 = gimple_call_arg (_c4, 0);
509
_q40 = do_valueize (valueize, _q40);
510
tree _q41 = gimple_call_arg (_c4, 1);
511
_q41 = do_valueize (valueize, _q41);
512
if (integer_zerop (_q21))
513
{
514
if (integer_minus_onep (_p1))
515
{
(gdb) call debug_tree (_q40)
<ssa_name 0x7ffff6a0ddc8
type <integer_type 0x7ffff6a437e0 long unsigned int sizes-gimplified public
unsigned DI
size <integer_cst 0x7ffff6a391b0 constant 64>
unit-size <integer_cst 0x7ffff6a391c8 constant 8>
align:64 warn_if_not_align:0 symtab:0 alias-set -1 canonical-type
0x7ffff6a437e0 precision:64 min <integer_cst 0x7ffff6a39480 0> max <integer_cst
0x7ffff6a03660 18446744073709551615>
pointer_to_this <pointer_type 0x7ffff6a522a0>>
visited
def_stmt _4 = *_3;
version:4>
(gdb) call debug_tree (_q41)
<integer_cst 0x7ffff6beac78 type <integer_type 0x7ffff6a435e8 int> constant
129>
Pan
-----Original Message-----
From: Richard Biener <richard.guent...@gmail.com>
Sent: Wednesday, July 10, 2024 7:36 PM
To: Li, Pan2 <pan2...@intel.com>
Cc: gcc-patches@gcc.gnu.org; juzhe.zh...@rivai.ai; kito.ch...@gmail.com;
tamar.christ...@arm.com; jeffreya...@gmail.com; rdapp....@gmail.com; Liu,
Hongtao <hongtao....@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...@intel.com> wrote:
>
> From: Pan Li <pan2...@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...@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
>
