Richard Sandiford <richard.sandif...@linaro.org> wrote on 30/08/2011
03:03:59 PM:

> From: Richard Sandiford <richard.sandif...@linaro.org>

> To: gcc-patches@gcc.gnu.org

> Cc: Ayal Zaks/Haifa/IBM@IBMIL

> Date: 30/08/2011 03:05 PM

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

>
> Instructions in a partial schedule are currently represented as a
> ddg node. This patch uses a more abstract id instead. At the moment,
> the ids map directly to ddg nodes, but the next patch will add register
> moves to the end.
>
> One slight advantage of using ids is that we can leave the ASAP value
> on the node; we don't need to copy it across to the scheduling info.
>
> (Later patches use the same scheduling info for moves, for which an ASAP
> value would be wasted space.)
>
> Richard
>

OK.

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. A couple of thoughts:

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.

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

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

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.

Thanks,
Ayal.

Reply via email to