[PATCH][MIPS] P5600 scheduling

Richard Sandiford rdsandiford@googlemail.com
Sun May 25 10:48:00 GMT 2014


Jaydeep Patil <Jaydeep.Patil@imgtec.com> writes:
> Hi Richard,
>
> Please refer to the attached patch files.
>
> gcc-p5600-noMSA.patch
>   The patch implements P5600 pipeline and scheduling for GP and FPU instructions.

This patch is OK, thanks.  Generally (i.e. when I remember to check)
we don't define TARGET_* options until they're needed, so please leave
out the TARGET_P5600 definition.

> gcc-p5600-noMSA-msched-weight.patch
>   The patch implements -msched-weight option.

> +/* Return 1 if size of REG_MODE matches size of REG_TYPE.  */
> +#define GET_REGTYPE_SIZE(REG_MODE, REG_TYPE)			\
> +  (((REG_TYPE) == GPREG						\
> +    && GET_MODE_SIZE ((REG_MODE)) <= GET_MODE_SIZE (DImode))	\
> +   || ((REG_TYPE) == VECREG					\
> +       && GET_MODE_SIZE ((REG_MODE)) > GET_MODE_SIZE (DImode)))

Is size the best way of distinguishing between these?  I would have
thought that SFmode values would be counted against the VEC/FP
register file instead.  Also, in 32-bit mode a DImode would take
up 2 registers rather than 1.  Maybe it would be better to count
SCALAR_INT_MODE_P modes against GPREG and others against VECREG.

That would still mishandle things like float<->integer conversions.
We could avoid that by checking the register classes in the instruction
constraints, but since this is a heuristic, in practice it might be OK
to ignore corner cases like that.

Rather than:

> +	  INSN_REGTYPE_WEIGHT (insn, GPREG)
> +	    = find_insn_regtype_weight (insn, GPREG);
> +	  INSN_REGTYPE_WEIGHT (insn, VECREG)
> +	    = find_insn_regtype_weight (insn, VECREG);

and:

> +	  if (regtype_weight[GPREG] != NULL)
> +	    CURR_REGTYPE_PRESSURE (GPREG)
> +	      += INSN_REGTYPE_WEIGHT (insn, GPREG);
> +	  if (regtype_weight[VECREG] != NULL)
> +	    CURR_REGTYPE_PRESSURE (VECREG)
> +	      += INSN_REGTYPE_WEIGHT (insn, VECREG);

I think it would be more efficient to have a per-insn structure that
contains both the GP and VEC pressures and only process each instruction
once.

> +  regtype_weight[GPREG] = (short *) xcalloc (old_max_uid, sizeof (short));
> +  regtype_weight[VECREG] = (short *) xcalloc (old_max_uid, sizeof (short));

... = XCNEWVEC (short, old_max_uid);

> +  FOR_EACH_BB_REVERSE_FN (b, cfun)
> +    {
> +      rtx insn, next_tail, head, tail;
> +      get_ebb_head_tail (b, b, &head, &tail);
> +      next_tail = NEXT_INSN (tail);
> +      for (insn = head; insn != next_tail; insn = NEXT_INSN (insn))
> +	{
> +	  /* Handle register life information.  */
> +	  if (!INSN_P (insn))
> +	    continue;

AFAICT this loop doesn't carry any state between instructions,
so it should be:

  FOR_EACH_BB_FN (b, cfun)
    FOR_BB_INSNS (b, insn)
      if (NONDEBUG_INSN_P (insn))
        {
          ...
        }

Thanks,
Richard



More information about the Gcc-patches mailing list