On Mon, Jun 8, 2026 at 2:47 AM Richard Biener
<[email protected]> wrote:
>
> On Fri, Jun 5, 2026 at 7:43 AM Andrew Pinski
> <[email protected]> wrote:
> >
> > r17-1273-g391ee229b737eb added support for the case where the middle bb was
> > non-empty but
> > with a trailing store. This meant if there was a load in the middle bb, it
> > might cause
> > nontrapping to have the lhs in it. So we now need to check for a load in
> > the middle-bb
> > to reject this case.
> >
> > Bootstrapped and tested on x86_64-linux-gnu.
> >
> > PR tree-optimization/125612
> >
> > gcc/ChangeLog:
> >
> > * tree-ssa-phiopt.cc (cond_store_replacement): For the case where
> > lhs is "known" to be nontrapping make sure there are no loads in
> > the middle bb.
> >
> > gcc/testsuite/ChangeLog:
> >
> > * gcc.dg/tree-ssa/pr125612-1.c: New test.
> >
> > Signed-off-by: Andrew Pinski <[email protected]>
> > ---
> > gcc/testsuite/gcc.dg/tree-ssa/pr125612-1.c | 26 ++++++++++++++++++++++
> > gcc/tree-ssa-phiopt.cc | 19 +++++++++++++++-
> > 2 files changed, 44 insertions(+), 1 deletion(-)
> > create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/pr125612-1.c
> >
> > diff --git a/gcc/testsuite/gcc.dg/tree-ssa/pr125612-1.c
> > b/gcc/testsuite/gcc.dg/tree-ssa/pr125612-1.c
> > new file mode 100644
> > index 00000000000..201f2f4fb3f
> > --- /dev/null
> > +++ b/gcc/testsuite/gcc.dg/tree-ssa/pr125612-1.c
> > @@ -0,0 +1,26 @@
> > +/* { dg-do run } */
> > +/* { dg-options "-O3 -fno-tree-dse -fno-tree-dce
> > -fdump-tree-cselim-details" } */
> > +/* PR tree-optimization/125612 */
> > +
> > +/* The conditional store of `h[2000]` should not become unconditional since
> > + it traps. */
> > +
> > +int a, b, c = 2, d, e[4], f, i;
> > +void g() {
> > + int h[5];
> > + e[0] = 2;
> > + while (1) {
> > + if (e[b]) {
> > + if (!c)
> > + h[2000] &= 1;
> > + return;
> > + }
> > + }
> > +}
> > +int main() {
> > + for (; i < 4; i++)
> > + g();
> > + return 0;
> > +}
> > +
> > +/* { dg-final { scan-tree-dump-not "Conditional store replacement
> > happened" "cselim"} } */
> > diff --git a/gcc/tree-ssa-phiopt.cc b/gcc/tree-ssa-phiopt.cc
> > index 3e7dd3b0ee3..0c5c4982d3f 100644
> > --- a/gcc/tree-ssa-phiopt.cc
> > +++ b/gcc/tree-ssa-phiopt.cc
> > @@ -3089,7 +3089,24 @@ cond_store_replacement (basic_block middle_bb,
> > basic_block join_bb, edge e0,
> > /* Prove that we can move the store down. We could also check
> > TREE_THIS_NOTRAP here, but in that case we also could move stores,
> > whose value is not available readily, which we want to avoid. */
> > - if (!nontrap->contains (lhs))
> > + if (nontrap->contains (lhs))
> > + {
> > + /* Make sure there is no load in the middle bb,
> > + this invalidates nontrap.
>
> How so? nontrap should only contain the load, not the store? We mark
> a MEM as not trapping if there's a dominating same MEM only and
> nontrap contains the actual tree (which is always unshared)
Right, there is a load from the same lhs that dominates the same MEM but only
in that branch. e.g.
```
if (a)
{
int t = d[1024];
t = t | 1;
d[1024] = t;
}
```
Here the load is dominating the store. I could have checked to see if
the load was the same but I didn't think it was that useful. Since
this just disables the transformation to what was done before
Pengxuan's patch in the case of the load there; I thought this was the
safest thing to do.
Thanks,
Andrea
>
> > + FIXME: this is over conserative, this check could be made to
> > + allow loads unrelated to lhs. */
> > + tree vuse = gimple_vuse (assign);
> > + imm_use_iterator iter;
> > + gimple *use_stmt;
> > + FOR_EACH_IMM_USE_STMT (use_stmt, iter, vuse)
> > + {
> > + if (use_stmt == assign)
> > + continue;
> > + if (gimple_bb (use_stmt) == middle_bb)
> > + return false;
> > + }
> > + }
> > + else
> > {
> > /* If LHS is an access to a local variable without address-taken
> > (or when we allow data races) and known not to trap, we could
> > --
> > 2.43.0
> >
