On Mon, Dec 8, 2025 at 4:18 AM Richard Biener
<[email protected]> wrote:
>
> On Mon, Dec 8, 2025 at 1:17 PM Richard Biener
> <[email protected]> wrote:
> >
> > On Sat, Dec 6, 2025 at 8:39 PM Andrew Pinski
> > <[email protected]> wrote:
> > >
> > > This fixes a regression introduced with r16-5258-g1d8e2d51e5c5cb.
> > > With GCC 12+, we would not merge forwarders (with phis, vops included),
> > > this meant that after the last cddce, degenerate phis would stay not
> > > merged which allowed for better expansion. Now after my patch, the 
> > > forwarder
> > > block would be removed and get worse expansion. This fixes the problem
> > > by creating the forwarder blocks in "optimized" and no other cleanupcfg
> > > is called afterwards.
> > >
> > > Oh this also fixes the problem at -O1 which was missed because the 
> > > agressive
> > > version of dce was not done at -O1.
> >
> > OK for this series.  I thought of this at some point but specifically wanted
> > to look at interaction with uncprop and whether that eventually causes
> > degenerate constant cases to disappear when different equality compares
> > feed the values.
>
> Oh, and both things are really out-of-SSA optimizations, so doing those as
> part of rewrite_out_of_ssa might make sense as well.

So from what I understand is the old out-of-ssa (the pre-TER/expand
directly from gimple one) used to handle this degenerated case.
I do think this should be part of rewrite out of ssa (that part needs
some more love really). And uncprop should almost definitely be part
of it. I have seen uncprop do worse code in some examples
(https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107099,
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109483 and others).

>
> Richard.
>
> > Thanks,
> > Richard.
> >
> > > Bootstrapped and tested on x86_64-linux-gnu.
> > >
> > >         PR tree-optimization/46555
> > > gcc/ChangeLog:
> > >
> > >         * tree-cfgcleanup.cc (execute_cleanup_cfg_post_optimizing):
> > >         Don't set todo to include cleanupcfg; do it manually.
> > >         Call make_forwarders_with_degenerate_phis if optimizing.
> > >
> > > gcc/testsuite/ChangeLog:
> > >
> > >         * gcc.dg/tree-ssa/pr46555.c: New test.
> > >
> > > Signed-off-by: Andrew Pinski <[email protected]>
> > > ---
> > >  gcc/testsuite/gcc.dg/tree-ssa/pr46555.c | 28 +++++++++++++++++++++++++
> > >  gcc/tree-cfgcleanup.cc                  |  9 ++++++--
> > >  2 files changed, 35 insertions(+), 2 deletions(-)
> > >  create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/pr46555.c
> > >
> > > diff --git a/gcc/testsuite/gcc.dg/tree-ssa/pr46555.c 
> > > b/gcc/testsuite/gcc.dg/tree-ssa/pr46555.c
> > > new file mode 100644
> > > index 00000000000..d4de7c2c170
> > > --- /dev/null
> > > +++ b/gcc/testsuite/gcc.dg/tree-ssa/pr46555.c
> > > @@ -0,0 +1,28 @@
> > > +/* { dg-do compile } */
> > > +/* { dg-options "-O2 -fdump-tree-optimized-details 
> > > -fdump-rtl-pro_and_epilogue" } */
> > > +/* PR tree-optimization/46555 */
> > > +/* Here should not remove the forwarder block (or rather recreate it and 
> > > not
> > > +   remove it again). This improves expansion to RTL as there is one less 
> > > copy
> > > +   (or constant formation) in some cases. In this case we also get the 
> > > ability
> > > +   to shrink wrap the function.  */
> > > +
> > > +int h(void);
> > > +int f(int a, int b, int c)
> > > +{
> > > +  if (a)
> > > +    return 2;
> > > +  h();
> > > +  if (b)
> > > +    return 2;
> > > +  h();
> > > +  if (c)
> > > +    return 2;
> > > +  h();
> > > +  return 4;
> > > +}
> > > +
> > > +/* { dg-final { scan-tree-dump-times "New forwarder block for edge" 1 
> > > "optimized" } } */
> > > +/* Make sure we only have a PHI with 2 arguments here, 2 and 4.  */
> > > +/* { dg-final { scan-tree-dump "PHI <2..., 4...>|PHI <4..., 2...>" 
> > > "optimized" } } */
> > > +/* Make sure we can shrink wrap the function now too. */
> > > +/* { dg-final { scan-rtl-dump "Performing shrink-wrapping" 
> > > "pro_and_epilogue" { target { { { i?86-*-* x86_64-*-* } && { ! ia32 } } 
> > > || { powerpc*-*-* aarch64*-*-* riscv*-*-* arm*-*-* }  } } } } */
> > > diff --git a/gcc/tree-cfgcleanup.cc b/gcc/tree-cfgcleanup.cc
> > > index 872ded3d15e..093fde93da6 100644
> > > --- a/gcc/tree-cfgcleanup.cc
> > > +++ b/gcc/tree-cfgcleanup.cc
> > > @@ -1415,8 +1415,13 @@ execute_cleanup_cfg_post_optimizing (void)
> > >      }
> > >    maybe_remove_unreachable_handlers ();
> > >    cleanup_dead_labels ();
> > > -  if (group_case_labels ())
> > > -    todo |= TODO_cleanup_cfg;
> > > +  if (group_case_labels () && cleanup_tree_cfg ())
> > > +    todo |= TODO_update_ssa;
> > > +
> > > +  /* When optimizing undo the merging of forwarder blocks
> > > +     that phis for better out of ssa expansion.  */
> > > +  if (optimize)
> > > +    make_forwarders_with_degenerate_phis (cfun);
> > >
> > >    basic_block bb = single_succ (ENTRY_BLOCK_PTR_FOR_FN (cfun));
> > >    gimple_stmt_iterator gsi = gsi_start_nondebug_after_labels_bb (bb);
> > > --
> > > 2.43.0
> > >

Reply via email to