On Sat, Apr 18, 2026 at 6:29 AM Andrew Pinski <[email protected]> wrote: > > Since adding the new function make_forwarder to tree-ssa-phiopt.cc, we > can use this to remove the restriction of the join back having 2 > predecessors for cselim and allow to optimize more cases. > We only create the new forwarder block on demand when doing the final > replacement so that we don't create basic blocks which would be cleaned up > otherwise. > > Bootstrapped and tested on x86_64-linux-gnu.
OK. Thanks, Richard. > PR tree-optimization/89430 > PR tree-optimization/123113 > > gcc/ChangeLog: > > * tree-ssa-phiopt.cc (edges_split): Move up in the file. > (make_forwarder): Likewise. > (cond_store_replacement): Use make_forwarder if the join > block has more than 2 predecessors. > (pass_cselim::execute): Remove the checks on the join > blocks having more than 2 predecessors. > > gcc/testsuite/ChangeLog: > > * gcc.dg/tree-ssa/pr89430-1a.c: New test. > > Signed-off-by: Andrew Pinski <[email protected]> > --- > gcc/testsuite/gcc.dg/tree-ssa/pr89430-1a.c | 16 ++++ > gcc/tree-ssa-phiopt.cc | 90 ++++++++++------------ > 2 files changed, 58 insertions(+), 48 deletions(-) > create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/pr89430-1a.c > > diff --git a/gcc/testsuite/gcc.dg/tree-ssa/pr89430-1a.c > b/gcc/testsuite/gcc.dg/tree-ssa/pr89430-1a.c > new file mode 100644 > index 00000000000..09534f628f2 > --- /dev/null > +++ b/gcc/testsuite/gcc.dg/tree-ssa/pr89430-1a.c > @@ -0,0 +1,16 @@ > +/* { dg-do compile } */ > +/* { dg-options "-O2 -ftree-cselim -fdump-tree-cselim-details" } */ > +/* PR tree-optimization/89430 */ > +/* PR tree-optimization/123113 */ > + > +unsigned test(unsigned k, unsigned b, int c) { > + unsigned a[2] = {1,2}; > + if (c) { > + if (b < a[k]) { > + a[k] = b; > + } > + } > + return a[0]+a[1]; > +} > + > +/* { dg-final { scan-tree-dump "Conditional store replacement" "cselim" } } > */ > diff --git a/gcc/tree-ssa-phiopt.cc b/gcc/tree-ssa-phiopt.cc > index 76527ff922c..ac75e117ad3 100644 > --- a/gcc/tree-ssa-phiopt.cc > +++ b/gcc/tree-ssa-phiopt.cc > @@ -2755,6 +2755,43 @@ 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 > @@ -3047,7 +3084,10 @@ cond_store_replacement (basic_block middle_bb, > basic_block join_bb, > } > > /* Now we've checked the constraints, so do the transformation: > - 1) Remove the single store. */ > + 0) Create a forwarder block if needed. */ > + if (EDGE_COUNT (join_bb->preds) > 2) > + make_forwarder (single_pred (middle_bb), middle_bb, join_bb); > + /* 1) Remove the single store. */ > gsi = gsi_for_stmt (assign); > unlink_stmt_vdef (assign); > gsi_remove (&gsi, true); > @@ -3115,42 +3155,6 @@ cond_store_replacement (basic_block middle_bb, > basic_block join_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); > -} > - > - > /* Do the main work of conditional store replacement. */ > > static bool > @@ -4278,13 +4282,6 @@ pass_cselim::execute (function *) > if (diamond_p) > { > basic_block bb3 = e1->dest; > - > - /* Only handle sinking of store from 2 bbs only, > - The middle bbs don't need to come from the > - if always since we are sinking rather than > - hoisting. */ > - if (EDGE_COUNT (bb3->preds) != 2) > - return; > if (cond_if_else_store_replacement (bb1, bb2, bb3)) > cfgchanged = true; > return; > @@ -4297,10 +4294,7 @@ pass_cselim::execute (function *) > return; > > /* bb1 is the middle block, bb2 the join block, bb the split block, > - e1 the fallthrough edge from bb1 to bb2. We can't do the > - optimization if the join block has more than two predecessors. */ > - if (EDGE_COUNT (bb2->preds) > 2) > - return; > + e1 the fallthrough edge from bb1 to bb2. */ > if (cond_store_replacement (bb1, bb2, e1, e2, nontrap)) > cfgchanged = true; > }; > -- > 2.43.0 >
