2011/9/13 Richard Sandiford <richard.sandif...@linaro.org>
>
> Ayal Zaks <ayal.z...@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].
>
Agreed. Both involve one indirection. The difference is in lookup
versus using each single direct pointer access.
>
> > 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.
If we add to ps_insn the same aux structure as ddg_node has, with info
pointing to node_sched_params, we could reuse the same set of macros.
>
> But I'll do that if you think it's better.
No need, your solution is fine. Just wanted to clarify the options.
>
> 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.
I'd prefer not to keep useless fields around. If left only for
debugging, better document and/or rename.
>
> > 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.
>
ok, right, in patch [3/4], for node_sched_params and ps_reg_move_info.
>
> > 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.
Thanks,
Ayal.
>
> Richard
