Hello, On Mon, 24 Feb 2025, Jeff Law wrote:
> >>>> 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. > But if all the old statements and all the new/adjusted statements are in the > same EH region is this still a problem? It depends, but even if that were no problem in some specific cases (perhaps by ensuring that such sequence isn't intermingled with insns that are in different EH regions) that doesn't seem to be what the proposal is doing? From the description it moves a EH note from a load (where it presumably was for a reason) that occurs after a store to something that so happens to be the (new) last instruction of a bb. Because otherwise the load couldn't be moved earlier into the middle of a BB. In essence the load would lose the EH note and a new EH note would be generated for a different insn out of thin air. I don't see how that can ever be a correct transformation in general. What _if_ the very load traps then? What if the new insn cannot trap? (Note that loads and stores may trap differently even on the same address, e.g. write to read-only mem, or an misaligned load but an aligned (or smaller-sized) write; though I'll say that GCC assumes that loads cannot trap if shadowed by an equivalent write, in some other passes) Ciao, Michael.
