Re: [Patch, PR 54128] ira.c change to fix mips bootstrap

[Richard Sandiford <rdsandiford@googlemail.com>]

Don't have time to look at this in detail, but FWIW:

Steve Ellcey <sellcey@mips.com> writes:
> On Wed, 2012-09-05 at 08:15 +0200, Jakub Jelinek wrote:
>> On Fri, Aug 31, 2012 at 10:58:51AM -0700, Steve Ellcey  wrote:
>> > Here is my patch to fix the bootstrap comparision failure (PR 54128) on
>> > MIPS.  The reason for the comparision failure was a difference in
>> > register usage and I tracked it down to build_insn_chain which checked
>> > all instructions for register usage in order to set the dead_or_set
>> > and live_relevant_regs bitmaps instead of checking only non-debug
>> > instructions.  Changing INSN_P to NONDEBUG_INSN_P in build_insn_chain
>> > allowed me to bootstrap and caused no regressions.
>> 
>> The debug insns generally shouldn't extend the lifetime of pseudos (see the
>> valtrack.c stuff), so if you hit this, there is probably some earlier bug
>> that didn't reset/adjust the debug insns in question.
>> I'm not saying the ira.c patch is absolutely a bad idea, but it would be
>> good if you could investigate where those debug insns started extending
>> lifetime of pseudos.
>> 
>> > 2012-08-31  Steve Ellcey  <sellcey@mips.com>
>> > 
>> > 	PR bootstrap/54128
>> > 	* ira.c (build_insn_chain): Check only NONDEBUG instructions for
>> > 	register usage.
>> 
>> 	Jakub
>
> I think I know where this may be going wrong, though I am having trouble
> actually creating a patch.  I think MIPS should define
> TARGET_DELAY_VARTRACK and call variable_tracking_main from mips_reorg.
>
> The systems that define TARGET_DELAY_VARTRACK all have this comment:
>
> /* Variable tracking should be run after all optimizations which
>    change order of insns.  It also needs a valid CFG.  */

That isn't possible on delayed-branch targets.  dbr_schedule changes
the order of insns and (necessarily, until someone rewrites it)
runs with the CFG pulled down.

> And I think mips_reorg could change the order of insns.  I have tried
> putting a call to variable_tracking_main in mips_df_reorg (and changed
> mips_cfg_in_reorg to return true if flag_var_tracking is true but that
> didn't fix the problem.  I thought this might be because some of the
> mips_reorg code that comes after mips_df_reorg is still changing
> insn ordering.

Right.  mips_reorg calls dbr_schedule as a subroutine after mips_df_reorg.
dbr_schedule ought to be the last point at which we change the order of the
instructions though (rather than splitting them, inserting hazard nops, etc.).
So if the comment above really is what vartrack expects, it sounds like
this is still a generic dbr_schedule vs. vartrack thing rather than
a mips_reorg vs. vartrack thing.

Richard