Re: [PATCH][v2] rtl-optimization/105231 - distribute_notes and REG_EH_REGION

[Segher Boessenkool <segher@kernel.crashing.org>]

Hi!

So the assert last week was for a landing pad <= 0?  < or =?

On Tue, Apr 19, 2022 at 01:02:09PM +0200, Richard Biener wrote:
> The following mitigates a problem in combine distribute_notes which
> places an original REG_EH_REGION based on only may_trap_p which is
> good to test whether a non-call insn can possibly throw but not if
> actually it does or we care.  That's something we decided at RTL
> expansion time where we possibly still know the insn evaluates
> to a constant.
> 
> In fact, the REG_EH_REGION can only come from the original i3 and
> an assert is added to that effect.  That means we only need to
> retain the note on i3 or, if that cannot trap, drop it but we
> should never move it to i2.  If splitting of i3 ever becomes a
> problem here the insn split should be rejected instead.
> 
> 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?

> +	  {
> +	    int lp_nr = INTVAL (XEXP (note, 0));
> +	    /* 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 ;-)

> +	    /* 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?)

Okay for trunk with maybe a word or two more there.  Thanks!


Segher