[gem5-dev] Change in gem5/gem5[master]: mem-cache: Refactor the cache recvTimingResp function
Nikos Nikoleris has submitted this change and it was merged. (
https://gem5-review.googlesource.com/10423 )
Change subject: mem-cache: Refactor the cache recvTimingResp function
..
mem-cache: Refactor the cache recvTimingResp function
The recvTimingResp function in the cache handles timing
responses. Over time, recvTimingResp has grown in complexity and code
size. This change factors out some of its functionality to a separate
function. The new function iterates through the in-service targets and
handles them accordingly.
Change-Id: I0ef28288640f6be1b30452b0664d32432e692ea6
Reviewed-on: https://gem5-review.googlesource.com/10423
Reviewed-by: Daniel Carvalho
Maintainer: Nikos Nikoleris
---
M src/mem/cache/cache.cc
M src/mem/cache/cache.hh
2 files changed, 113 insertions(+), 89 deletions(-)
Approvals:
Daniel Carvalho: Looks good to me, approved
Nikos Nikoleris: Looks good to me, approved
diff --git a/src/mem/cache/cache.cc b/src/mem/cache/cache.cc
index e9aec49..dfdb5ee 100644
--- a/src/mem/cache/cache.cc
+++ b/src/mem/cache/cache.cc
@@ -1322,99 +1322,20 @@
}
void
-Cache::recvTimingResp(PacketPtr pkt)
+Cache::serviceMSHRTargets(MSHR *mshr, const PacketPtr pkt, CacheBlk *blk,
+ PacketList &writebacks)
{
-assert(pkt->isResponse());
-
-// all header delay should be paid for by the crossbar, unless
-// this is a prefetch response from above
-panic_if(pkt->headerDelay != 0 && pkt->cmd != MemCmd::HardPFResp,
- "%s saw a non-zero packet delay\n", name());
-
-bool is_error = pkt->isError();
-
-if (is_error) {
-DPRINTF(Cache, "%s: Cache received %s with error\n", __func__,
-pkt->print());
-}
-
-DPRINTF(Cache, "%s: Handling response %s\n", __func__,
-pkt->print());
-
-// if this is a write, we should be looking at an uncacheable
-// write
-if (pkt->isWrite()) {
-assert(pkt->req->isUncacheable());
-handleUncacheableWriteResp(pkt);
-return;
-}
-
-// we have dealt with any (uncacheable) writes above, from here on
-// we know we are dealing with an MSHR due to a miss or a prefetch
-MSHR *mshr = dynamic_cast(pkt->popSenderState());
-assert(mshr);
-
-if (mshr == noTargetMSHR) {
-// we always clear at least one target
-clearBlocked(Blocked_NoTargets);
-noTargetMSHR = nullptr;
-}
-
-// Initial target is used just for stats
MSHR::Target *initial_tgt = mshr->getTarget();
-int stats_cmd_idx = initial_tgt->pkt->cmdToIndex();
-Tick miss_latency = curTick() - initial_tgt->recvTime;
+// First offset for critical word first calculations
+const int initial_offset = initial_tgt->pkt->getOffset(blkSize);
-if (pkt->req->isUncacheable()) {
-assert(pkt->req->masterId() < system->maxMasters());
-mshr_uncacheable_lat[stats_cmd_idx][pkt->req->masterId()] +=
-miss_latency;
-} else {
-assert(pkt->req->masterId() < system->maxMasters());
-mshr_miss_latency[stats_cmd_idx][pkt->req->masterId()] +=
-miss_latency;
-}
-
-bool wasFull = mshrQueue.isFull();
-
-PacketList writebacks;
-
-Tick forward_time = clockEdge(forwardLatency) + pkt->headerDelay;
-
+const bool is_error = pkt->isError();
bool is_fill = !mshr->isForward &&
(pkt->isRead() || pkt->cmd == MemCmd::UpgradeResp);
-
-CacheBlk *blk = tags->findBlock(pkt->getAddr(), pkt->isSecure());
-const bool valid_blk = blk && blk->isValid();
-// If the response indicates that there are no sharers and we
-// either had the block already or the response is filling we can
-// promote our copy to writable
-if (!pkt->hasSharers() &&
-(is_fill || (valid_blk && !pkt->req->isCacheInvalidate( {
-mshr->promoteWritable();
-}
-
-if (is_fill && !is_error) {
-DPRINTF(Cache, "Block for addr %#llx being updated in Cache\n",
-pkt->getAddr());
-
-blk = handleFill(pkt, blk, writebacks, mshr->allocOnFill());
-assert(blk != nullptr);
-}
-
// allow invalidation responses originating from write-line
// requests to be discarded
bool is_invalidate = pkt->isInvalidate();
-// The block was marked as not readable while there was a pending
-// cache maintenance operation, restore its flag.
-if (pkt->isClean() && !is_invalidate && valid_blk) {
-blk->status |= BlkReadable;
-}
-
-// First offset for critical word first calculations
-int initial_offset = initial_tgt->pkt->getOffset(blkSize);
-
MSHR::TargetList targets = mshr->extractServiceableTargets(pkt);
for (auto &target: targets) {
Packet *tgt_pkt = target.pkt;
@@ -1450,7 +1371,7 @@
// NB: we use the original packet here and not the
response!
blk = handleFill(tgt_pkt, blk, writebacks,
[gem5-dev] Change in gem5/gem5[master]: mem-cache: Refactor the cache recvTimingResp function
Hello Daniel Carvalho, Jason Lowe-Power, I'd like you to reexamine a change. Please visit https://gem5-review.googlesource.com/10423 to look at the new patch set (#4). Change subject: mem-cache: Refactor the cache recvTimingResp function .. mem-cache: Refactor the cache recvTimingResp function The recvTimingResp function in the cache handles timing responses. Over time, recvTimingResp has grown in complexity and code size. This change factors out some of its functionality to a separate function. The new function iterates through the in-service targets and handles them accordingly. Change-Id: I0ef28288640f6be1b30452b0664d32432e692ea6 --- M src/mem/cache/cache.cc M src/mem/cache/cache.hh 2 files changed, 113 insertions(+), 89 deletions(-) -- To view, visit https://gem5-review.googlesource.com/10423 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: I0ef28288640f6be1b30452b0664d32432e692ea6 Gerrit-Change-Number: 10423 Gerrit-PatchSet: 4 Gerrit-Owner: Nikos Nikoleris Gerrit-Reviewer: Daniel Carvalho Gerrit-Reviewer: Jason Lowe-Power Gerrit-Reviewer: Nikos Nikoleris Gerrit-MessageType: newpatchset ___ gem5-dev mailing list gem5-dev@gem5.org http://m5sim.org/mailman/listinfo/gem5-dev
[gem5-dev] Change in gem5/gem5[master]: mem-cache: Refactor the cache recvTimingResp function
Hello Daniel Carvalho, Jason Lowe-Power, I'd like you to reexamine a change. Please visit https://gem5-review.googlesource.com/10423 to look at the new patch set (#3). Change subject: mem-cache: Refactor the cache recvTimingResp function .. mem-cache: Refactor the cache recvTimingResp function The recvTimingResp function in the cache handles timing responses. Over time, recvTimingResp has grown in complexity and code size. This change factors out some of its functionality to a separate function. The new function iterates through the in-service targets and handles them accordingly. Change-Id: I0ef28288640f6be1b30452b0664d32432e692ea6 --- M src/mem/cache/cache.cc M src/mem/cache/cache.hh 2 files changed, 115 insertions(+), 91 deletions(-) -- To view, visit https://gem5-review.googlesource.com/10423 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: I0ef28288640f6be1b30452b0664d32432e692ea6 Gerrit-Change-Number: 10423 Gerrit-PatchSet: 3 Gerrit-Owner: Nikos Nikoleris Gerrit-Reviewer: Daniel Carvalho Gerrit-Reviewer: Jason Lowe-Power Gerrit-Reviewer: Nikos Nikoleris Gerrit-MessageType: newpatchset ___ gem5-dev mailing list gem5-dev@gem5.org http://m5sim.org/mailman/listinfo/gem5-dev
[gem5-dev] Change in gem5/gem5[master]: mem-cache: Refactor the cache recvTimingResp function
Nikos Nikoleris has uploaded a new patch set (#2). ( https://gem5-review.googlesource.com/10423 ) Change subject: mem-cache: Refactor the cache recvTimingResp function .. mem-cache: Refactor the cache recvTimingResp function The recvTimingResp function in the cache handles timing responses. Over time, recvTimingResp has grown in complexity and code size. This change factors out some of its functionality to a separate function. The new function iterates through the in-service targets and handles them accordingly. Change-Id: I0ef28288640f6be1b30452b0664d32432e692ea6 --- M src/mem/cache/cache.cc M src/mem/cache/cache.hh 2 files changed, 115 insertions(+), 91 deletions(-) -- To view, visit https://gem5-review.googlesource.com/10423 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: I0ef28288640f6be1b30452b0664d32432e692ea6 Gerrit-Change-Number: 10423 Gerrit-PatchSet: 2 Gerrit-Owner: Nikos Nikoleris Gerrit-Reviewer: Nikos Nikoleris Gerrit-CC: Daniel Carvalho Gerrit-MessageType: newpatchset ___ gem5-dev mailing list gem5-dev@gem5.org http://m5sim.org/mailman/listinfo/gem5-dev
[gem5-dev] Change in gem5/gem5[master]: mem-cache: Refactor the cache recvTimingResp function
Nikos Nikoleris has uploaded this change for review. (
https://gem5-review.googlesource.com/10423
Change subject: mem-cache: Refactor the cache recvTimingResp function
..
mem-cache: Refactor the cache recvTimingResp function
The recvTimingResp function in the cache handles timing
responses. Over time, recvTimingResp has grown in complexity and code
size. This change factors out some of its functionality to a separate
function. The new function iterates through the in-service targets and
handles them accordingly.
Change-Id: I0ef28288640f6be1b30452b0664d32432e692ea6
---
M src/mem/cache/cache.cc
M src/mem/cache/cache.hh
2 files changed, 115 insertions(+), 96 deletions(-)
diff --git a/src/mem/cache/cache.cc b/src/mem/cache/cache.cc
index b9625be..c0d4401 100644
--- a/src/mem/cache/cache.cc
+++ b/src/mem/cache/cache.cc
@@ -1322,100 +1322,20 @@
}
void
-Cache::recvTimingResp(PacketPtr pkt)
+Cache::serviceMSHRTargets(MSHR *mshr, const PacketPtr pkt, CacheBlk *blk,
+ PacketList &writebacks)
{
-assert(pkt->isResponse());
-
-// all header delay should be paid for by the crossbar, unless
-// this is a prefetch response from above
-panic_if(pkt->headerDelay != 0 && pkt->cmd != MemCmd::HardPFResp,
- "%s saw a non-zero packet delay\n", name());
-
-bool is_error = pkt->isError();
-
-if (is_error) {
-DPRINTF(Cache, "%s: Cache received %s with error\n", __func__,
-pkt->print());
-}
-
-DPRINTF(Cache, "%s: Handling response %s\n", __func__,
-pkt->print());
-
-// if this is a write, we should be looking at an uncacheable
-// write
-if (pkt->isWrite()) {
-assert(pkt->req->isUncacheable());
-handleUncacheableWriteResp(pkt);
-return;
-}
-
-// we have dealt with any (uncacheable) writes above, from here on
-// we know we are dealing with an MSHR due to a miss or a prefetch
-MSHR *mshr = dynamic_cast(pkt->popSenderState());
-assert(mshr);
-
-if (mshr == noTargetMSHR) {
-// we always clear at least one target
-clearBlocked(Blocked_NoTargets);
-noTargetMSHR = nullptr;
-}
-
-// Initial target is used just for stats
MSHR::Target *initial_tgt = mshr->getTarget();
-int stats_cmd_idx = initial_tgt->pkt->cmdToIndex();
-Tick miss_latency = curTick() - initial_tgt->recvTime;
+// First offset for critical word first calculations
+int initial_offset = initial_tgt->pkt->getOffset(blkSize);
-if (pkt->req->isUncacheable()) {
-assert(pkt->req->masterId() < system->maxMasters());
-mshr_uncacheable_lat[stats_cmd_idx][pkt->req->masterId()] +=
-miss_latency;
-} else {
-assert(pkt->req->masterId() < system->maxMasters());
-mshr_miss_latency[stats_cmd_idx][pkt->req->masterId()] +=
-miss_latency;
-}
-
-bool wasFull = mshrQueue.isFull();
-
-PacketList writebacks;
-
-Tick forward_time = clockEdge(forwardLatency) + pkt->headerDelay;
-
+const bool is_error = pkt->isError();
bool is_fill = !mshr->isForward &&
(pkt->isRead() || pkt->cmd == MemCmd::UpgradeResp);
-
-CacheBlk *blk = tags->findBlock(pkt->getAddr(), pkt->isSecure());
-const bool valid_blk = blk && blk->isValid();
-// If the response indicates that there are no sharers and we
-// either had the block already or the response is filling we can
-// promote our copy to writable
-if (!pkt->hasSharers() &&
-(is_fill || (valid_blk && !pkt->req->isCacheInvalidate( {
-mshr->promoteWritable();
-}
-
-if (is_fill && !is_error) {
-DPRINTF(Cache, "Block for addr %#llx being updated in Cache\n",
-pkt->getAddr());
-
-blk = handleFill(pkt, blk, writebacks, mshr->allocOnFill());
-assert(blk != nullptr);
-}
-
// allow invalidation responses originating from write-line
// requests to be discarded
bool is_invalidate = pkt->isInvalidate();
-// The block was marked as not readable while there was a pending
-// cache maintenance operation, restore its flag.
-if (pkt->isClean() && !is_invalidate && valid_blk) {
-blk->status |= BlkReadable;
-}
-
-// First offset for critical word first calculations
-int initial_offset = initial_tgt->pkt->getOffset(blkSize);
-
-bool from_cache = false;
MSHR::TargetList targets = mshr->extractServiceableTargets(pkt);
for (auto &target: targets) {
Packet *tgt_pkt = target.pkt;
@@ -1437,10 +1357,6 @@
break; // skip response
}
-// keep track of whether we have responded to another
-// cache
-from_cache = from_cache || tgt_pkt->fromCache();
-
// unlike the other packet flows, where data is found in other
// caches or memory and brought back, write-li
