Daniel Carvalho has uploaded this change for review. (
https://gem5-review.googlesource.com/c/public/gem5/+/18209
Change subject: mem-cache: Fix lack of atomic writebuffer
......................................................................
mem-cache: Fix lack of atomic writebuffer
Previously all atomic writebacks concerned a single block,
therefore, when a block was evicted, no other block would be
pending eviction. With sector tags (and compression),
however, a single replacement can generate many evictions.
This can cause problems, since a writeback that evicts a block
may evict blocks in the lower cache. If one of these conflict
with one of the blocks pending eviction in the higher level, the
snoop must inform it to the lower level.
Change-Id: I2fc2f9eb0f26248ddf91adbe987d158f5a2e592b
Signed-off-by: Daniel R. Carvalho <[email protected]>
---
M src/mem/cache/base.cc
M src/mem/cache/base.hh
M src/mem/cache/cache.cc
M src/mem/cache/cache.hh
M src/mem/cache/noncoherent_cache.cc
M src/mem/cache/noncoherent_cache.hh
6 files changed, 86 insertions(+), 41 deletions(-)
diff --git a/src/mem/cache/base.cc b/src/mem/cache/base.cc
index 10575b4..4d91d38 100644
--- a/src/mem/cache/base.cc
+++ b/src/mem/cache/base.cc
@@ -1358,6 +1358,22 @@
}
void
+BaseCache::doWritebacksAtomic(PacketList& writebacks)
+{
+ // Move the writebacks to the temp writebuffer, so that they can be
+ // checked on the snoops caused by themselves
+ assert(tempWritebuffer.empty());
+ std::swap(tempWritebuffer, writebacks);
+
+ while (!tempWritebuffer.empty()) {
+ PacketPtr wb_pkt = tempWritebuffer.front();
+ doWritebacksAtomic(wb_pkt);
+ tempWritebuffer.pop_front();
+ delete wb_pkt;
+ }
+}
+
+void
BaseCache::invalidateBlock(CacheBlk *blk)
{
// If handling a block present in the Tags, let it do its invalidation
diff --git a/src/mem/cache/base.hh b/src/mem/cache/base.hh
index 8d5ed11..723cbab 100644
--- a/src/mem/cache/base.hh
+++ b/src/mem/cache/base.hh
@@ -361,6 +361,14 @@
TempCacheBlk *tempBlock;
/**
+ * Temporary list of on-going atomic writebacks. It is only used by
+ * compressed caches to check if a snoop caused by the writeback of a
+ * sub-pkt conflicts with any of the writebacks waiting to be sent.
+ * Behaves like a writebuffer for the atomic mode.
+ */
+ PacketList tempWritebuffer;
+
+ /**
* Upstream caches need this packet until true is returned, so
* hold it for deletion until a subsequent call
*/
@@ -595,9 +603,15 @@
virtual void doWritebacks(PacketList& writebacks, Tick forward_time) =
0;
/**
- * Send writebacks down the memory hierarchy in atomic mode
+ * Send writebacks down the memory hierarchy in atomic mode. The
+ * writeback list is parsed in a per-packet basis, and then the
coherence
+ * specific function does the proper calls to effectuate the
writebacks.
+ *
+ * @param writebacks The list of writeback packets to be sent.
+ * @param pkt The writeback packet.
*/
- virtual void doWritebacksAtomic(PacketList& writebacks) = 0;
+ void doWritebacksAtomic(PacketList& writebacks);
+ virtual void doWritebacksAtomic(PacketPtr pkt) = 0;
/**
* Create an appropriate downstream bus request packet.
diff --git a/src/mem/cache/cache.cc b/src/mem/cache/cache.cc
index b94cab9..3894356 100644
--- a/src/mem/cache/cache.cc
+++ b/src/mem/cache/cache.cc
@@ -232,39 +232,34 @@
}
void
-Cache::doWritebacksAtomic(PacketList& writebacks)
+Cache::doWritebacksAtomic(PacketPtr pkt)
{
- while (!writebacks.empty()) {
- PacketPtr wbPkt = writebacks.front();
- // Call isCachedAbove for both Writebacks and CleanEvicts. If
- // isCachedAbove returns true we set BLOCK_CACHED flag in
Writebacks
- // and discard CleanEvicts.
- if (isCachedAbove(wbPkt, false)) {
- if (wbPkt->cmd == MemCmd::WritebackDirty ||
- wbPkt->cmd == MemCmd::WriteClean) {
- // Set BLOCK_CACHED flag in Writeback and send below,
- // so that the Writeback does not reset the bit
- // corresponding to this address in the snoop filter
- // below. We can discard CleanEvicts because cached
- // copies exist above. Atomic mode isCachedAbove
- // modifies packet to set BLOCK_CACHED flag
- memSidePort.sendAtomic(wbPkt);
- }
- } else {
- // If the block is not cached above, send packet below. Both
- // CleanEvict and Writeback with BLOCK_CACHED flag cleared will
- // reset the bit corresponding to this address in the snoop
filter
- // below.
- memSidePort.sendAtomic(wbPkt);
+ // Call isCachedAbove for both Writebacks and CleanEvicts. If
+ // isCachedAbove returns true we set BLOCK_CACHED flag in Writebacks
+ // and discard CleanEvicts.
+ if (isCachedAbove(pkt, false)) {
+ if (pkt->cmd == MemCmd::WritebackDirty ||
+ pkt->cmd == MemCmd::WriteClean) {
+ // Set BLOCK_CACHED flag in Writeback and send below,
+ // so that the Writeback does not reset the bit
+ // corresponding to this address in the snoop filter
+ // below. We can discard CleanEvicts because cached
+ // copies exist above. Atomic mode isCachedAbove
+ // modifies packet to set BLOCK_CACHED flag
+ memSidePort.sendAtomic(pkt);
}
- writebacks.pop_front();
- // In case of CleanEvicts, the packet destructor will delete the
- // request object because this is a non-snoop request packet which
- // does not require a response.
- delete wbPkt;
+ } else {
+ // If the block is not cached above, send packet below. Both
+ // CleanEvict and Writeback with BLOCK_CACHED flag cleared will
+ // reset the bit corresponding to this address in the snoop filter
+ // below.
+ memSidePort.sendAtomic(pkt);
}
-}
+ // In case of CleanEvicts, the packet destructor will delete the
+ // request object because this is a non-snoop request packet which
+ // does not require a response.
+}
void
Cache::recvTimingSnoopResp(PacketPtr pkt)
@@ -1030,7 +1025,7 @@
pkt->headerDelay;
doWritebacks(writebacks, forward_time);
} else {
- doWritebacksAtomic(writebacks);
+ BaseCache::doWritebacksAtomic(writebacks);
}
pkt->setSatisfied();
}
@@ -1273,6 +1268,31 @@
return 0;
}
+ const unsigned blk_size = blkSize;
+ auto wb_pkt = std::find_if(tempWritebuffer.begin(),
tempWritebuffer.end(),
+ [pkt, blk_size](const PacketPtr wb_pkt)
+ {
+ return pkt->matchBlockAddr(wb_pkt, blk_size);
+ });
+ if (wb_pkt != tempWritebuffer.end()) {
+ assert((*wb_pkt)->isEviction() ||
+ (*wb_pkt)->cmd == MemCmd::WriteClean);
+
+ DPRINTF(Cache, "Snoop %s hit to atomic writebuffer %s\n",
pkt->print(),
+ (*wb_pkt)->print());
+
+ if (pkt->isEviction()) {
+ // if the block is found in the write queue, set the
BLOCK_CACHED
+ // flag for Writeback/CleanEvict snoop. On return the snoop
will
+ // propagate the BLOCK_CACHED flag in Writeback packets and
prevent
+ // any CleanEvicts from travelling down the memory hierarchy.
+ pkt->setBlockCached();
+ DPRINTF(Cache, "%s: Squashing %s from lower cache on atomic " \
+ "writebuffer hit\n", __func__, pkt->print());
+ return lookupLatency * clockPeriod();
+ }
+ }
+
CacheBlk *blk = tags->findBlock(pkt->getAddr(), pkt->isSecure());
uint32_t snoop_delay = handleSnoop(pkt, blk, false, false, false);
return snoop_delay + lookupLatency * clockPeriod();
diff --git a/src/mem/cache/cache.hh b/src/mem/cache/cache.hh
index 33c5a24..a99d583 100644
--- a/src/mem/cache/cache.hh
+++ b/src/mem/cache/cache.hh
@@ -101,7 +101,7 @@
void doWritebacks(PacketList& writebacks, Tick forward_time) override;
- void doWritebacksAtomic(PacketList& writebacks) override;
+ void doWritebacksAtomic(PacketPtr pkt) override;
void serviceMSHRTargets(MSHR *mshr, const PacketPtr pkt,
CacheBlk *blk) override;
diff --git a/src/mem/cache/noncoherent_cache.cc
b/src/mem/cache/noncoherent_cache.cc
index bc2b096..0b76e29 100644
--- a/src/mem/cache/noncoherent_cache.cc
+++ b/src/mem/cache/noncoherent_cache.cc
@@ -108,14 +108,9 @@
}
void
-NoncoherentCache::doWritebacksAtomic(PacketList& writebacks)
+NoncoherentCache::doWritebacksAtomic(PacketPtr pkt)
{
- while (!writebacks.empty()) {
- PacketPtr wb_pkt = writebacks.front();
- memSidePort.sendAtomic(wb_pkt);
- writebacks.pop_front();
- delete wb_pkt;
- }
+ memSidePort.sendAtomic(pkt);
}
void
diff --git a/src/mem/cache/noncoherent_cache.hh
b/src/mem/cache/noncoherent_cache.hh
index 3da87d9..6e84761 100644
--- a/src/mem/cache/noncoherent_cache.hh
+++ b/src/mem/cache/noncoherent_cache.hh
@@ -83,7 +83,7 @@
void doWritebacks(PacketList& writebacks,
Tick forward_time) override;
- void doWritebacksAtomic(PacketList& writebacks) override;
+ void doWritebacksAtomic(PacketPtr pkt) override;
void serviceMSHRTargets(MSHR *mshr, const PacketPtr pkt,
CacheBlk *blk) override;
--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/18209
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: I2fc2f9eb0f26248ddf91adbe987d158f5a2e592b
Gerrit-Change-Number: 18209
Gerrit-PatchSet: 1
Gerrit-Owner: Daniel Carvalho <[email protected]>
Gerrit-MessageType: newchange
_______________________________________________
gem5-dev mailing list
[email protected]
http://m5sim.org/mailman/listinfo/gem5-dev
