Jaydeep Patil <jaydeep.pa...@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
