diff options
| -rw-r--r-- | src/mem/coherent_xbar.cc | 4 | ||||
| -rwxr-xr-x | src/mem/snoop_filter.cc | 48 | ||||
| -rwxr-xr-x | src/mem/snoop_filter.hh | 32 |
3 files changed, 43 insertions, 41 deletions
diff --git a/src/mem/coherent_xbar.cc b/src/mem/coherent_xbar.cc index 7181cb965..206f94406 100644 --- a/src/mem/coherent_xbar.cc +++ b/src/mem/coherent_xbar.cc @@ -235,7 +235,7 @@ CoherentXBar::recvTimingReq(PacketPtr pkt, PortID slave_port_id) 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 @@ CoherentXBar::recvAtomic(PacketPtr pkt, PortID slave_port_id) // 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 --git a/src/mem/snoop_filter.cc b/src/mem/snoop_filter.cc index a52d86380..f6e6ef1b4 100755 --- a/src/mem/snoop_filter.cc +++ b/src/mem/snoop_filter.cc @@ -70,8 +70,8 @@ SnoopFilter::lookupRequest(const Packet* cpkt, const SlavePort& slave_port) 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 @@ SnoopFilter::lookupRequest(const Packet* cpkt, const SlavePort& slave_port) 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 @@ SnoopFilter::lookupRequest(const Packet* cpkt, const SlavePort& slave_port) } 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()); - - // 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; + 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; + + 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 --git a/src/mem/snoop_filter.hh b/src/mem/snoop_filter.hh index ee2c82b6e..b1e33dc8f 100755 --- a/src/mem/snoop_filter.hh +++ b/src/mem/snoop_filter.hh @@ -88,7 +88,8 @@ class SnoopFilter : public SimObject { 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 @@ class SnoopFilter : public SimObject { } /** - * 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 @@ class SnoopFilter : public SimObject { 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 @@ class SnoopFilter : public SimObject { /** 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; |