Daniel Carvalho has uploaded this change for review. (
https://gem5-review.googlesource.com/c/public/gem5/+/19749 )
Change subject: mem: Encapsulate retry variables of SnoopFilter
......................................................................
mem: Encapsulate retry variables of SnoopFilter
Group all variables related to the restoration of a snoop filter
entry due to a crossbar retry.
Change-Id: I4e03edb3afd06563b7a5812959739876709eceeb
Signed-off-by: Daniel R. Carvalho <oda...@yahoo.com.br>
---
M src/mem/snoop_filter.cc
M src/mem/snoop_filter.hh
2 files changed, 44 insertions(+), 21 deletions(-)
diff --git a/src/mem/snoop_filter.cc b/src/mem/snoop_filter.cc
index 8168c3d..d368dd8 100644
--- a/src/mem/snoop_filter.cc
+++ b/src/mem/snoop_filter.cc
@@ -76,8 +76,8 @@
line_addr |= LineSecure;
}
SnoopMask req_port = portToMask(slave_port);
- reqLookupResult = cachedLocations.find(line_addr);
- bool is_hit = (reqLookupResult != cachedLocations.end());
+ reqLookupResult.it = cachedLocations.find(line_addr);
+ bool is_hit = (reqLookupResult.it != cachedLocations.end());
// If the snoop filter has no entry, and we should not allocate,
// do not create a new snoop filter entry, simply return a NULL
@@ -86,15 +86,17 @@
return snoopDown(lookupLatency);
// If no hit in snoop filter create a new element and update iterator
- if (!is_hit)
- reqLookupResult = cachedLocations.emplace(line_addr,
SnoopItem()).first;
- SnoopItem& sf_item = reqLookupResult->second;
+ if (!is_hit) {
+ reqLookupResult.it =
+ cachedLocations.emplace(line_addr, SnoopItem()).first;
+ }
+ SnoopItem& sf_item = reqLookupResult.it->second;
SnoopMask interested = sf_item.holder | sf_item.requested;
// Store unmodified value of snoop filter item in temp storage in
// case we need to revert because of a send retry in
// updateRequest.
- retryItem = sf_item;
+ reqLookupResult.retryItem = sf_item;
totRequests++;
if (is_hit) {
@@ -155,25 +157,26 @@
void
SnoopFilter::finishRequest(bool will_retry, Addr addr, bool is_secure)
{
- if (reqLookupResult != cachedLocations.end()) {
+ if (reqLookupResult.it != cachedLocations.end()) {
// since we rely on the caller, do a basic check to ensure
// that finishRequest is being called following lookupRequest
Addr line_addr = (addr & ~(Addr(linesize - 1)));
if (is_secure) {
line_addr |= LineSecure;
}
- assert(reqLookupResult->first == line_addr);
+ assert(reqLookupResult.it->first == line_addr);
if (will_retry) {
+ SnoopItem retry_item = reqLookupResult.retryItem;
// Undo any changes made in lookupRequest to the snoop filter
// entry if the request will come again. retryItem holds
// the previous value of the snoopfilter entry.
- reqLookupResult->second = retryItem;
+ reqLookupResult.it->second = retry_item;
DPRINTF(SnoopFilter, "%s: restored SF value %x.%x\n",
- __func__, retryItem.requested, retryItem.holder);
+ __func__, retry_item.requested, retry_item.holder);
}
- eraseIfNullEntry(reqLookupResult);
+ eraseIfNullEntry(reqLookupResult.it);
}
}
diff --git a/src/mem/snoop_filter.hh b/src/mem/snoop_filter.hh
index 9c06615..4d1eea9 100644
--- a/src/mem/snoop_filter.hh
+++ b/src/mem/snoop_filter.hh
@@ -94,7 +94,7 @@
typedef std::vector<QueuedSlavePort*> SnoopList;
SnoopFilter (const SnoopFilterParams *p) :
- SimObject(p), reqLookupResult(cachedLocations.end()), retryItem{0,
0},
+ SimObject(p), reqLookupResult(cachedLocations.end()),
linesize(p->system->cacheLineSize()),
lookupLatency(p->lookup_latency),
maxEntryCount(p->max_capacity / p->system->cacheLineSize())
{
@@ -261,17 +261,37 @@
/** Simple hash set of cached addresses. */
SnoopFilterCache cachedLocations;
+
/**
- * Iterator used to store the result from lookupRequest until we
- * call finishRequest.
+ * A request lookup must be followed by a call to finishRequest to
inform
+ * the operation's success. If a retry is needed, however, all changes
+ * made to the snoop filter while performing the lookup must be undone.
+ * This structure keeps track of the state previous to such changes.
*/
- SnoopFilterCache::iterator reqLookupResult;
- /**
- * Variable to temporarily store value of snoopfilter entry
- * incase finishRequest needs to undo changes made in lookupRequest
- * (because of crossbar retry)
- */
- SnoopItem retryItem;
+ struct ReqLookupResult {
+ /** Iterator used to store the result from lookupRequest. */
+ SnoopFilterCache::iterator it;
+
+ /**
+ * Variable to temporarily store value of snoopfilter entry
+ * in case finishRequest needs to undo changes made in
lookupRequest
+ * (because of crossbar retry)
+ */
+ SnoopItem retryItem;
+
+ /**
+ * The constructor must be informed of the internal cache's end
+ * iterator, so do not allow the compiler to implictly define it.
+ *
+ * @param end_it Iterator to the end of the internal cache.
+ */
+ ReqLookupResult(SnoopFilterCache::iterator end_it)
+ : it(end_it), retryItem{0, 0}
+ {
+ }
+ ReqLookupResult() = delete;
+ } reqLookupResult;
+
/** List of all attached snooping slave ports. */
SnoopList slavePorts;
/** Track the mapping from port ids to the local mask ids. */
--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/19749
To unsubscribe, or for help writing mail filters, visit
https://gem5-review.googlesource.com/settings
Gerrit-Project: public/gem5
Gerrit-Branch: master
Gerrit-Change-Id: I4e03edb3afd06563b7a5812959739876709eceeb
Gerrit-Change-Number: 19749
Gerrit-PatchSet: 1
Gerrit-Owner: Daniel Carvalho <oda...@yahoo.com.br>
Gerrit-MessageType: newchange
_______________________________________________
gem5-dev mailing list
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev