On Fri, Mar 13, 2026 at 12:56 AM Richard Biener
<[email protected]> wrote:
>
> On Fri, Mar 13, 2026 at 4:44 AM Andrew Pinski
> <[email protected]> wrote:
> >
> > cprop_hardreg changes:
> > ```
> > (insn 69 62 183 6 (set (reg:SI 1 x1 [orig:107 _12 ] [107])
> > (zero_extend:SI (mem:QI (reg/f:DI 1 x1 [150]) [0 MEM[(char *)_4]+0
> > S1 A64]))) "/app/example.cpp":8:7 discrim 1 149 {*zero_extendqisi2_aarch64}
> > (expr_list:REG_EH_REGION (const_int 2 [0x2])
> > (nil)))
> > ```
> >
> > into:
> > ```
> > (insn 69 62 183 6 (set (reg:SI 1 x1 [orig:107 _12 ] [107])
> > (zero_extend:SI (mem:QI (plus:DI (reg/f:DI 31 sp)
> > (const_int 56 [0x38])) [0 MEM[(char *)_4]+0 S1 A64])))
> > "/app/example.cpp":8:7 discrim 1 149 {*zero_extendqisi2_aarch64}
> > (expr_list:REG_EH_REGION (const_int 2 [0x2])
> > (nil)))
> > ```
> >
> > But that new instruction no longer traps as it is load from the stack. So
> > later on
> > purge_dead_edges removes the REG_EH_REGION note and the edge to the eh
> > landing pad
> > but this is during find_many_sub_basic_blocks in split3 and nothing then
> > removes the
> > unreachable basic blocks.
> >
> > To fix this, instead of depending on a later pass to clean this up,
> > cprop_hardreg
> > should call purge_all_dead_edges if non-call exceptions were enabled and
> > then
> > also call cleanup_cfg (to remove the unreachable blocks).
> >
> > Bootstrapped and tested on x86_64-linux-gnu and lightly tested for
> > aarch64-linux-gnu.
> >
> > PR rtl-optimization/124454
> >
> > gcc/ChangeLog:
> >
> > * regcprop.cc (pass_cprop_hardreg::execute): If something
> > changed and non-call exceptions is on, call purge_all_dead_edges
> > and cleanup_cfg.
> >
> > gcc/testsuite/ChangeLog:
> >
> > * gcc.dg/pr124454-1.c: New test.
> >
> > Signed-off-by: Andrew Pinski <[email protected]>
> > ---
> > gcc/regcprop.cc | 21 +++++++++++++++++++--
> > gcc/testsuite/gcc.dg/pr124454-1.c | 15 +++++++++++++++
> > 2 files changed, 34 insertions(+), 2 deletions(-)
> > create mode 100644 gcc/testsuite/gcc.dg/pr124454-1.c
> >
> > diff --git a/gcc/regcprop.cc b/gcc/regcprop.cc
> > index e884cb5a966..b1f00ca0435 100644
> > --- a/gcc/regcprop.cc
> > +++ b/gcc/regcprop.cc
> > @@ -36,6 +36,7 @@
> > #include "cfgrtl.h"
> > #include "target.h"
> > #include "function-abi.h"
> > +#include "cfgcleanup.h"
> >
> > /* The following code does forward propagation of hard register copies.
> > The object is to eliminate as many dependencies as possible, so that
> > @@ -1443,6 +1444,7 @@ pass_cprop_hardreg::execute (function *fun)
> > auto_vec<int> *curr = &worklist1;
> > auto_vec<int> *next = &worklist2;
> > bool any_debug_changes = false;
> > + bool any_changes = false;
> >
> > /* We need accurate notes. Earlier passes such as if-conversion may
> > leave notes in an inconsistent state. */
> > @@ -1462,7 +1464,10 @@ pass_cprop_hardreg::execute (function *fun)
> > FOR_EACH_BB_FN (bb, fun)
> > {
> > if (cprop_hardreg_bb (bb, all_vd, visited))
> > - curr->safe_push (bb->index);
> > + {
> > + curr->safe_push (bb->index);
> > + any_changes = true;
> > + }
> > if (all_vd[bb->index].n_debug_insn_changes)
> > any_debug_changes = true;
> > }
> > @@ -1489,7 +1494,10 @@ pass_cprop_hardreg::execute (function *fun)
> > {
> > bb = BASIC_BLOCK_FOR_FN (fun, index);
> > if (cprop_hardreg_bb (bb, all_vd, visited))
> > - next->safe_push (bb->index);
> > + {
> > + next->safe_push (bb->index);
> > + any_changes = true;
> > + }
> > if (all_vd[bb->index].n_debug_insn_changes)
> > any_debug_changes = true;
> > }
> > @@ -1501,6 +1509,15 @@ pass_cprop_hardreg::execute (function *fun)
> > }
> >
> > free (all_vd);
> > +
> > + /* Copy propagation of the sp register into an mem's address
> > + can cause what was a trapping instruction into
> > + a non-trapping one so cleanup after that in the case of non-call
> > + exceptions. */
> > + if (any_changes
> > + && cfun->can_throw_non_call_exceptions
> > + && purge_all_dead_edges ())
> > + cleanup_cfg (0);
>
> OK.
>
> Thanks,
> Richard.
>
> Btw, I see dse.cc does
>
> /* DSE can eliminate potentially-trapping MEMs.
> Remove any EH edges associated with them. */
> if ((locally_deleted || globally_deleted)
> && cfun->can_throw_non_call_exceptions
> && purge_all_dead_edges ())
> {
> free_dominance_info (CDI_DOMINATORS);
> cleanup_cfg (0);
>
> and earlier
>
> if ((locally_deleted || globally_deleted)
> && cfun->can_throw_non_call_exceptions
> && purge_all_dead_edges ())
> {
> free_dominance_info (CDI_DOMINATORS);
> delete_unreachable_blocks ();
> }
>
> where the former freeing of dominance info looks overzealous. I also wonder
> about cfg_cleanup vs. delete_unreachable_blocks; the latter might be good
> enough? Or what are we expecting from cfg_cleanup here?
That is a good question.
indirect_jump_optimize in ira, does delete_unreachable_blocks also. (PR 46755)
While postreload_cse in postreload.cc does cleanup_cfg. (PR 55263)
Both of these changes were done by the same person even (Steven B.)
which makes it even funnier.
But I think delete_unreachable_blocks is only needed in all cases
after the call to purge_all_dead_edges.
Looks like the only major difference is compact_blocks.
I will file a bug and try to resolve this inconsistency for GCC 17.
Thanks,
Andrew
>
>
> > return 0;
> > }
> >
> > diff --git a/gcc/testsuite/gcc.dg/pr124454-1.c
> > b/gcc/testsuite/gcc.dg/pr124454-1.c
> > new file mode 100644
> > index 00000000000..11c4d803d1b
> > --- /dev/null
> > +++ b/gcc/testsuite/gcc.dg/pr124454-1.c
> > @@ -0,0 +1,15 @@
> > +/* { dg-do compile } */
> > +/* { dg-additional-options "-O3 -fno-forward-propagate
> > -fnon-call-exceptions -fno-dse -fprofile-generate -fopenmp
> > -finstrument-functions" } */
> > +/* { dg-additional-options "-mno-outline-atomics" { target aarch64*-*-* }
> > } */
> > +
> > +/* PR rtl-optimization/124454 */
> > +
> > +int a, b;
> > +
> > +void
> > +foo()
> > +{
> > + long c;
> > + a *= b;
> > + b = *(char *)__builtin_memset(&c, 1, 4);
> > +}
> > --
> > 2.43.0
> >