changeset 22e739752f47 in /z/repo/gem5
details: http://repo.gem5.org/gem5?cmd=changeset;node=22e739752f47
description:
mem: Store snoop filter lookup result to avoid second lookup
This patch introduces a private member storing the iterator from the
lookupRequest call, such that it can be re-used when the request
eventually finishes. The method previously called updateRequest is
renamed finishRequest to make it more clear that the two functions
must be called together.
diffstat:
src/mem/coherent_xbar.cc | 4 ++--
src/mem/snoop_filter.cc | 46 ++++++++++++++++++++--------------------------
src/mem/snoop_filter.hh | 32 ++++++++++++++++++++------------
3 files changed, 42 insertions(+), 40 deletions(-)
diffs (162 lines):
diff -r 45a23e44e93d -r 22e739752f47 src/mem/coherent_xbar.cc
--- a/src/mem/coherent_xbar.cc Fri Sep 25 07:26:57 2015 -0400
+++ b/src/mem/coherent_xbar.cc Fri Sep 25 07:26:57 2015 -0400
@@ -235,7 +235,7 @@
if (snoopFilter && !system->bypassCaches()) {
// Let the snoop filter know about the success of the send operation
- snoopFilter->updateRequest(pkt, *src_port, !success);
+ snoopFilter->finishRequest(!success, pkt);
}
// check if we were successful in sending the packet onwards
@@ -610,7 +610,7 @@
// operation, and do it even before sending it onwards to
// avoid situations where atomic upward snoops sneak in
// between and change the filter state
- snoopFilter->updateRequest(pkt, *slavePorts[slave_port_id], false);
+ snoopFilter->finishRequest(false, pkt);
snoop_result = forwardAtomic(pkt, slave_port_id, InvalidPortID,
sf_res.first);
diff -r 45a23e44e93d -r 22e739752f47 src/mem/snoop_filter.cc
--- a/src/mem/snoop_filter.cc Fri Sep 25 07:26:57 2015 -0400
+++ b/src/mem/snoop_filter.cc Fri Sep 25 07:26:57 2015 -0400
@@ -70,8 +70,8 @@
bool allocate = !cpkt->req->isUncacheable() && slave_port.isSnooping();
Addr line_addr = cpkt->getBlockAddr(linesize);
SnoopMask req_port = portToMask(slave_port);
- auto sf_it = cachedLocations.find(line_addr);
- bool is_hit = (sf_it != cachedLocations.end());
+ reqLookupResult = cachedLocations.find(line_addr);
+ bool is_hit = (reqLookupResult != 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
@@ -79,8 +79,10 @@
if (!is_hit && !allocate)
return snoopDown(lookupLatency);
- // Create a new element through operator[] and modify in-place
- SnoopItem& sf_item = is_hit ? sf_it->second : cachedLocations[line_addr];
+ // 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;
SnoopMask interested = sf_item.holder | sf_item.requested;
// Store unmodified value of snoop filter item in temp storage in
@@ -144,32 +146,24 @@
}
void
-SnoopFilter::updateRequest(const Packet* cpkt, const SlavePort& slave_port,
- bool will_retry)
+SnoopFilter::finishRequest(bool will_retry, const Packet* cpkt)
{
- DPRINTF(SnoopFilter, "%s: packet src %s addr 0x%x cmd %s\n",
- __func__, slave_port.name(), cpkt->getAddr(), cpkt->cmdString());
+ if (reqLookupResult != cachedLocations.end()) {
+ // since we rely on the caller, do a basic check to ensure
+ // that finishRequest is being called following lookupRequest
+ assert(reqLookupResult->first == cpkt->getBlockAddr(linesize));
+ if (will_retry) {
+ // 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;
- // Ultimately we should check if the packet came from an
- // allocating source, not just if the port is snooping
- bool allocate = !cpkt->req->isUncacheable() && slave_port.isSnooping();
- if (!allocate)
- return;
+ DPRINTF(SnoopFilter, "%s: restored SF value %x.%x\n",
+ __func__, retryItem.requested, retryItem.holder);
+ }
- Addr line_addr = cpkt->getBlockAddr(linesize);
- auto sf_it = cachedLocations.find(line_addr);
- assert(sf_it != cachedLocations.end());
- if (will_retry) {
- // 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.
- sf_it->second = retryItem;
-
- DPRINTF(SnoopFilter, "%s: restored SF value %x.%x\n",
- __func__, retryItem.requested, retryItem.holder);
+ eraseIfNullEntry(reqLookupResult);
}
-
- eraseIfNullEntry(sf_it);
}
std::pair<SnoopFilter::SnoopList, Cycles>
diff -r 45a23e44e93d -r 22e739752f47 src/mem/snoop_filter.hh
--- a/src/mem/snoop_filter.hh Fri Sep 25 07:26:57 2015 -0400
+++ b/src/mem/snoop_filter.hh Fri Sep 25 07:26:57 2015 -0400
@@ -88,7 +88,8 @@
public:
typedef std::vector<QueuedSlavePort*> SnoopList;
- SnoopFilter (const SnoopFilterParams *p) : SimObject(p),
+ SnoopFilter (const SnoopFilterParams *p) :
+ SimObject(p), reqLookupResult(cachedLocations.end()), retryItem{0, 0},
linesize(p->system->cacheLineSize()), lookupLatency(p->lookup_latency)
{
}
@@ -104,10 +105,12 @@
}
/**
- * Lookup a request (from a slave port) in the snoop filter and return a
- * list of other slave ports that need forwarding of the resulting snoops.
- * Additionally, update the tracking structures with new request
- * information.
+ * Lookup a request (from a slave port) in the snoop filter and
+ * return a list of other slave ports that need forwarding of the
+ * resulting snoops. Additionally, update the tracking structures
+ * with new request information. Note that the caller must also
+ * call finishRequest once it is known if the request needs to
+ * retry or not.
*
* @param cpkt Pointer to the request packet. Not changed.
* @param slave_port Slave port where the request came from.
@@ -117,15 +120,15 @@
const SlavePort& slave_port);
/**
- * For a successful request, update all data structures in the snoop filter
- * reflecting the changes caused by that request
+ * For an un-successful request, revert the change to the snoop
+ * filter. Also take care of erasing any null entries. This method
+ * relies on the result from lookupRequest being stored in
+ * reqLookupResult.
*
- * @param cpkt Pointer to the request packet. Not changed.
- * @param slave_port Slave port where the request came from.
* @param will_retry This request will retry on this bus / snoop filter
+ * @param cpkt Request packet, merely for sanity checking
*/
- void updateRequest(const Packet* cpkt, const SlavePort& slave_port,
- bool will_retry);
+ void finishRequest(bool will_retry, const Packet* cpkt);
/**
* Handle an incoming snoop from below (the master port). These can
upgrade the
@@ -235,8 +238,13 @@
/** Simple hash set of cached addresses. */
SnoopFilterCache cachedLocations;
/**
+ * Iterator used to store the result from lookupRequest until we
+ * call finishRequest.
+ */
+ SnoopFilterCache::iterator reqLookupResult;
+ /**
* Variable to temporarily store value of snoopfilter entry
- * incase updateRequest needs to undo changes made in lookupRequest
+ * incase finishRequest needs to undo changes made in lookupRequest
* (because of crossbar retry)
*/
SnoopItem retryItem;
_______________________________________________
gem5-dev mailing list
[email protected]
http://m5sim.org/mailman/listinfo/gem5-dev
