ruby: change clock periods for requests associated with ruby port

Information
Submitter:	Brandon Potter
Repository:	gem5
Branch:	default
Bugs:
Depends On:
Reviewers
Groups:	Default
People:

Description

Changeset 11444:4e99eb9997e2
---------------------------
ruby: change clock periods for requests associated with ruby port

This suggestion was brought up here: reviews.gem5.org/r/3442

This change involves using the clock period set by a ruby port (instead of
the clock period used by the ruby system) when issuing requests through
the ruby port object to attached objects.

Testing Done

Issue Summary

Description	From	Last Updated	Status
Doh! Apologies for the incorrect recommendation in the other review request: instead of (curTick() + rp->clockPeriod()), these should be tightened ...	Joel Hestness	April 21, 2016, 12:49 p.m.	Resolved
May I propose we take one step further and do not use a queued port here? This should be as ...	Andreas Hansson	May 12, 2016, 1:35 p.m.	Dropped

Ship It!

src/mem/ruby/system/RubyPort.cc (Diff revision 1)

Doh! Apologies for the incorrect recommendation in the other review request: instead of (curTick() + rp->clockPeriod()), these should be tightened up even farther to just use (rp->nextCycle()).

Show all issues

Description:

~		Changeset 11444:32d1739cbfd7
	~	Changeset 11444:4e99eb9997e2

		ruby: change clock periods for requests associated with ruby port

		This suggestion was brought up here: reviews.gem5.org/r/3442

		This change involves using the clock period set by a ruby port (instead of
		the clock period used by the ruby system) when issuing requests through
		the ruby port object to attached objects.

Diff:

Revision 2 (+4 -6)

Show changes

src/mem/ruby/system/RubyPort.cc

src/mem/ruby/system/RubyPort.cc (Diff revision 2)

May I propose we take one step further and do not use a queued port here?

This should be as easy as just returning rp->pioSlavePort.sendTimingResp(pkt);

The retry should not be difficult to wire up.

I really don't think there is a need for a queued port here.

Show all issues

Joel Hestness April 23, 2016, 11:18 a.m. (April 23, 2016, 11:18 a.m.)

I certainly agree that we should eventually pull out the QueuedPort, but I think that would be beyond the scope of this fix.

Andreas Hansson April 25, 2016, 1:13 a.m. (April 25, 2016, 1:13 a.m.)

I'd suggest to JFDI :-)

It should be a 50 line fix in the end, and avoids shaking up all the stats twice.

Brandon Potter May 12, 2016, 1:57 p.m. (May 12, 2016, 1:57 p.m.)

I'm dropping the issue because it's unrelated to the original patch as Joel mentioned.

I understand that it's important, but I'd have to come back to this later when I have time to spend on making the changes and testing it. We've talked about it here at AMD and it will be solved at some point in the future.

Ship It!

You have a pending review.

Review Board 2.0.15

Screenshots

Files

Issue Summary

Description:

Changeset 11444:32d1739cbfd7

Changeset 11444:4e99eb9997e2

Diff: