On Tue, Oct 8, 2024 at 10:34 AM <pan2...@intel.com> wrote:
>
> From: Pan Li <pan2...@intel.com>
>
> When try to matching saturation related pattern on PHI node, we may have
> to try each pattern for all phi node of bb. Aka:
>
> for each PHI node in bb:
> gphi *phi = xxx;
> try_match_sat_add (, phi);
> try_match_sat_sub (, phi);
> try_match_sat_trunc (, phi);
>
> The PHI node will be removed if one of the above 3 sat patterns are
> matched. There will be a problem that, for example, sat_add is
> matched and then the phi is removed(freed), and the next 2 sat_sub and
> sat_trunc will depend on the removed(freed) phi node.
>
> This patch would like to fix this consume after phi node released issue.
> To ensure at most one pattern of the above will be matched.
OK.
thanks,
Richard.
> The below test suites are passed for this patch.
> * The rv64gcv fully regression test.
> * The x86 bootstrap test.
> * The x86 fully regression test.
>
> gcc/ChangeLog:
>
> * tree-ssa-math-opts.cc (build_saturation_binary_arith_call): Rename
> to...
> (build_saturation_binary_arith_call_and_replace): ...this.
> (build_saturation_binary_arith_call_and_insert): ...this.
> (match_unsigned_saturation_add): Leverage renamed func.
> (match_unsigned_saturation_sub): Ditto.
> (match_saturation_add): Return bool on matched and leverage
> renamed func.
> (match_saturation_sub): Ditto.
> (match_saturation_trunc): Ditto.
> (math_opts_dom_walker::after_dom_children): Ensure at most one
> pattern will be matched for each phi node.
>
> Signed-off-by: Pan Li <pan2...@intel.com>
> ---
> gcc/tree-ssa-math-opts.cc | 102 +++++++++++++++++++++-----------------
> 1 file changed, 56 insertions(+), 46 deletions(-)
>
> diff --git a/gcc/tree-ssa-math-opts.cc b/gcc/tree-ssa-math-opts.cc
> index 831c244b23a..8e83eb15d91 100644
> --- a/gcc/tree-ssa-math-opts.cc
> +++ b/gcc/tree-ssa-math-opts.cc
> @@ -4028,8 +4028,9 @@ 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,
> - tree lhs, tree op_0, tree op_1)
> +build_saturation_binary_arith_call_and_replace (gimple_stmt_iterator *gsi,
> + internal_fn fn, tree lhs,
> + tree op_0, tree op_1)
> {
> if (direct_internal_fn_supported_p (fn, TREE_TYPE (lhs),
> OPTIMIZE_FOR_BOTH))
> {
> @@ -4039,20 +4040,19 @@ build_saturation_binary_arith_call
> (gimple_stmt_iterator *gsi, internal_fn fn,
> }
> }
>
> -static void
> -build_saturation_binary_arith_call (gimple_stmt_iterator *gsi, gphi *phi,
> - internal_fn fn, tree lhs, tree op_0,
> - tree op_1)
> +static bool
> +build_saturation_binary_arith_call_and_insert (gimple_stmt_iterator *gsi,
> + internal_fn fn, tree lhs,
> + tree op_0, tree op_1)
> {
> - if (direct_internal_fn_supported_p (fn, TREE_TYPE (op_0),
> OPTIMIZE_FOR_BOTH))
> - {
> - gcall *call = gimple_build_call_internal (fn, 2, op_0, op_1);
> - gimple_call_set_lhs (call, lhs);
> - gsi_insert_before (gsi, call, GSI_SAME_STMT);
> + if (!direct_internal_fn_supported_p (fn, TREE_TYPE (op_0),
> OPTIMIZE_FOR_BOTH))
> + return false;
>
> - gimple_stmt_iterator psi = gsi_for_stmt (phi);
> - remove_phi_node (&psi, /* release_lhs_p */ false);
> - }
> + gcall *call = gimple_build_call_internal (fn, 2, op_0, op_1);
> + gimple_call_set_lhs (call, lhs);
> + gsi_insert_before (gsi, call, GSI_SAME_STMT);
> +
> + return true;
> }
>
> /*
> @@ -4072,7 +4072,8 @@ match_unsigned_saturation_add (gimple_stmt_iterator
> *gsi, gassign *stmt)
> tree lhs = gimple_assign_lhs (stmt);
>
> if (gimple_unsigned_integer_sat_add (lhs, ops, NULL))
> - build_saturation_binary_arith_call (gsi, IFN_SAT_ADD, lhs, ops[0],
> ops[1]);
> + build_saturation_binary_arith_call_and_replace (gsi, IFN_SAT_ADD, lhs,
> + ops[0], ops[1]);
> }
>
> /*
> @@ -4114,19 +4115,22 @@ match_unsigned_saturation_add (gimple_stmt_iterator
> *gsi, gassign *stmt)
> * =>
> * _6 = .SAT_ADD (x_5(D), y_6(D)); [tail call] */
>
> -static void
> +static bool
> match_saturation_add (gimple_stmt_iterator *gsi, gphi *phi)
> {
> if (gimple_phi_num_args (phi) != 2)
> - return;
> + return false;
>
> tree ops[2];
> tree phi_result = gimple_phi_result (phi);
>
> - if (gimple_unsigned_integer_sat_add (phi_result, ops, NULL)
> - || gimple_signed_integer_sat_add (phi_result, ops, NULL))
> - build_saturation_binary_arith_call (gsi, phi, IFN_SAT_ADD, phi_result,
> - ops[0], ops[1]);
> + if (!gimple_unsigned_integer_sat_add (phi_result, ops, NULL)
> + && !gimple_signed_integer_sat_add (phi_result, ops, NULL))
> + return false;
> +
> + return build_saturation_binary_arith_call_and_insert (gsi, IFN_SAT_ADD,
> + phi_result, ops[0],
> + ops[1]);
> }
>
> /*
> @@ -4144,7 +4148,8 @@ match_unsigned_saturation_sub (gimple_stmt_iterator
> *gsi, gassign *stmt)
> tree lhs = gimple_assign_lhs (stmt);
>
> if (gimple_unsigned_integer_sat_sub (lhs, ops, NULL))
> - build_saturation_binary_arith_call (gsi, IFN_SAT_SUB, lhs, ops[0],
> ops[1]);
> + build_saturation_binary_arith_call_and_replace (gsi, IFN_SAT_SUB, lhs,
> + ops[0], ops[1]);
> }
>
> /*
> @@ -4163,19 +4168,22 @@ match_unsigned_saturation_sub (gimple_stmt_iterator
> *gsi, gassign *stmt)
> * =>
> * <bb 4> [local count: 1073741824]:
> * _1 = .SAT_SUB (x_2(D), y_3(D)); */
> -static void
> +static bool
> match_saturation_sub (gimple_stmt_iterator *gsi, gphi *phi)
> {
> if (gimple_phi_num_args (phi) != 2)
> - return;
> + return false;
>
> tree ops[2];
> tree phi_result = gimple_phi_result (phi);
>
> - if (gimple_unsigned_integer_sat_sub (phi_result, ops, NULL)
> - || gimple_signed_integer_sat_sub (phi_result, ops, NULL))
> - build_saturation_binary_arith_call (gsi, phi, IFN_SAT_SUB, phi_result,
> - ops[0], ops[1]);
> + if (!gimple_unsigned_integer_sat_sub (phi_result, ops, NULL)
> + && !gimple_signed_integer_sat_sub (phi_result, ops, NULL))
> + return false;
> +
> + return build_saturation_binary_arith_call_and_insert (gsi, IFN_SAT_SUB,
> + phi_result, ops[0],
> + ops[1]);
> }
>
> /*
> @@ -4242,29 +4250,30 @@ match_unsigned_saturation_trunc (gimple_stmt_iterator
> *gsi, gassign *stmt)
> * _6 = .SAT_TRUNC (x_4(D));
> */
>
> -static void
> +static bool
> match_saturation_trunc (gimple_stmt_iterator *gsi, gphi *phi)
> {
> if (gimple_phi_num_args (phi) != 2)
> - return;
> + return false;
>
> 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);
> + if (!gimple_unsigned_integer_sat_trunc (phi_result, ops, NULL)
> + && !gimple_signed_integer_sat_trunc (phi_result, ops, NULL))
> + return false;
>
> - gimple_stmt_iterator psi = gsi_for_stmt (phi);
> - remove_phi_node (&psi, /* release_lhs_p */ false);
> - }
> + if (!direct_internal_fn_supported_p (IFN_SAT_TRUNC,
> + tree_pair (type, TREE_TYPE (ops[0])),
> + OPTIMIZE_FOR_BOTH))
> + return false;
> +
> + 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);
> +
> + return true;
> }
>
> /* Recognize for unsigned x
> @@ -6198,11 +6207,12 @@ math_opts_dom_walker::after_dom_children (basic_block
> bb)
> gsi_next (&psi_next);
>
> gimple_stmt_iterator gsi = gsi_after_labels (bb);
> + gphi *phi = psi.phi ();
>
> - /* The match_* may remove phi node. */
> - match_saturation_add (&gsi, psi.phi ());
> - match_saturation_sub (&gsi, psi.phi ());
> - match_saturation_trunc (&gsi, psi.phi ());
> + if (match_saturation_add (&gsi, phi)
> + || match_saturation_sub (&gsi, phi)
> + || match_saturation_trunc (&gsi, phi))
> + remove_phi_node (&psi, /* release_lhs_p */ false);
> }
>
> for (gsi = gsi_after_labels (bb); !gsi_end_p (gsi);)
> --
> 2.43.0
>
