Hi Revital,
Revital Eres <revital.e...@linaro.org> writes:
> The attached patch is a resubmission following comments made by Ayal
> and Richard.
>
> Tested and bootstrap with the other patches in the series on
> ppc64-redhat-linux, enabling SMS on loops with SC 1.
Looks really good to me. I'll leave any approval to Ayal though.
Just one suggestion:
> + /* Handle register moves. */
> + if (ps_i->id >= ps->g->num_nodes)
> + {
> + int old_regno = REGNO (ps_reg_move (ps, ps_i->id)->old_reg);
> + int new_regno = REGNO (ps_reg_move (ps, ps_i->id)->new_reg);
> +
> + change_pressure (old_regno, true);
> + change_pressure (new_regno, false);
> + change_pressure (old_regno, true);
> + }
> + else
> + {
> + for (use_rec = DF_INSN_USES (insn); *use_rec; use_rec++)
> + change_pressure (DF_REF_REGNO (*use_rec), true);
> +
> + for (def_rec = DF_INSN_DEFS (insn); *def_rec; def_rec++)
> + change_pressure (DF_REF_REGNO (*def_rec), false);
> +
> + for (use_rec = DF_INSN_USES (insn); *use_rec; use_rec++)
> + change_pressure (DF_REF_REGNO (*use_rec), true);
> + }
It might be worth adding a comment to say why the code is doing it this way.
E.g.:
/* Process all uses, all defs, and then all uses again. The first
two steps give us an estimate of the pressure when dying inputs
cannot be tied to outputs (which is the worst case). The second
two steps update the set of live registers ready for the next
instruction. */
Also, as a general "future direction" comment, I think we should seriously
consider turning both this and -fmodulo-sched-allow-regmoves on by default,
so that -fmodulo-sched uses them unless explicitly told not to. The results
for ARM seemed to suggest that it is now the best SMS mode (although the
results can't be shared, unfortunately). It'd be unfortunate if users
had to write:
-fmodulo-sched -fmodulo-sched-allow-regmoves -fmodulo-sched-reg-pressure
instead of plain:
-fmodulo-sched
It might make sense as a follow-on patch rather than part of the main commit.
There's also the separate debate about whether SMS should now be enabled
by default for ARM at -O3, but that's for another day...
Richard
