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]. > 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