On Tue, Apr 19, 2022 at 02:58:26PM +0200, Richard Biener wrote: > On Tue, 19 Apr 2022, Segher Boessenkool wrote: > The assert was for any landing pad which obviously failed - the > testsuite fails were all for MUST_NOT_THROW (negative) regions > which do not end basic-blocks.
I see, thanks. > > > We are also considering all REG_EH_REGION equal, including > > > must-not-throw and nothrow kinds but when those are not from i3 > > > we have no good idea what should happen to them, so the following > > > simply drops them. > > > > 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? > 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. > 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? :-) > > > + /* A REG_EH_REGION note transfering control can only ever come > > > + from i3. */ > > > + gcc_assert (lp_nr <= 0 || from_insn == i3); > > > > if (lp_nr > 0) > > gcc_assert (from_insn == i3); > > is less obfuscated ;-) > > I find that less obvious, but you can have it your way if you like. It corresponds more directly to the comment, the narrative guides the reader? But please use whichever you think best. > > > + /* For nothrow (lp_nr == 0 or lp_nr == INT_MIN) and > > > + for insns in a MUST_NOT_THROW region (lp_nr < 0) > > > + it's difficult to decide what to do for notes > > > + coming from an insn that is not i3. Just drop > > > + those. */ > > > > That sounds like we do not know what is correct to do, so just sweep it > > under the carpet and hope it works out. "Just drop those, that is > > always safe"? (Is it always safe?) > > If it is not safe we should have rejected the combination. I fully > expect that we'd need to have a piece during analysis constraining > what cases we feed into here to be really "safe". I'm really not > familiar with combine so I know nothing of the constraints it has > (like is only i3 ever going to be a CALL_INSN_P?) https://gcc.gnu.org/git/?p=gcc.git;a=blob;f=gcc/combine.cc;h=53dcac92abc248a80fc32dd1d3bb641a650d4d9a;hb=HEAD#l1882 https://gcc.gnu.org/git/?p=gcc.git;a=blob;f=gcc/combine.cc;h=53dcac92abc248a80fc32dd1d3bb641a650d4d9a;hb=HEAD#l2644 None of the insns other than i3 are call insns, ever. combine does not consider EH_REGIONs anywhere else. It uses insn_nothrow_p in some places though. Segher
