Re: Fix a case where combine generates noncanonical debug rtl

[Eric Botcazou <ebotcazou at adacore dot com>]

> As a stress test, I tried calling make_compound_operation unconditionally,
> rather than performing it lazily when first needed.  This showed up a
> problem in:
>
> 	    if (reg == i2dest && i2scratch)
> 	      {
> 		/* If we used i2dest as a scratch register with a
> 		   different mode, substitute it for the original
> 		   i2src while its original mode is temporarily
> 		   restored, and then clear i2scratch so that we don't
> 		   do it again later.  */
> 		propagate_for_debug (i2, i3, reg, i2src, false);
> 		i2scratch = false;
> 		/* Put back the new mode.  */
> 		adjust_reg_mode (reg, new_mode);
> 	      }
>
> i2scratch is true if we have combined three instructions into one
> pattern that didn't match, we have determined that i2dest is a safe
> scratch register, and either:
>
>   (1) we managed to use a define_split to generate 2 patterns; or
>   (2) we managed to find a split point using find_split_point
>
> In both cases, i1src has been substituted into i2src and i2src
> has been substituted into the candidate replacement for i3.  So:
>
>   - Despite what the comment says, i2src isn't strictly speaking the
>     "original" i2src.  At best it's i2src after replacing i1dest
>     with i1src.  That in itself shouldn't matter, because if the
>     replacement hadn't been made, the end of the function would
>     do it anyway.
>
>   - i2src is now effectively part of the new i3.  This probably
>     doesn't matter for (1), because although splitters can reuse
>     rtxes in the original insn, they wouldn't normally modify them
>     destructively.  However, it is a problem for (2), because the
>     split point could be inside i2src.  That is, the use of the
>     i2dest scratch register could now be in i2src.
>
> The patch avoids this by creating a copy before performing the split.

The copy is conditionalized on MAY_HAVE_DEBUG_INSNS though and i2src isn't 
dead for !MAY_HAVE_DEBUG_INSNS at that point, which should be avoided.

Can you fix easily that (no, not by doing the copy unconditionally ;-)?  In my 
opinion, a valid fix is to replace the second hunk with a ??? comment before 
the code you quoted, explaining what can happen under extreme conditions, but 
it's up to you.


> +	  /* *SUBST may be part of I2SRC, so make sure we have the
> +	     original expression around for later debug processing.  */
> +	  if (MAY_HAVE_DEBUG_INSNS)
> +	    i2src = copy_rtx (i2src);
> +

*SPLIT?

-- 
Eric Botcazou