Matthew Poremba has submitted this change. (
https://gem5-review.googlesource.com/c/public/gem5/+/56450 )
(
1 is the latest approved patch-set.
No files were changed between the latest approved patch-set and the
submitted one.
)Change subject: cpu: Only acquire needed tokens in PTL tester
......................................................................
cpu: Only acquire needed tokens in PTL tester
The tester currently assumes that one token per lane is needed when
checking if an action is ready to be issued. When actually issuing
requests, it is possible that a memory location is not valid for various
reasons. This was not being considered when checking for tokens causing
the tester to acquire more tokens than requests sent. Since tokens are
returned when requests are coalesced, this was causing some tokens never
to be returned, eventually depleting the token pool and causing a
deadlock.
Add a new method which determines the number of tokens needed for an
action. This is called by both the ready check method and the method to
issue the action to ensure they are aligned.
Change-Id: Ic1af72085c3b077539eb3fd129331e776ebdffbc
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/56450
Reviewed-by: Matt Sinclair <mattdsincl...@gmail.com>
Maintainer: Matt Sinclair <mattdsincl...@gmail.com>
Tested-by: kokoro <noreply+kok...@google.com>
---
M src/cpu/testers/gpu_ruby_test/tester_thread.cc
M src/cpu/testers/gpu_ruby_test/tester_thread.hh
2 files changed, 73 insertions(+), 6 deletions(-)
Approvals:
Matt Sinclair: Looks good to me, approved; Looks good to me, approved
kokoro: Regressions pass
diff --git a/src/cpu/testers/gpu_ruby_test/tester_thread.cc
b/src/cpu/testers/gpu_ruby_test/tester_thread.cc
index 2dfd54a..760f8c2 100644
--- a/src/cpu/testers/gpu_ruby_test/tester_thread.cc
+++ b/src/cpu/testers/gpu_ruby_test/tester_thread.cc
@@ -152,6 +152,37 @@
episodeHistory.push_back(curEpisode);
}
+int
+TesterThread::getTokensNeeded()
+{
+ if (!tokenPort) {
+ return 0;
+ }
+
+ int tokens_needed = 0;
+ curAction = curEpisode->peekCurAction();
+
+ switch(curAction->getType()) {
+ case Episode::Action::Type::ATOMIC:
+ tokens_needed = numLanes;
+ break;
+ case Episode::Action::Type::LOAD:
+ case Episode::Action::Type::STORE:
+ for (int lane = 0; lane < numLanes; ++lane) {
+ Location loc = curAction->getLocation(lane);
+
+ if (loc != AddressManager::INVALID_LOCATION && loc >= 0) {
+ tokens_needed++;
+ }
+ }
+ break;
+ default:
+ tokens_needed = 0;
+ }
+
+ return tokens_needed;
+}
+
bool
TesterThread::isNextActionReady()
{
@@ -162,10 +193,13 @@
// Only GPU wavefront threads have a token port. For all other
types
// of threads evaluate to true.
- bool haveTokens = tokenPort ? tokenPort->haveTokens(numLanes) :
true;
+ bool haveTokens = true;
switch(curAction->getType()) {
case Episode::Action::Type::ATOMIC:
+ haveTokens = tokenPort ?
+ tokenPort->haveTokens(getTokensNeeded()) : true;
+
// an atomic action must wait for all previous requests
// to complete
if (pendingLdStCount == 0 &&
@@ -206,7 +240,7 @@
assert(pendingAtomicCount == 0);
// can't issue if there is a pending fence
- if (pendingFenceCount > 0 || !haveTokens) {
+ if (pendingFenceCount > 0) {
return false;
}
@@ -215,7 +249,7 @@
for (int lane = 0; lane < numLanes; ++lane) {
Location loc = curAction->getLocation(lane);
- if (loc != AddressManager::INVALID_LOCATION) {
+ if (loc != AddressManager::INVALID_LOCATION && loc >=
0) {
Addr addr = addrManager->getAddress(loc);
if (outstandingLoads.find(addr) !=
@@ -237,6 +271,12 @@
}
}
+ haveTokens = tokenPort ?
+ tokenPort->haveTokens(getTokensNeeded()) : true;
+ if (!haveTokens) {
+ return false;
+ }
+
return true;
default:
panic("The tester got an invalid action\n");
@@ -250,7 +290,7 @@
switch(curAction->getType()) {
case Episode::Action::Type::ATOMIC:
if (tokenPort) {
- tokenPort->acquireTokens(numLanes);
+ tokenPort->acquireTokens(getTokensNeeded());
}
issueAtomicOps();
break;
@@ -262,13 +302,13 @@
break;
case Episode::Action::Type::LOAD:
if (tokenPort) {
- tokenPort->acquireTokens(numLanes);
+ tokenPort->acquireTokens(getTokensNeeded());
}
issueLoadOps();
break;
case Episode::Action::Type::STORE:
if (tokenPort) {
- tokenPort->acquireTokens(numLanes);
+ tokenPort->acquireTokens(getTokensNeeded());
}
issueStoreOps();
break;
diff --git a/src/cpu/testers/gpu_ruby_test/tester_thread.hh
b/src/cpu/testers/gpu_ruby_test/tester_thread.hh
index 5612df9..9877d63 100644
--- a/src/cpu/testers/gpu_ruby_test/tester_thread.hh
+++ b/src/cpu/testers/gpu_ruby_test/tester_thread.hh
@@ -178,6 +178,7 @@
// constraints and is ready to issue
bool isNextActionReady();
void issueNextAction();
+ int getTokensNeeded();
// issue Ops to Ruby memory
// must be implemented by a child class
--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/56450
To unsubscribe, or for help writing mail filters, visit
https://gem5-review.googlesource.com/settings
Gerrit-Project: public/gem5
Gerrit-Branch: develop
Gerrit-Change-Id: Ic1af72085c3b077539eb3fd129331e776ebdffbc
Gerrit-Change-Number: 56450
Gerrit-PatchSet: 3
Gerrit-Owner: Matthew Poremba <matthew.pore...@amd.com>
Gerrit-Reviewer: Gabe Black <gabe.bl...@gmail.com>
Gerrit-Reviewer: Jason Lowe-Power <ja...@lowepower.com>
Gerrit-Reviewer: Matt Sinclair <mattdsincl...@gmail.com>
Gerrit-Reviewer: Matthew Poremba <matthew.pore...@amd.com>
Gerrit-Reviewer: kokoro <noreply+kok...@google.com>
Gerrit-MessageType: merged
_______________________________________________
gem5-dev mailing list -- gem5-dev@gem5.org
To unsubscribe send an email to gem5-dev-le...@gem5.org
%(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s
