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
> >

Reply via email to