On Thu, May 14, 2026 at 10:16 AM Andrew Pinski <[email protected]> wrote: > > This reverts the group_case_labels_stmt part of r8-546-gca4d2851687875. > This is placed in the wrong location to remove the case statements that go > directly to __builtin_unreachable. In fact the removal of the case statements > make us lose optimizations in some cases (Wuninitialized-pr107919-1.C for > one). > > Also this fixes PR 125290 by no longer leaving around a switch which just > has a default case. > > Bootstrapped and tested on x86_64-linux-gnu.
OK for trunk. Thanks, Richard. > PR tree-optimization/125290 > > gcc/ChangeLog: > > * tree-cfg.cc (group_case_labels_stmt): Remove code that was > added to remove `cases` that goto blocks of unreachable. > * tree-ssa-forwprop.cc (optimize_unreachable): Remove the > comment about switch cases being handled. > > gcc/testsuite/ChangeLog: > > * g++.dg/warn/Wuninitialized-pr107919-1.C: Remove xfail. > * gcc.dg/analyzer/taint-assert.c: Update for the non-removal > of block containing unreachable. > * gcc.dg/torture/pr125290-1.c: New test. > > Signed-off-by: Andrew Pinski <[email protected]> > --- > .../g++.dg/warn/Wuninitialized-pr107919-1.C | 2 +- > gcc/testsuite/gcc.dg/analyzer/taint-assert.c | 4 +- > gcc/testsuite/gcc.dg/torture/pr125290-1.c | 40 +++++++++++++++ > gcc/tree-cfg.cc | 51 +------------------ > gcc/tree-ssa-forwprop.cc | 3 +- > 5 files changed, 46 insertions(+), 54 deletions(-) > create mode 100644 gcc/testsuite/gcc.dg/torture/pr125290-1.c > > diff --git a/gcc/testsuite/g++.dg/warn/Wuninitialized-pr107919-1.C > b/gcc/testsuite/g++.dg/warn/Wuninitialized-pr107919-1.C > index 049fa4d307a..b3ed4628bdd 100644 > --- a/gcc/testsuite/g++.dg/warn/Wuninitialized-pr107919-1.C > +++ b/gcc/testsuite/g++.dg/warn/Wuninitialized-pr107919-1.C > @@ -14,4 +14,4 @@ void do_something(void* storage) > std::swap(event, swappedValue); > } > > -// { dg-bogus "may be used uninitialized" "" { xfail *-*-* } 0 } > +// { dg-bogus "may be used uninitialized" "" 0 } > diff --git a/gcc/testsuite/gcc.dg/analyzer/taint-assert.c > b/gcc/testsuite/gcc.dg/analyzer/taint-assert.c > index a858e996a80..e46cbe95801 100644 > --- a/gcc/testsuite/gcc.dg/analyzer/taint-assert.c > +++ b/gcc/testsuite/gcc.dg/analyzer/taint-assert.c > @@ -322,8 +322,8 @@ test_switch_bogus_case_unreachable (int n) > case 2: > return -1; > case 42: > - /* This case gets optimized away before we see it. */ > - __builtin_unreachable (); > + /* The wording is rather inaccurate here. */ > + __builtin_unreachable (); /* { dg-warning "use of attacked-controlled > value in condition for assertion" } */ > } > } > > 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..5f751e15d9d 100644 > --- a/gcc/tree-cfg.cc > +++ b/gcc/tree-cfg.cc > @@ -1712,7 +1712,6 @@ group_case_labels_stmt (gswitch *stmt) > int old_size = gimple_switch_num_labels (stmt); > int i, next_index, new_size; > basic_block default_bb = NULL; > - hash_set<tree> *removed_labels = NULL; > > default_bb = gimple_switch_default_bb (cfun, stmt); > > @@ -1728,12 +1727,9 @@ group_case_labels_stmt (gswitch *stmt) > gcc_assert (base_case); > base_bb = label_to_block (cfun, CASE_LABEL (base_case)); > > - /* Discard cases that have the same destination as the default case or > - whose destination blocks have already been removed as unreachable. > */ > + /* Discard cases that have the same destination as the default case. > */ > if (base_bb == NULL > - || base_bb == default_bb > - || (removed_labels > - && removed_labels->contains (CASE_LABEL (base_case)))) > + || base_bb == default_bb) > { > i++; > continue; > @@ -1756,8 +1752,6 @@ group_case_labels_stmt (gswitch *stmt) > /* Merge the cases if they jump to the same place, > and their ranges are consecutive. */ > if (merge_bb == base_bb > - && (removed_labels == NULL > - || !removed_labels->contains (CASE_LABEL (merge_case))) > && wi::to_wide (CASE_LOW (merge_case)) == bhp1) > { > base_high > @@ -1770,46 +1764,6 @@ group_case_labels_stmt (gswitch *stmt) > break; > } > > - /* Discard cases that have an unreachable destination block. */ > - if (EDGE_COUNT (base_bb->succs) == 0 > - && gimple_seq_unreachable_p (bb_seq (base_bb)) > - /* Don't optimize this if __builtin_unreachable () is the > - implicitly added one by the C++ FE too early, before > - -Wreturn-type can be diagnosed. We'll optimize it later > - during switchconv pass or any other cfg cleanup. */ > - && (gimple_in_ssa_p (cfun) > - || (LOCATION_LOCUS (gimple_location (last_nondebug_stmt > (base_bb))) > - != BUILTINS_LOCATION))) > - { > - edge base_edge = find_edge (gimple_bb (stmt), base_bb); > - if (base_edge != NULL) > - { > - for (gimple_stmt_iterator gsi = gsi_start_bb (base_bb); > - !gsi_end_p (gsi); gsi_next (&gsi)) > - if (glabel *stmt = dyn_cast <glabel *> (gsi_stmt (gsi))) > - { > - if (FORCED_LABEL (gimple_label_label (stmt)) > - || DECL_NONLOCAL (gimple_label_label (stmt))) > - { > - /* Forced/non-local labels aren't going to be removed, > - but they will be moved to some neighbouring basic > - block. If some later case label refers to one of > - those labels, we should throw that case away rather > - than keeping it around and referring to some random > - other basic block without an edge to it. */ > - if (removed_labels == NULL) > - removed_labels = new hash_set<tree>; > - removed_labels->add (gimple_label_label (stmt)); > - } > - } > - else > - break; > - remove_edge_and_dominated_blocks (base_edge); > - } > - i = next_index; > - continue; > - } > - > if (new_size < i) > gimple_switch_set_label (stmt, new_size, > gimple_switch_label (stmt, i)); > @@ -1822,7 +1776,6 @@ group_case_labels_stmt (gswitch *stmt) > if (new_size < old_size) > gimple_switch_set_num_labels (stmt, new_size); > > - delete removed_labels; > return new_size < old_size; > } > > diff --git a/gcc/tree-ssa-forwprop.cc b/gcc/tree-ssa-forwprop.cc > index 0d76f85e2ac..a45ca961ec8 100644 > --- a/gcc/tree-ssa-forwprop.cc > +++ b/gcc/tree-ssa-forwprop.cc > @@ -5262,8 +5262,7 @@ optimize_unreachable (basic_block bb) > } > else > { > - /* Todo: handle other cases. Note that unreachable switch case > - statements have already been removed. */ > + /* Todo: handle other cases. e.g. switch. */ > continue; > } > > -- > 2.43.0 >
