On Thu, Aug 27, 2026, 1:23 AM Richard Biener <[email protected]> wrote:
> On Thu, Aug 27, 2026 at 1:31 AM Andrea Pinski > <[email protected]> wrote: > > > > So when I wrote this code I limited the middle store to only > > be a SSA name. This is too limited and we reject constants which the > full > > cselim can handle just fine. This extends the check to support gimple > > min invariants too. > > > > Also the check for clobber is changed to be instead to be ssa name/min > invariant > > instead. This will still reject clobbers correctly and support what we > can support > > here. > > > > This was found while looking into std::optional code generation in some > cases. > > Note this includes stores from STRING_CST - I think you basically > want is_gimple_reg_type () instead? > There is a check for is_gimple_reg_type already right before the check for min invariant/ssa name. Just a check for is_gimple_reg_type will allow in clobbers. So maybe check should be !clobbers after the check for is_gimple_reg_type. > > Bootstrapped and tested on x86_64-linux-gnu. > > > > PR tree-optimization/127052 > > > > gcc/ChangeLog: > > > > * tree-ssa-phiopt.cc (cond_store_replacement_limited): Extend > check > > to support gimple min invariants. > > Change check for non clobbers to be ssa name or min invariant. > > > > gcc/testsuite/ChangeLog: > > > > * gcc.dg/tree-ssa/cselim-6.c: New test. > > * gcc.dg/tree-ssa/cselim-7.c: New test. > > > > Signed-off-by: Andrea Pinski <[email protected]> > > --- > > gcc/testsuite/gcc.dg/tree-ssa/cselim-6.c | 17 +++++++++++++++++ > > gcc/testsuite/gcc.dg/tree-ssa/cselim-7.c | 18 ++++++++++++++++++ > > gcc/tree-ssa-phiopt.cc | 10 ++++++---- > > 3 files changed, 41 insertions(+), 4 deletions(-) > > create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/cselim-6.c > > create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/cselim-7.c > > > > diff --git a/gcc/testsuite/gcc.dg/tree-ssa/cselim-6.c > b/gcc/testsuite/gcc.dg/tree-ssa/cselim-6.c > > new file mode 100644 > > index 00000000000..54d1446ecd7 > > --- /dev/null > > +++ b/gcc/testsuite/gcc.dg/tree-ssa/cselim-6.c > > @@ -0,0 +1,17 @@ > > +/* { dg-do compile } */ > > +/* { dg-options "-O2 -ftree-cselim -fdump-tree-phiopt1-details" } */ > > +/* PR tree-optimization/127052 */ > > + > > +int *sink(int*); > > +void f(int a, int c, int d, int *e) > > +{ > > + e = sink(&a); > > + a = d; > > + c = *e; > > + c += a; > > + if (c) > > + a = 0; > > + sink(&a); > > +} > > + > > +/* { dg-final { scan-tree-dump "Conditional store replacement" > "phiopt1" } } */ > > diff --git a/gcc/testsuite/gcc.dg/tree-ssa/cselim-7.c > b/gcc/testsuite/gcc.dg/tree-ssa/cselim-7.c > > new file mode 100644 > > index 00000000000..8db89c24b56 > > --- /dev/null > > +++ b/gcc/testsuite/gcc.dg/tree-ssa/cselim-7.c > > @@ -0,0 +1,18 @@ > > +/* { dg-do compile } */ > > +/* { dg-options "-O2 -ftree-cselim -fdump-tree-phiopt1-details" } */ > > +/* PR tree-optimization/127052 */ > > + > > +void sink(int*); > > + > > +int f(int b) > > +{ > > + int a; > > + sink(&a); > > + a = b; > > + if (a) > > + a = 0; > > + return a; > > +} > > + > > + > > +/* { dg-final { scan-tree-dump "Conditional store replacement" > "phiopt1" } } */ > > diff --git a/gcc/tree-ssa-phiopt.cc b/gcc/tree-ssa-phiopt.cc > > index f8dbd26abc1..ddb2e353cf4 100644 > > --- a/gcc/tree-ssa-phiopt.cc > > +++ b/gcc/tree-ssa-phiopt.cc > > @@ -3655,7 +3655,8 @@ cond_store_replacement_limited (basic_block > middle_bb, basic_block join_bb, > > && !DECL_P (lhs)) > > || !is_gimple_reg_type (TREE_TYPE (lhs))) > > return false; > > - if (TREE_CODE (rhs) != SSA_NAME) > > + if (TREE_CODE (rhs) != SSA_NAME > > + && !is_gimple_min_invariant (rhs)) > > return false; > > > > /* Three cases that can be handled: > > @@ -3678,11 +3679,12 @@ cond_store_replacement_limited (basic_block > middle_bb, basic_block join_bb, > > tree beforelhs = gimple_assign_lhs (vdef_before); > > /* Only allow the store to be right before the condition. */ > > if (gimple_bb (vdef_before) == cond_bb > > - /* This can't be a clobber */ > > - && !gimple_clobber_p (vdef_before) > > /* An exact match is only supported. > > FIXME: Allow for clique/base mismatch? */ > > - && operand_equal_p (lhs, beforelhs)) > > + && operand_equal_p (lhs, beforelhs) > > + // The rhs needs to be a ssa name or a min invariant. > > + && (TREE_CODE (gimple_assign_rhs1 (vdef_before)) == SSA_NAME > > + || is_gimple_min_invariant (gimple_assign_rhs1 > (vdef_before)))) > > { > > /* The vuse of the of store in the middle should be also > > the entry in the phi for the other edge. */ > > -- > > 2.43.0 > > >
