mem: Add predecessor to SenderState base class
Review Request #1708 - Created Feb. 13, 2013 and submitted
| Information | |
|---|---|
| Andreas Hansson | |
| gem5 | |
| default | |
| Reviewers | |
| Default | |
Changeset 9541:c7358871f1e5 --------------------------- mem: Add predecessor to SenderState base class This patch adds a predecessor field to the SenderState base class to make the process of linking them up more uniform, and enable a traversal of the stack without knowing the specific type of the subclasses. There are a number of simplifications done as part of changing the SenderState, particularly in the RubyTest.
All regressions passing (excluding t1000 and rio)
Posted (Feb. 16, 2013, 3:22 a.m.)
Nice!
It may not be worth it, but if I were doing this from scratch, I would consider adding two methods to Packet:
void pushSenderState(SenderState *);
SenderState *popSenderState();
Then I think that may of the places that currently have idioms like:
FooSenderState *senderState = dynamic_cast<FooSenderState *>(pkt->senderState);
pkt->senderState = senderState->predecessor;
could be rewritten as
FooSenderState *senderState = dynamic_cast<FooSenderState *>(pkt->popSenderState());
and places that have
Packet::SenderState* senderState = pkt->senderState;
pkt->senderState = new FooSenderState(senderState, ...);
could be rewritten as
pkt->pushSenderState(new FooSenderState(...));
It's not a huge win, and if you think it's more effort than it's worth to retrofit this then don't bother. Just a suggestion.
Review request changed
Updated (Feb. 17, 2013, 7:59 p.m.)
Description: |
|
||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Diff: |
Revision 2 (+120 -130) |
That was quick! Thanks again!