On Thu, Aug 27, 2026 at 10:31 AM Andrew Pinski <[email protected]> wrote: > > > > 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.
Yeah, I guess that would work. Alternatively explicitly exclude STRING_CST or handle STRING_CST by placing &STRING_CST in the PHI and change the sunk store to be from a dereference. > > >> >> > 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 >> >
