On Sun, May 7, 2023 at 6:44 AM Andrew Pinski via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> After using factor_out_conditional_conversion with diamond bb,
> we should be able do use it also for all normal unary gimple and not
> just conversions. This allows to optimize PR 59424 for an example.
> This is also a start to optimize PR 64700 and a few others.
>
> OK? Bootstrapped and tested on x86_64-linux-gnu with no regressions.
OK.
> An example of this is:
> ```
> static inline unsigned long long g(int t)
> {
> unsigned t1 = t;
> return t1;
> }
> static int abs1(int a)
> {
> if (a < 0)
> a = -a;
> return a;
> }
> unsigned long long f(int c, int d, int e)
> {
> unsigned long long t;
> if (d > e)
> t = g(abs1(d));
> else
> t = g(abs1(e));
> return t;
> }
> ```
>
> Which should be optimized to:
> _9 = MAX_EXPR <d_5(D), e_6(D)>;
> _4 = ABS_EXPR <_9>;
> t_3 = (long long unsigned intD.16) _4;
>
> gcc/ChangeLog:
>
> * tree-ssa-phiopt.cc (factor_out_conditional_conversion): Rename to
> ...
> (factor_out_conditional_operation): This and add support for all unary
> operations.
> (pass_phiopt::execute): Update call to
> factor_out_conditional_conversion
> to call factor_out_conditional_operation instead.
>
> PR tree-optimization/109424
> PR tree-optimization/59424
>
> gcc/testsuite/ChangeLog:
>
> * gcc.dg/tree-ssa/abs-2.c: Update tree scan for
> details change in wording.
> * gcc.dg/tree-ssa/minmax-17.c: Likewise.
> * gcc.dg/tree-ssa/pr103771.c: Likewise.
> * gcc.dg/tree-ssa/minmax-18.c: New test.
> * gcc.dg/tree-ssa/minmax-19.c: New test.
> ---
> gcc/testsuite/gcc.dg/tree-ssa/abs-2.c | 2 +-
> gcc/testsuite/gcc.dg/tree-ssa/minmax-17.c | 2 +-
> gcc/testsuite/gcc.dg/tree-ssa/minmax-18.c | 27 +++++++++++
> gcc/testsuite/gcc.dg/tree-ssa/minmax-19.c | 10 ++++
> gcc/testsuite/gcc.dg/tree-ssa/pr103771.c | 2 +-
> gcc/tree-ssa-phiopt.cc | 56 +++++++++++++----------
> 6 files changed, 71 insertions(+), 28 deletions(-)
> create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/minmax-18.c
> create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/minmax-19.c
>
> diff --git a/gcc/testsuite/gcc.dg/tree-ssa/abs-2.c
> b/gcc/testsuite/gcc.dg/tree-ssa/abs-2.c
> index 328b1802541..f8bbeb43237 100644
> --- a/gcc/testsuite/gcc.dg/tree-ssa/abs-2.c
> +++ b/gcc/testsuite/gcc.dg/tree-ssa/abs-2.c
> @@ -16,5 +16,5 @@ test_abs(int *cur)
>
> /* We should figure out that test_abs has an ABS_EXPR in it. */
> /* { dg-final { scan-tree-dump " = ABS_EXPR" "phiopt1"} } */
> -/* { dg-final { scan-tree-dump-times "changed to factor conversion out from"
> 1 "phiopt1"} } */
> +/* { dg-final { scan-tree-dump-times "changed to factor operation out from"
> 1 "phiopt1"} } */
>
> diff --git a/gcc/testsuite/gcc.dg/tree-ssa/minmax-17.c
> b/gcc/testsuite/gcc.dg/tree-ssa/minmax-17.c
> index bd737e6b4cb..7c76cfc62a9 100644
> --- a/gcc/testsuite/gcc.dg/tree-ssa/minmax-17.c
> +++ b/gcc/testsuite/gcc.dg/tree-ssa/minmax-17.c
> @@ -18,4 +18,4 @@ unsigned long long test_max(int c, int d, int e)
>
> /* We should figure out that test_max has an MAX_EXPR in it. */
> /* { dg-final { scan-tree-dump " = MAX_EXPR" "phiopt1"} } */
> -/* { dg-final { scan-tree-dump-times "changed to factor conversion out from"
> 2 "phiopt1"} } */
> +/* { dg-final { scan-tree-dump-times "changed to factor operation out from"
> 2 "phiopt1"} } */
> diff --git a/gcc/testsuite/gcc.dg/tree-ssa/minmax-18.c
> b/gcc/testsuite/gcc.dg/tree-ssa/minmax-18.c
> new file mode 100644
> index 00000000000..c8e1670f64a
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/tree-ssa/minmax-18.c
> @@ -0,0 +1,27 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O -fdump-tree-phiopt1-details" } */
> +
> +static inline unsigned long long g(int t)
> +{
> + unsigned t1 = t;
> + return t1;
> +}
> +static inline int abs1(int a)
> +{
> + if (a < 0)
> + a = -a;
> + return a;
> +}
> +unsigned long long f(int c, int d, int e)
> +{
> + unsigned long long t;
> + if (d > e)
> + t = g(abs1(d));
> + else
> + t = g(abs1(e));
> + return t;
> +}
> +
> +/* { dg-final { scan-tree-dump " = MAX_EXPR" "phiopt1"} } */
> +/* { dg-final { scan-tree-dump-times " = ABS_EXPR" 2 "phiopt1"} } */
> +/* { dg-final { scan-tree-dump-times "changed to factor operation out from"
> 3 "phiopt1"} } */
> diff --git a/gcc/testsuite/gcc.dg/tree-ssa/minmax-19.c
> b/gcc/testsuite/gcc.dg/tree-ssa/minmax-19.c
> new file mode 100644
> index 00000000000..5ed55fe2e23
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/tree-ssa/minmax-19.c
> @@ -0,0 +1,10 @@
> +/* PR tree-optimization/109424 */
> +/* { dg-do compile } */
> +/* { dg-options "-O -fdump-tree-phiopt1-details" } */
> +
> +int f2(int x, int y)
> +{
> + return (x > y) ? ~x : ~y;
> +}
> +/* { dg-final { scan-tree-dump " = MAX_EXPR" "phiopt1"} } */
> +/* { dg-final { scan-tree-dump-times "changed to factor operation out from"
> 1 "phiopt1"} } */
> diff --git a/gcc/testsuite/gcc.dg/tree-ssa/pr103771.c
> b/gcc/testsuite/gcc.dg/tree-ssa/pr103771.c
> index 97c9db846cb..8faa45a8222 100644
> --- a/gcc/testsuite/gcc.dg/tree-ssa/pr103771.c
> +++ b/gcc/testsuite/gcc.dg/tree-ssa/pr103771.c
> @@ -1,6 +1,6 @@
> /* { dg-do compile } */
> /* { dg-options "-O3 -fdump-tree-phiopt1-details" } */
> -/* { dg-final { scan-tree-dump-times "changed to factor conversion out from
> COND_EXPR." 1 "phiopt1" } } */
> +/* { dg-final { scan-tree-dump-times "changed to factor operation out from
> COND_EXPR." 1 "phiopt1" } } */
>
> typedef unsigned char uint8_t;
>
> diff --git a/gcc/tree-ssa-phiopt.cc b/gcc/tree-ssa-phiopt.cc
> index 7fe088b13ff..2fb28b4e60e 100644
> --- a/gcc/tree-ssa-phiopt.cc
> +++ b/gcc/tree-ssa-phiopt.cc
> @@ -209,13 +209,13 @@ replace_phi_edge_with_variable (basic_block cond_block,
> bb->index);
> }
>
> -/* PR66726: Factor conversion out of COND_EXPR. If the arguments of the PHI
> +/* PR66726: Factor operations out of COND_EXPR. If the arguments of the PHI
> stmt are CONVERT_STMT, factor out the conversion and perform the
> conversion
> to the result of PHI stmt. COND_STMT is the controlling predicate.
> Return the newly-created PHI, if any. */
>
> static gphi *
> -factor_out_conditional_conversion (edge e0, edge e1, gphi *phi,
> +factor_out_conditional_operation (edge e0, edge e1, gphi *phi,
> tree arg0, tree arg1, gimple *cond_stmt)
> {
> gimple *arg0_def_stmt = NULL, *arg1_def_stmt = NULL, *new_stmt;
> @@ -224,7 +224,7 @@ factor_out_conditional_conversion (edge e0, edge e1, gphi
> *phi,
> gphi *newphi;
> gimple_stmt_iterator gsi, gsi_for_def;
> location_t locus = gimple_location (phi);
> - enum tree_code convert_code;
> + enum tree_code op_code;
>
> /* Handle only PHI statements with two arguments. TODO: If all
> other arguments to PHI are INTEGER_CST or if their defining
> @@ -246,15 +246,17 @@ factor_out_conditional_conversion (edge e0, edge e1,
> gphi *phi,
> return NULL;
>
> /* Check if arg0 is an SSA_NAME and the stmt which defines arg0 is
> - a conversion. */
> + an unary operation. */
> arg0_def_stmt = SSA_NAME_DEF_STMT (arg0);
> - if (!gimple_assign_cast_p (arg0_def_stmt))
> + if (!is_gimple_assign (arg0_def_stmt)
> + || (gimple_assign_rhs_class (arg0_def_stmt) != GIMPLE_UNARY_RHS
> + && gimple_assign_rhs_code (arg0_def_stmt) != VIEW_CONVERT_EXPR))
> return NULL;
>
> /* Use the RHS as new_arg0. */
> - convert_code = gimple_assign_rhs_code (arg0_def_stmt);
> + op_code = gimple_assign_rhs_code (arg0_def_stmt);
> new_arg0 = gimple_assign_rhs1 (arg0_def_stmt);
> - if (convert_code == VIEW_CONVERT_EXPR)
> + if (op_code == VIEW_CONVERT_EXPR)
> {
> new_arg0 = TREE_OPERAND (new_arg0, 0);
> if (!is_gimple_reg_type (TREE_TYPE (new_arg0)))
> @@ -267,10 +269,10 @@ factor_out_conditional_conversion (edge e0, edge e1,
> gphi *phi,
> if (TREE_CODE (arg1) == SSA_NAME)
> {
> /* Check if arg1 is an SSA_NAME and the stmt which defines arg1
> - is a conversion. */
> + is an unary operation. */
> arg1_def_stmt = SSA_NAME_DEF_STMT (arg1);
> - if (!is_gimple_assign (arg1_def_stmt)
> - || gimple_assign_rhs_code (arg1_def_stmt) != convert_code)
> + if (!is_gimple_assign (arg1_def_stmt)
> + || gimple_assign_rhs_code (arg1_def_stmt) != op_code)
> return NULL;
>
> /* Either arg1_def_stmt or arg0_def_stmt should be conditional. */
> @@ -281,7 +283,7 @@ factor_out_conditional_conversion (edge e0, edge e1, gphi
> *phi,
>
> /* Use the RHS as new_arg1. */
> new_arg1 = gimple_assign_rhs1 (arg1_def_stmt);
> - if (convert_code == VIEW_CONVERT_EXPR)
> + if (op_code == VIEW_CONVERT_EXPR)
> new_arg1 = TREE_OPERAND (new_arg1, 0);
> if (TREE_CODE (new_arg1) == SSA_NAME
> && SSA_NAME_OCCURS_IN_ABNORMAL_PHI (new_arg1))
> @@ -289,6 +291,10 @@ factor_out_conditional_conversion (edge e0, edge e1,
> gphi *phi,
> }
> else
> {
> + /* TODO: handle more than just casts here. */
> + if (!gimple_assign_cast_p (arg0_def_stmt))
> + return NULL;
> +
> /* arg0_def_stmt should be conditional. */
> if (dominated_by_p (CDI_DOMINATORS, gimple_bb (phi), gimple_bb
> (arg0_def_stmt)))
> return NULL;
> @@ -366,13 +372,13 @@ factor_out_conditional_conversion (edge e0, edge e1,
> gphi *phi,
> fprintf (dump_file, "PHI ");
> print_generic_expr (dump_file, gimple_phi_result (phi));
> fprintf (dump_file,
> - " changed to factor conversion out from COND_EXPR.\n");
> - fprintf (dump_file, "New stmt with CAST that defines ");
> + " changed to factor operation out from COND_EXPR.\n");
> + fprintf (dump_file, "New stmt with OPERATION that defines ");
> print_generic_expr (dump_file, result);
> fprintf (dump_file, ".\n");
> }
>
> - /* Remove the old cast(s) that has single use. */
> + /* Remove the old operation(s) that has single use. */
> gsi_for_def = gsi_for_stmt (arg0_def_stmt);
> gsi_remove (&gsi_for_def, true);
> release_defs (arg0_def_stmt);
> @@ -387,14 +393,14 @@ factor_out_conditional_conversion (edge e0, edge e1,
> gphi *phi,
> add_phi_arg (newphi, new_arg0, e0, locus);
> add_phi_arg (newphi, new_arg1, e1, locus);
>
> - /* Create the conversion stmt and insert it. */
> - if (convert_code == VIEW_CONVERT_EXPR)
> + /* Create the operation stmt and insert it. */
> + if (op_code == VIEW_CONVERT_EXPR)
> {
> temp = fold_build1 (VIEW_CONVERT_EXPR, TREE_TYPE (result), temp);
> new_stmt = gimple_build_assign (result, temp);
> }
> else
> - new_stmt = gimple_build_assign (result, convert_code, temp);
> + new_stmt = gimple_build_assign (result, op_code, temp);
> gsi = gsi_after_labels (gimple_bb (phi));
> gsi_insert_before (&gsi, new_stmt, GSI_SAME_STMT);
>
> @@ -402,7 +408,7 @@ factor_out_conditional_conversion (edge e0, edge e1, gphi
> *phi,
> gsi = gsi_for_stmt (phi);
> gsi_remove (&gsi, true);
>
> - statistics_counter_event (cfun, "factored out cast", 1);
> + statistics_counter_event (cfun, "factored out operation", 1);
>
> return newphi;
> }
> @@ -3847,11 +3853,11 @@ gate_hoist_loads (void)
> This pass also performs a fifth transformation of a slightly different
> flavor.
>
> - Factor conversion in COND_EXPR
> + Factor operations in COND_EXPR
> ------------------------------
>
> - This transformation factors the conversion out of COND_EXPR with
> - factor_out_conditional_conversion.
> + This transformation factors the unary operations out of COND_EXPR with
> + factor_out_conditional_operation.
>
> For example:
> if (a <= CST) goto <bb 3>; else goto <bb 4>;
> @@ -4092,15 +4098,15 @@ pass_phiopt::execute (function *)
> while (newphi)
> {
> phi = newphi;
> - /* factor_out_conditional_conversion may create a new PHI in
> + /* factor_out_conditional_operation may create a new PHI in
> BB2 and eliminate an existing PHI in BB2. Recompute values
> that may be affected by that change. */
> arg0 = gimple_phi_arg_def (phi, e1->dest_idx);
> arg1 = gimple_phi_arg_def (phi, e2->dest_idx);
> gcc_assert (arg0 != NULL_TREE && arg1 != NULL_TREE);
> - newphi = factor_out_conditional_conversion (e1, e2, phi,
> - arg0, arg1,
> - cond_stmt);
> + newphi = factor_out_conditional_operation (e1, e2, phi,
> + arg0, arg1,
> + cond_stmt);
> }
> }
>
> --
> 2.31.1
>
