[2/4] SMS: Use ids to represent ps_insns

Richard Sandiford richard.sandiford@linaro.org
Tue Sep 13 11:38:00 GMT 2011


Ayal Zaks <ayal.zaks@gmail.com> writes:
> So instead of navigating directly from
> ps_insn->ddg_node->node_sched_params, we now use indices and lookup
> pointees in ddg_node and node_sched_params arrays. A bit of a
> nuisance, but it's ok with me.

Well, IMO, ps_insn->ddg_node->node_sched_params is the same amount
of indirection as node_sched_params[ps_insn->id].

> o One could still achieve the goal of having ps_insn's with
> node_sched_params but free of ddg_node's, w/o penalizing the existing
> direct access from ddg_node->node_sched_params, and w/o replicating
> the interface. If you add an (insn field and an) aux union to ps_insn,
> whose info points to your node_sched_params, you could reuse the same
> set of macros. Admittedly, it's a space-time tradeoff, where the space
> is probably not a major concern, and there are other places to look
> for first in sms to save time.

I'm not sure what you mean by "replicating the interface".  There's
still only one interface for getting schedule params: everything still
goes through the SCHED_* macros.

If anything, I think having a way of going directly from the ps_insn to
the sched params _would_ replicate the interface, because some parts of
SMS want the scheduling parameters for a ddg node rather than a ps_insn.
So some parts would (presumably) still use SCHED_* macros for a node,
while the rest would use some other interface for ps_insns.

But I'll do that if you think it's better.  If we do, I think we
should remove the current SCHED_* macros and just have one that
goes from a ddg node to the parameters structure itself.
So SCHED_TIME (node) becomes SCHED_PARAMS (node)->time, etc.
Code that operates on ps_insns could just use pi->sched_params->time.

But if we want to keep SCHED_TIME, etc., macros that can be used for
everything, then I don't see how changes to ps_insn would help.

> o first_reg_move and nreg_moves will be redundant for regmoves, right?
> No refactoring is perfect...

After the patches, they're only there for debugging.  I did wonder
whether I should just remove them.

> o it could be useful to have a debug version which checks array
> bounds, in case one feeds a bad index.

FWIW, VEC does this for us.

> o and a minor comment below:
>
>> /* The scheduling parameters held for each node. */
>> typedef struct node_sched_params
>> {
>> - int asap; /* A lower-bound on the absolute scheduling cycle. */
>> int time; /* The absolute scheduling cycle (time >= asap). */
>
> Please fix/remove the "(time >= asap)" comment, as there's no asap
> there any more.

OK.

Richard



More information about the Gcc-patches mailing list