Re: [PATCH] [PR rtl-optimization/65618] Fix MIPS ADA bootstrap failure

[Jeff Law <law@redhat.com>]

On 12/19/2016 08:44 AM, James Cowgill wrote:
> Hi,
>
> This patch fixes PR 65618 where ADA cannot be bootstrapped natively on
> mips due to a bootstrap comparison failure. The PR is currently in the
> target component, but should be in the rtl-optimization component.
>
> The underlying bug is in gcc/emit-rtl.c:try_split and is a result of
> the fix for PR rtl-optimization/48826. In that PR, if a call_insn is
> split into two instructions, the following NOTE_INSN_CALL_ARG_LOCATION
> is moved so that it immediately follows the new call_insn. However,
> after doing that the "after" variable was not updated and it could
> still point to the old note instruction (the instruction after the
> instruction to be split). The "after" variable is later used to obtain
> the last instruction in the split and is then passed back to the
> delayed branch scheduler influencing how delay slots are assigned. My
> patch adjusts the code which handles the NOTE_INSN_CALL_ARG_LOCATION
> note so that "after" is updated if necessary.
>
> This bug causes the ADA bootstrap comparison failure in a-except.o
> because the branch delay scheduling operates slightly differently for
> that file if debug information is turned on.
>
> Thanks,
> James
>
> gcc/Changelog:
>
> 2016-12-16  James Cowgill  <James.Cowgill@imgtec.com>
>
> 	PR rtl-optimization/65618
> 	* emit-rtl.c (try_split): Update "after" when moving a
> 	NOTE_INSN_CALL_ARG_LOCATION.
>
> diff --git a/gcc/emit-rtl.c b/gcc/emit-rtl.c
> index 7de17454037..6be124ac038 100644
> --- a/gcc/emit-rtl.c
> +++ b/gcc/emit-rtl.c
> @@ -3742,6 +3742,11 @@ try_split (rtx pat, rtx_insn *trial, int last)
>  		   next = NEXT_INSN (next))
>  		if (NOTE_KIND (next) == NOTE_INSN_CALL_ARG_LOCATION)
>  		  {
> +		    /* Advance after to the next instruction if it is about to
> +		       be removed.  */
> +		    if (after == next)
> +		      after = NEXT_INSN (after);
> +
>  		    remove_insn (next);
>  		    add_insn_after (next, insn, NULL);
>  		    break;
>
So the thing I don't like when looking at this code is we set AFTER 
immediately upon entry to try_split.  But we don't use it until near the 
very end of try_split.  That's just asking for trouble.

Can we reasonably initialize AFTER just before it's used?

Jeff