Thanks richard for comments.
> Use HOST_WIDE_INT_1U instead of 1ull
> OK with that change.
Got it, will commit it with this change if no surprise from test.
Pan
-----Original Message-----
From: Richard Biener <richard.guent...@gmail.com>
Sent: Tuesday, October 8, 2024 5:07 PM
To: Li, Pan2 <pan2...@intel.com>
Cc: gcc-patches@gcc.gnu.org; tamar.christ...@arm.com; juzhe.zh...@rivai.ai;
kito.ch...@gmail.com; jeffreya...@gmail.com; rdapp....@gmail.com
Subject: Re: [PATCH v1 1/4] Match: Support form 1 for scalar signed integer
SAT_TRUNC
On Tue, Oct 8, 2024 at 10:34 AM <pan2...@intel.com> wrote:
>
> From: Pan Li <pan2...@intel.com>
>
> This patch would like to support the form 1 of the scalar signed
> integer SAT_TRUNC. Aka below example:
>
> Form 1:
> #define DEF_SAT_S_TRUNC_FMT_1(NT, WT, NT_MIN, NT_MAX) \
> NT __attribute__((noinline)) \
> sat_s_trunc_##WT##_to_##NT##_fmt_1 (WT x) \
> { \
> NT trunc = (NT)x; \
> return (WT)NT_MIN <= x && x <= (WT)NT_MAX \
> ? trunc \
> : x < 0 ? NT_MIN : NT_MAX; \
> }
>
> DEF_SAT_S_TRUNC_FMT_1(int64_t, int32_t, INT32_MIN, INT32_MAX)
>
> Before this patch:
> 4 │ __attribute__((noinline))
> 5 │ int32_t sat_s_trunc_int64_t_to_int32_t_fmt_1 (int64_t x)
> 6 │ {
> 7 │ int32_t trunc;
> 8 │ unsigned long x.0_1;
> 9 │ unsigned long _2;
> 10 │ int32_t _3;
> 11 │ _Bool _7;
> 12 │ int _8;
> 13 │ int _9;
> 14 │ int _10;
> 15 │
> 16 │ ;; basic block 2, loop depth 0
> 17 │ ;; pred: ENTRY
> 18 │ x.0_1 = (unsigned long) x_4(D);
> 19 │ _2 = x.0_1 + 2147483648;
> 20 │ if (_2 > 4294967295)
> 21 │ goto <bb 4>; [50.00%]
> 22 │ else
> 23 │ goto <bb 3>; [50.00%]
> 24 │ ;; succ: 4
> 25 │ ;; 3
> 26 │
> 27 │ ;; basic block 3, loop depth 0
> 28 │ ;; pred: 2
> 29 │ trunc_5 = (int32_t) x_4(D);
> 30 │ goto <bb 5>; [100.00%]
> 31 │ ;; succ: 5
> 32 │
> 33 │ ;; basic block 4, loop depth 0
> 34 │ ;; pred: 2
> 35 │ _7 = x_4(D) < 0;
> 36 │ _8 = (int) _7;
> 37 │ _9 = -_8;
> 38 │ _10 = _9 ^ 2147483647;
> 39 │ ;; succ: 5
> 40 │
> 41 │ ;; basic block 5, loop depth 0
> 42 │ ;; pred: 3
> 43 │ ;; 4
> 44 │ # _3 = PHI <trunc_5(3), _10(4)>
> 45 │ return _3;
> 46 │ ;; succ: EXIT
> 47 │
> 48 │ }
>
> After this patch:
> 4 │ __attribute__((noinline))
> 5 │ int32_t sat_s_trunc_int64_t_to_int32_t_fmt_1 (int64_t x)
> 6 │ {
> 7 │ int32_t _3;
> 8 │
> 9 │ ;; basic block 2, loop depth 0
> 10 │ ;; pred: ENTRY
> 11 │ _3 = .SAT_TRUNC (x_4(D)); [tail call]
> 12 │ return _3;
> 13 │ ;; succ: EXIT
> 14 │
> 15 │ }
>
> The below test suites are passed for this patch.
> * The rv64gcv fully regression test with pr116861-1.c failed.
> * The x86 bootstrap test.
> * The x86 fully regression test.
>
> The failed pr116861-1.c ice will be fixed in underlying patch, as it
> just trigger one existing bug.
>
> gcc/ChangeLog:
>
> * match.pd: Add case 1 matching pattern for signed SAT_TRUNC.
> * tree-ssa-math-opts.cc (gimple_signed_integer_sat_trunc): Add
> new decl for signed SAT_TRUNC.
> (match_saturation_trunc): Add new func impl to try SAT_TRUNC
> pattern on phi node.
> (math_opts_dom_walker::after_dom_children): Add
> match_saturation_trunc for phi node iteration.
>
> Signed-off-by: Pan Li <pan2...@intel.com>
> ---
> gcc/match.pd | 22 ++++++++++++++
> gcc/tree-ssa-math-opts.cc | 61 +++++++++++++++++++++++++++++++++++++++
> 2 files changed, 83 insertions(+)
>
> diff --git a/gcc/match.pd b/gcc/match.pd
> index d50b732bc86..5108ce45357 100644
> --- a/gcc/match.pd
> +++ b/gcc/match.pd
> @@ -3461,6 +3461,28 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
> }
> (if (wi::eq_p (trunc_max, int_cst_1) && wi::eq_p (max, int_cst_2))))))
>
> +/* Signed saturation truncate, case 1, sizeof (WT) > sizeof (NT).
> + SAT_S_TRUNC(X) = (unsigned)X + NT_MAX + 1 > Unsigned_MAX ? (NT)X. */
> +(match (signed_integer_sat_trunc @0)
> + (cond^ (gt (plus:c (convert@4 @0) INTEGER_CST@1) INTEGER_CST@2)
> + (bit_xor:c (negate (convert (lt @0 integer_zerop))) INTEGER_CST@3)
> + (convert @0))
> + (if (INTEGRAL_TYPE_P (type) && !TYPE_UNSIGNED (type)
> + && !TYPE_UNSIGNED (TREE_TYPE (@0)) && TYPE_UNSIGNED (TREE_TYPE (@4)))
> + (with
> + {
> + unsigned itype_precision = TYPE_PRECISION (TREE_TYPE (@0));
> + unsigned otype_precision = TYPE_PRECISION (type);
> + wide_int offset = wi::uhwi (1ull << (otype_precision - 1),
> itype_precision);
Use HOST_WIDE_INT_1U instead of 1ull
OK with that change.
Richard.
> + wide_int trunc_max = wi::mask (otype_precision, false, itype_precision);
> + wide_int max = wi::mask (otype_precision - 1, false, otype_precision);
> + wide_int int_cst_1 = wi::to_wide (@1);
> + wide_int int_cst_2 = wi::to_wide (@2);
> + wide_int int_cst_3 = wi::to_wide (@3);
> + }
> + (if (wi::eq_p (int_cst_1, offset) && wi::eq_p (int_cst_2, trunc_max)
> + && wi::eq_p (int_cst_3, max))))))
> +
> /* x > y && x != XXX_MIN --> x > y
> x > y && x == XXX_MIN --> false . */
> (for eqne (eq ne)
> diff --git a/gcc/tree-ssa-math-opts.cc b/gcc/tree-ssa-math-opts.cc
> index a61559c52a9..831c244b23a 100644
> --- a/gcc/tree-ssa-math-opts.cc
> +++ b/gcc/tree-ssa-math-opts.cc
> @@ -4025,6 +4025,7 @@ extern bool gimple_unsigned_integer_sat_trunc (tree,
> tree*, tree (*)(tree));
>
> extern bool gimple_signed_integer_sat_add (tree, tree*, tree (*)(tree));
> extern bool gimple_signed_integer_sat_sub (tree, tree*, tree (*)(tree));
> +extern bool gimple_signed_integer_sat_trunc (tree, tree*, tree (*)(tree));
>
> static void
> build_saturation_binary_arith_call (gimple_stmt_iterator *gsi, internal_fn
> fn,
> @@ -4207,6 +4208,65 @@ match_unsigned_saturation_trunc (gimple_stmt_iterator
> *gsi, gassign *stmt)
> }
> }
>
> +/*
> + * Try to match saturation truncate.
> + * Aka:
> + * x.0_1 = (unsigned long) x_4(D);
> + * _2 = x.0_1 + 2147483648;
> + * if (_2 > 4294967295)
> + * goto <bb 4>; [50.00%]
> + * else
> + * goto <bb 3>; [50.00%]
> + * ;; succ: 4
> + * ;; 3
> + *
> + * ;; basic block 3, loop depth 0
> + * ;; pred: 2
> + * trunc_5 = (int32_t) x_4(D);
> + * goto <bb 5>; [100.00%]
> + * ;; succ: 5
> + *
> + * ;; basic block 4, loop depth 0
> + * ;; pred: 2
> + * _7 = x_4(D) < 0;
> + * _8 = (int) _7;
> + * _9 = -_8;
> + * _10 = _9 ^ 2147483647;
> + * ;; succ: 5
> + *
> + * ;; basic block 5, loop depth 0
> + * ;; pred: 3
> + * ;; 4
> + * # _3 = PHI <trunc_5(3), _10(4)>
> + * =>
> + * _6 = .SAT_TRUNC (x_4(D));
> + */
> +
> +static void
> +match_saturation_trunc (gimple_stmt_iterator *gsi, gphi *phi)
> +{
> + if (gimple_phi_num_args (phi) != 2)
> + return;
> +
> + tree ops[1];
> + tree phi_result = gimple_phi_result (phi);
> + tree type = TREE_TYPE (phi_result);
> +
> + if ((gimple_unsigned_integer_sat_trunc (phi_result, ops, NULL)
> + || gimple_signed_integer_sat_trunc (phi_result, ops, NULL))
> + && direct_internal_fn_supported_p (IFN_SAT_TRUNC,
> + tree_pair (type, TREE_TYPE (ops[0])),
> + OPTIMIZE_FOR_BOTH))
> + {
> + gcall *call = gimple_build_call_internal (IFN_SAT_TRUNC, 1, ops[0]);
> + gimple_call_set_lhs (call, phi_result);
> + gsi_insert_before (gsi, call, GSI_SAME_STMT);
> +
> + gimple_stmt_iterator psi = gsi_for_stmt (phi);
> + remove_phi_node (&psi, /* release_lhs_p */ false);
> + }
> +}
> +
> /* Recognize for unsigned x
> x = y - z;
> if (x > y)
> @@ -6142,6 +6202,7 @@ math_opts_dom_walker::after_dom_children (basic_block
> bb)
> /* The match_* may remove phi node. */
> match_saturation_add (&gsi, psi.phi ());
> match_saturation_sub (&gsi, psi.phi ());
> + match_saturation_trunc (&gsi, psi.phi ());
> }
>
> for (gsi = gsi_after_labels (bb); !gsi_end_p (gsi);)
> --
> 2.43.0
>
