On Wed, May 13, 2026 at 12:01 AM Richard Biener <[email protected]> wrote: > > On Wed, May 13, 2026 at 8:51 AM Andrew Pinski > <[email protected]> wrote: > > > > On Tue, May 12, 2026 at 11:38 PM Richard Biener > > <[email protected]> wrote: > > > > > > On Wed, May 13, 2026 at 1:46 AM Andrew Pinski > > > <[email protected]> wrote: > > > > > > > > So the problem here is that group_case_labels_stmt can remove all > > > > cases if they all go directly to a BB just containing > > > > __builtin_unreachable. So we left with a switch which has one label > > > > left; the default case. Code in ifcvt (which is the pass right after > > > > this cleanup happens) is not ready for that case because well it is just > > > > an fallthrough at this point and it should not have to worry about it. > > > > > > > > I have not figured out why this only started to show up recently but > > > > it is definitely a latent bug since group_case_labels_stmt started > > > > removing > > > > case labels that go to a BB that contains __builtin_unreachable; > > > > r8-546-gca4d2851687875. > > > > > > Huh, I didn't expect end_recording_case_labels to affect the CFG. Which > > > is why I'm also worried about this patch ... > > > > > > IMO this belongs to CFG cleanup. I see we're recording case labels > > > only from two places, critical edge splitting, CFG cleanup itself and > > > uninit analysis. Only CFG cleanup should have exposed the need > > > for r8-546, right? So ... can we somehow move this all there? > > > > Yes I agree it belongs in a different place from the current > > placement. In fact I think it should not happen until after vrp1. > > I will look into moving it tomorrow as the first step. Though since > > this is a regression so the big question is what do we do for GCC 16 > > branch? > > What happens if we revert r8-546?
Reverting only the group_case_labels_stmt part seems to fix the testcase. I will also try to update optimize_unreachable now to do this part. Thanks, Drea > > > > > Thanks, > > Andrew > > > > > > > > > What this does is if group_case_labels_stmt returns true and we only > > > > have a default case left; remove the switch and change the flow into > > > > a fallthrough. > > > > > > > > There is some slight cleanups in end_recording_case_labels done > > > > with just using continue instead f indention to make it easier > > > > to read on what is going on. > > > > > > > > Bootstrapped and tested on x86_64-linux-gnu. > > > > > > > > PR tree-optimization/125290 > > > > > > > > gcc/ChangeLog: > > > > > > > > * tree-cfg.cc (end_recording_case_labels): Fixup > > > > case for only switch with default case left. > > > > > > > > gcc/testsuite/ChangeLog: > > > > > > > > * gcc.dg/torture/pr125290-1.c: New test. > > > > > > > > Signed-off-by: Andrew Pinski <[email protected]> > > > > --- > > > > gcc/testsuite/gcc.dg/torture/pr125290-1.c | 40 +++++++++++++++++++++++ > > > > gcc/tree-cfg.cc | 20 +++++++++--- > > > > 2 files changed, 56 insertions(+), 4 deletions(-) > > > > create mode 100644 gcc/testsuite/gcc.dg/torture/pr125290-1.c > > > > > > > > diff --git a/gcc/testsuite/gcc.dg/torture/pr125290-1.c > > > > b/gcc/testsuite/gcc.dg/torture/pr125290-1.c > > > > new file mode 100644 > > > > index 00000000000..6183cbd2681 > > > > --- /dev/null > > > > +++ b/gcc/testsuite/gcc.dg/torture/pr125290-1.c > > > > @@ -0,0 +1,40 @@ > > > > +/* PR tree-optimization/125290 */ > > > > +/* { dg-do compile } */ > > > > + > > > > +int g21; > > > > +int g31, f14f16_c12; > > > > +void f14f16() > > > > +{ > > > > + short v7; > > > > + long v9; > > > > + long v13; > > > > + if (g31) > > > > + { > > > > + g21 = 0; > > > > + switch (v7) > > > > + { > > > > + case 4: > > > > + case 66: break; > > > > + default: __builtin_unreachable(); > > > > + } > > > > + } > > > > + else > > > > + { > > > > + lbl_bf2: > > > > + v9 = g21; > > > > + } > > > > + v13 = v9; > > > > + switch (v13) > > > > + { > > > > + case 55: > > > > + case 2: goto lbl_sw12; > > > > + default: > > > > + if (f14f16_c12) > > > > + goto lbl_bf2; > > > > + else > > > > + return; > > > > + } > > > > +lbl_sw12: > > > > + __builtin_unreachable(); > > > > +} > > > > + > > > > diff --git a/gcc/tree-cfg.cc b/gcc/tree-cfg.cc > > > > index b2968c412ed..da6d6b34da8 100644 > > > > --- a/gcc/tree-cfg.cc > > > > +++ b/gcc/tree-cfg.cc > > > > @@ -1271,10 +1271,22 @@ end_recording_case_labels (void) > > > > EXECUTE_IF_SET_IN_BITMAP (touched_switch_bbs, 0, i, bi) > > > > { > > > > basic_block bb = BASIC_BLOCK_FOR_FN (cfun, i); > > > > - if (bb) > > > > - { > > > > - if (gswitch *stmt = safe_dyn_cast <gswitch *> (*gsi_last_bb > > > > (bb))) > > > > - group_case_labels_stmt (stmt); > > > > + if (!bb) > > > > + continue; > > > > + gswitch *stmt = safe_dyn_cast <gswitch *> (*gsi_last_bb (bb)); > > > > + if (!stmt) > > > > + continue; > > > > + if (group_case_labels_stmt (stmt) > > > > + && gimple_switch_num_labels (stmt) == 1) > > > > + { > > > > + /* group_case_labels_stmt can remove all > > > > + cases except for the default (via __builtin_unreachable()) > > > > + so fixup that case. */ > > > > + edge taken_edge = single_succ_edge (bb); > > > > + auto gsi = gsi_last_bb (bb); > > > > + gsi_remove (&gsi, true); > > > > + taken_edge->flags &= ~(EDGE_TRUE_VALUE|EDGE_FALSE_VALUE); > > > > + taken_edge->flags |= EDGE_FALLTHRU; > > > > } > > > > } > > > > BITMAP_FREE (touched_switch_bbs); > > > > -- > > > > 2.43.0 > > > >
