On Tue, Apr 19, 2022 at 05:00:12PM +0200, Richard Biener wrote:
> On Tue, 19 Apr 2022, Segher Boessenkool wrote:
> > > > And that always is safe? Why do we have REG_EH_REGION for those cases
> > > > at all, then?
> > >
> > > It's the only "safe" thing to do at distribute_notes time I think. If
> > > it is not "safe" (it might lose must-not-throw EH events, or lose
> > > optimization when dropping nothrow) we probably have to reject the
> > > combination earlier.
> >
> > So assert this does not happen please?
>
> I'm not sure how I can.
Losing optimisation is always safe. If removing a must-not-throw is not
safe we should assert this does not happen (or if we know it does
happen, actually fix that).
> AFAICS I cannot rely on all other REG_EH_REGION
> notes be already present on 'i3' when processing notes from the
> original i1 or i2. I can only assert we never have any REG_EH_REGION
> notes from i1 or i2 but I already know we do from the last round
> of testsuite failures ;)
The notes originally from i3 are distributed before those from i2, which
are before those from i1, and those are distributed before those from
i0.
> If you can point me to a (hopefully single) place in combine.cc
> where the set of original source insns is fixed and the original
> notes still present I can come up with a test for what should be
> safe input.
Up until
/* Get the old REG_NOTES and LOG_LINKS from all our insns and
clear them. */
all old notes are still intact.
The insns are i3, i2, etc.; their patterns can change during combine,
but the insns themselves don't.
> > > As I understand combining to multiple insns always happens via
> > > a split (so we combine up to three insns into one and then might
> > > end up splitting that into at most two insns).
> >
> > Yes, but not necessarily a define_split or similar: combine itself knows
> > how to split things, too. The obvious one is a parallel of multiple
> > sets, but it can also break the rhs of a set apart, for example.
>
> I see. So I was aiming at "distributing" the notes to the single
> combined insn _before_ splitting it and then making the split process
> DTRT - that's much easier to get right than the current setup where
> we receive notes from random insns a time to be distributed to another
> random insn.
That is impossible to get right, in general. That is why there is a
from_insn argument to distribute_notes. If you first move everything
to one combined insn you would have to pry things apart again :-(
> > > The only case we
> > > could in theory special-case is when _all_ original insns combined
> > > have the exact same REG_EH_REGION (all have it with the same
> > > landing pad number) or some have none but i3 at least has one.
> > > Then we should be able to distribute the note to all possibly
> > > two result insns. But I can't see that distribute_note has
> > > this info readily available (that there not exist conflicting
> > > REG_EH_REGIONs, like MNT on the original i2 and a > 0 one on i3).
> >
> > Not sure this would be worth the complexity. Do we see this happen
> > ever, can we even test it? :-)
>
> We cannot test this in distribute_notes, we could test for this when
> we have all source insns and reject the cases we cannot possibly
> recover from in the current distribute_notes implementation.
Yes.
> > None of the insns other than i3 are call insns, ever.
>
> Good.
An understatement :-)
Segher
