Hi Richard,

From: Richard Sandiford <richard.sandif...@arm.com>
> > @@ -3719,6 +3722,7 @@ static unsigned int rest_of_handle_sched (void) 
> > { #ifdef INSN_SCHEDULING
> > +  first_moveable_pseudo = last_moveable_pseudo;
> >    if (flag_selective_scheduling
> >        && ! maybe_skip_selective_scheduling ())
> >      run_selective_scheduling ();
> 
> I think instead we should zero out both variables at the end of IRA.
> There are other places besides the scheduler that call into the IRA code, so 
> tackling the problem there seems more general.

If you zero first_moveable_pseudo and last_moveable_pseudo after IRA then
they'll be zero for the second scheduler pass, which uses them.

In fact, I've just realised that the GCSE and move_loop_invariants passes
also use them (they both call ira_set_pseudo_classes which calls
find_costs_and_classes which uses these variables).

They need to be zeroed or set equal to each other before the first pass that
uses them (move_loop_invariants), but kept alive until after the last pass
that uses them (GCSE). So if there's a function that sets things up right
before the RTL passes start then I think that's a good location candidate.

> > +/* We have plenty of spare registers, so check nothing has been 
> > +spilled. */
> > +/* { dg-final { scan-assembler-not "str" } } */
> 
> The testcase looks good, but it's probably better to make that “\tstr\t”.
> The problem with plain “str” is that assembly output can often include 
> pathnames and version strings, and it's possible that one of those could 
> contain “str”.

Good catch, I'll keep that tip in mind for future!

Thanks,
Omar

Reply via email to