Re: [2/4] SMS: Use ids to represent ps_insns
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
Re: [2/4] SMS: Use ids to represent ps_insns
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
Re: [2/4] SMS: Use ids to represent ps_insns
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.
