On Sat, Apr 18, 2026 at 9:03 PM Andrew Pinski <[email protected]> wrote: > > Like the previous patches but this is for factoring out operations. > There needs some extra work needed to be handled here due to the > result of the phi changing after the creating the fowarder block. > We need to update the lhs of the last statement in the sequence to > what the phi is. > > phi_on_compare-3.c needed to be updated since phiopt factors out > a cast and now dom2 does one jump threading instead of 3. Note > the code is better in the end. And does not delete a basic-block > but instead merges 2. > ssa-dom-thread-16.c needs to disable phiopt as we factor out a cast > and now can combine one more pair of if statements. This testcase > already disables passes so I thought it was ok to do that too. > > Bootstrapped and tested on x86_64-linux-gnu.
OK. Thanks, Richard. > PR tree-optimization/123113 > gcc/ChangeLog: > > * tree-ssa-phiopt.cc (edges_split): Move earlier. > (make_forwarder): Likewise. > (factor_out_conditional_operation): Handle more than > 2 predecessors. > (pass_phiopt::execute): Don't restrict factoring on > 2 predecessors. > > gcc/testsuite/ChangeLog: > > * gcc.dg/tree-ssa/phi_on_compare-3.c: Update testcase. > * gcc.dg/tree-ssa/ssa-dom-thread-16.c: Add -fno-ssa-phiopt. > * gcc.dg/tree-ssa/phi-opt-factor-2.c: New test. > > Signed-off-by: Andrew Pinski <[email protected]> > --- > .../gcc.dg/tree-ssa/phi-opt-factor-2.c | 31 +++++++ > .../gcc.dg/tree-ssa/phi_on_compare-3.c | 5 +- > .../gcc.dg/tree-ssa/ssa-dom-thread-16.c | 4 +- > gcc/tree-ssa-phiopt.cc | 91 ++++++++++--------- > 4 files changed, 85 insertions(+), 46 deletions(-) > create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/phi-opt-factor-2.c > > diff --git a/gcc/testsuite/gcc.dg/tree-ssa/phi-opt-factor-2.c > b/gcc/testsuite/gcc.dg/tree-ssa/phi-opt-factor-2.c > new file mode 100644 > index 00000000000..8282e6604b8 > --- /dev/null > +++ b/gcc/testsuite/gcc.dg/tree-ssa/phi-opt-factor-2.c > @@ -0,0 +1,31 @@ > +/* { dg-options "-O2 -fdump-tree-phiopt" } */ > + > +/* PR tree-optimization/116699 > + PR tree-optimization/123113 > + Make sure the return PREDICT has no factor in deciding > + if we factor out the conversion. > + Also check that we can do the factoring even if there are > + more than 2 predecessors. */ > + > +short f0(int a, int b, int c, int d) > +{ > + if (d) return 1234; > + int t1 = 4; > + if (c < t1) return (c > -1 ? c : -1); > + return t1; > +} > + > + > +short f1(int a, int b, int c, int d) > +{ > + if (d) return 1234; > + int t1 = 4; > + short t = t1; > + if (c < t1) t = (c > -1 ? c : -1); > + return t; > +} > + > +/* Both f1 and f0 should be optimized at phiopt1 to the same thing. */ > +/* { dg-final { scan-tree-dump-times "MAX_EXPR " 2 "phiopt1" } } */ > +/* { dg-final { scan-tree-dump-times "MIN_EXPR " 2 "phiopt1" } } */ > +/* { dg-final { scan-tree-dump-times "if " 2 "phiopt1" } } */ > diff --git a/gcc/testsuite/gcc.dg/tree-ssa/phi_on_compare-3.c > b/gcc/testsuite/gcc.dg/tree-ssa/phi_on_compare-3.c > index b48ecbf6e61..9e17b16a980 100644 > --- a/gcc/testsuite/gcc.dg/tree-ssa/phi_on_compare-3.c > +++ b/gcc/testsuite/gcc.dg/tree-ssa/phi_on_compare-3.c > @@ -1,5 +1,5 @@ > /* { dg-do compile } */ > -/* { dg-options "-Ofast -fdump-tree-dom2" } */ > +/* { dg-options "-Ofast -fdump-tree-dom2-details" } */ > > void g (void); > void g1 (void); > @@ -22,4 +22,5 @@ f (long a, long b, long c, long d, int x) > } > } > > -/* { dg-final { scan-tree-dump-times "Removing basic block" 1 "dom2" } } */ > +/* { dg-final { scan-tree-dump-times "Merging blocks" 2 "dom2" } } */ > +/* { dg-final { scan-tree-dump-times "Threaded jump" 1 "dom2" } } */ > diff --git a/gcc/testsuite/gcc.dg/tree-ssa/ssa-dom-thread-16.c > b/gcc/testsuite/gcc.dg/tree-ssa/ssa-dom-thread-16.c > index e8555f2d963..d180d4cff5d 100644 > --- a/gcc/testsuite/gcc.dg/tree-ssa/ssa-dom-thread-16.c > +++ b/gcc/testsuite/gcc.dg/tree-ssa/ssa-dom-thread-16.c > @@ -1,5 +1,7 @@ > /* { dg-do compile } */ > -/* { dg-options "-O2 -fdump-tree-dom2-details -w --param > logical-op-non-short-circuit=1 -fdisable-tree-threadfull1" } */ > +/* { dg-options "-O2 -fdump-tree-dom2-details -w --param > logical-op-non-short-circuit=1 -fdisable-tree-threadfull1 -fno-ssa-phiopt" } > */ > +/* Disable phiopt to disable factoring of a cast from bool to `unsigned > char` which removes > + a jump threading opportunity as we instead have `a & b`. */ > unsigned char > validate_subreg (unsigned int offset, unsigned int isize, unsigned int > osize, int zz, int qq) > { > diff --git a/gcc/tree-ssa-phiopt.cc b/gcc/tree-ssa-phiopt.cc > index ac75e117ad3..bdc807bca75 100644 > --- a/gcc/tree-ssa-phiopt.cc > +++ b/gcc/tree-ssa-phiopt.cc > @@ -299,6 +299,42 @@ is_factor_profitable (gimple *def_stmt, basic_block > merge, tree arg) > return true; > } > > +/* Callback function for make_forwarder_block, > + returns true if the edge src is either of > + the 2 basic blocks in the array BBS (DATA). */ > + > +static bool > +edges_split (edge e, void *data) > +{ > + basic_block *bbs = (basic_block*)data; > + return e->src == bbs[0] || e->src == bbs[1]; > +} > + > + > +/* Makes a forwarder block for JOIN_BB such that > + THEN_BB and ELSE_BB are the only 2 predecessors > + of the block. Note the JOIN_BB is required to be > + the same and the PHI nodes of JOIN_BB have to be > + the same too. */ > + > +static void > +make_forwarder (basic_block then_bb, basic_block else_bb, > + basic_block join_bb) > +{ > + basic_block bbs[2]; > + > + /* If the join block already has 2 predecessors, then there > + is nothing to be done. */ > + if (EDGE_COUNT (join_bb->preds) == 2) > + return; > + bbs[0] = then_bb; > + bbs[1] = else_bb; > + edge ne = make_forwarder_block (join_bb, edges_split, bbs); > + gcc_assert (join_bb == ne->src); > + statistics_counter_event (cfun, "if-then-else split for inserting common", > 1); > +} > + > + > /* PR66726: Factor operations out of COND_EXPR. If the arguments of the PHI > stmt are Unary operator, factor out the operation and perform the > operation > to the result of PHI stmt. COND_STMT is the controlling predicate. > @@ -315,9 +351,6 @@ factor_out_conditional_operation (edge e0, edge e1, > basic_block merge, > location_t locus = gimple_location (phi); > gimple_match_op arg0_op, arg1_op; > > - /* We should only get here if the phi had two arguments. */ > - gcc_assert (gimple_phi_num_args (phi) == 2); > - > /* Virtual operands are never handled. */ > if (virtual_operand_p (gimple_phi_result (phi))) > return false; > @@ -523,12 +556,22 @@ factor_out_conditional_operation (edge e0, edge e1, > basic_block merge, > return false; > } > > + /* Create a forwarder block for the common code if needed. */ > + if (EDGE_COUNT (merge->preds) != 2) > + { > + make_forwarder (e0->src, e1->src, merge); > + /* Update the last statement to set the correct result. */ > + gimple *last_seq = *gsi_last (seq); > + result = gimple_phi_result (phi); > + gimple_set_lhs (last_seq, result); > + } > + > if (locus != UNKNOWN_LOCATION) > annotate_all_with_location (seq, locus); > - gsi = gsi_after_labels (gimple_bb (phi)); > + gsi = gsi_after_labels (merge); > gsi_insert_seq_before (&gsi, seq, GSI_CONTINUE_LINKING); > > - newphi = create_phi_node (temp, gimple_bb (phi)); > + newphi = create_phi_node (temp, merge); > > if (dump_file && (dump_flags & TDF_DETAILS)) > { > @@ -2755,43 +2798,6 @@ cond_removal_in_builtin_zero_pattern (basic_block > cond_bb, > return true; > } > > -/* Callback function for make_forwarder_block, > - returns true if the edge src is either of > - the 2 basic blocks in the array BBS (DATA). */ > - > -static bool > -edges_split (edge e, void *data) > -{ > - basic_block *bbs = (basic_block*)data; > - return e->src == bbs[0] || e->src == bbs[1]; > -} > - > - > -/* Makes a forwarder block for JOIN_BB such that > - THEN_BB and ELSE_BB are the only 2 predecessors > - of the block. Note the JOIN_BB is required to be > - the same and the PHI nodes of JOIN_BB have to be > - the same too. */ > - > -static void > -make_forwarder (basic_block then_bb, basic_block else_bb, > - basic_block join_bb) > -{ > - basic_block bbs[2]; > - > - /* If the join block already has 2 predecessors, then there > - is nothing to be done. */ > - if (EDGE_COUNT (join_bb->preds) == 2) > - return; > - bbs[0] = then_bb; > - bbs[1] = else_bb; > - edge ne = make_forwarder_block (join_bb, edges_split, bbs); > - gcc_assert (join_bb == ne->src); > - statistics_counter_event (cfun, "if-then-else split for inserting common", > 1); > -} > - > - > - > /* Auxiliary functions to determine the set of memory accesses which > can't trap because they are preceded by accesses to the same memory > portion. We do that for MEM_REFs, so we only need to track > @@ -4111,7 +4117,6 @@ pass_phiopt::execute (function *) > > /* Factor out operations from the phi if possible. */ > if (single_pred_p (bb1) > - && EDGE_COUNT (merge->preds) == 2 > && !optimize_debug) > { > for (gsi = gsi_start (phis); !gsi_end_p (gsi); ) > -- > 2.43.0 >
