[PATCH SMS 2/2, RFC] Register pressure estimation for the partial schedule (re-submission)

Richard Sandiford richard.sandiford@linaro.org
Mon Dec 19 14:38:00 GMT 2011


Hi Revital,

Revital Eres <revital.eres@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



More information about the Gcc-patches mailing list