On Fri, Feb 21, 2025 at 4:55 PM Konstantinos Eleftheriou
<konstantinos.elefther...@vrull.eu> wrote:
>
> 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?
I don't think it makes much sense in trying to handle loads/stores with EH,
you generally can't re-order stuff around EH since that changes what is
observable from the EH handler and the EH might also guard exceptions
on later stmts.
Richard.
>
> 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
> > >
