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 >