Hi Richard, thanks for the feedback.
On Tue, Feb 18, 2025 at 9:17 PM Richard Biener
<richard.guent...@gmail.com> wrote:
>
>
>
> > Am 18.02.2025 um 17:04 schrieb Konstantinos Eleftheriou
> > <konstantinos.elefther...@vrull.eu>:
> >
> > From: kelefth <konstantinos.elefther...@vrull.eu>
> >
> > The pass rejects the transformation when there are instructions in the
> > sequence that might throw an exception. This was added due to having
> > cases that the load instruction contains a REG_EH_REGION note and
> > moving it before the store instructions caused an error, as it was
> > no longer the last instruction in the basic block.
> >
> > This patch handles those cases by moving a possible REG_EH_REGION
> > note from the load instruction of the store-load sequence to the
> > last instruction of the basic block.
>
> But that’s not a correct transform and will lead to bogus exception handling?
> You’d need to move the note and split the block, possibly updating the EH
> info on the side.
I had originally thought about splitting the block, but tried to get
away with this solution. In case that we are splitting the block after
the load, we wouldn't need to also move the note, right?
Thanks,
Konstantinos
> Richard
>
> > gcc/ChangeLog:
> >
> > * avoid-store-forwarding.cc (process_store_forwarding):
> > (store_forwarding_analyzer::avoid_store_forwarding):
> > Move a possible REG_EH_REGION note from the load instruction
> > to the last instruction of the basic block.
> > ---
> > gcc/avoid-store-forwarding.cc | 13 ++++++++++++-
> > 1 file changed, 12 insertions(+), 1 deletion(-)
> >
> > diff --git a/gcc/avoid-store-forwarding.cc b/gcc/avoid-store-forwarding.cc
> > index 34a7bba4043..05c91bb1a82 100644
> > --- a/gcc/avoid-store-forwarding.cc
> > +++ b/gcc/avoid-store-forwarding.cc
> > @@ -400,6 +400,17 @@ process_store_forwarding (vec<store_fwd_info> &stores,
> > rtx_insn *load_insn,
> > if (load_elim)
> > delete_insn (load_insn);
> >
> > + /* Find possible REG_EH_REGION note in the load instruction and move it
> > + into the last instruction of the basic block. */
> > + rtx reg_eh_region_note = find_reg_note (load_insn, REG_EH_REGION,
> > NULL_RTX);
> > + if (reg_eh_region_note != NULL_RTX)
> > + {
> > + remove_note (load_insn, reg_eh_region_note);
> > + basic_block load_bb = BLOCK_FOR_INSN (load_insn);
> > + add_reg_note (BB_END (load_bb), REG_EH_REGION,
> > + XEXP (reg_eh_region_note, 0));
> > + }
> > +
> > return true;
> > }
> >
> > @@ -425,7 +436,7 @@ store_forwarding_analyzer::avoid_store_forwarding
> > (basic_block bb)
> >
> > rtx set = single_set (insn);
> >
> > - if (!set || insn_could_throw_p (insn))
> > + if (!set)
> > {
> > store_exprs.truncate (0);
> > continue;
> > --
> > 2.47.0
> >
