[gem5-dev] Change in gem5/gem5[master]: mem-cache: Refactor the cache recvTimingReq function

[Nikos Nikoleris (Gerrit)]

Nikos Nikoleris has submitted this change and it was merged. (  
https://gem5-review.googlesource.com/10424 )


Change subject: mem-cache: Refactor the cache recvTimingReq function
..

mem-cache: Refactor the cache recvTimingReq function

The recvTimingReq function in the cache handles timing requests. Over
time, recvTimingReq has grown in complexity and code size. This change
factors out some of its functionality in two separate functions. The
new functions handle timing requests that hit and timing requests that
miss separately.

Change-Id: I09902d648d7272f0f9ec2851fa6376f7305ba418
Reviewed-on: https://gem5-review.googlesource.com/10424
Reviewed-by: Jason Lowe-Power 
Reviewed-by: Daniel Carvalho 
Maintainer: Nikos Nikoleris 
---
M src/mem/cache/cache.cc
M src/mem/cache/cache.hh
2 files changed, 223 insertions(+), 192 deletions(-)

Approvals:
  Jason Lowe-Power: Looks good to me, approved
  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 dfdb5ee..9b26675 100644
--- a/src/mem/cache/cache.cc
+++ b/src/mem/cache/cache.cc
@@ -643,6 +643,191 @@
 }

 void
+Cache::handleTimingReqHit(PacketPtr pkt, CacheBlk *blk, Tick request_time)
+{
+// should never be satisfying an uncacheable access as we
+// flush and invalidate any existing block as part of the
+// lookup
+assert(!pkt->req->isUncacheable());
+
+if (pkt->needsResponse()) {
+pkt->makeTimingResponse();
+// @todo: Make someone pay for this
+pkt->headerDelay = pkt->payloadDelay = 0;
+
+// In this case we are considering request_time that takes
+// into account the delay of the xbar, if any, and just
+// lat, neglecting responseLatency, modelling hit latency
+// just as lookupLatency or or the value of lat overriden
+// by access(), that calls accessBlock() function.
+cpuSidePort->schedTimingResp(pkt, request_time, true);
+} else {
+DPRINTF(Cache, "%s satisfied %s, no response needed\n", __func__,
+pkt->print());
+
+// queue the packet for deletion, as the sending cache is
+// still relying on it; if the block is found in access(),
+// CleanEvict and Writeback messages will be deleted
+// here as well
+pendingDelete.reset(pkt);
+}
+}
+
+void
+Cache::handleTimingReqMiss(PacketPtr pkt, CacheBlk *blk, Tick forward_time,
+   Tick request_time)
+{
+Addr blk_addr = pkt->getBlockAddr(blkSize);
+
+// ignore any existing MSHR if we are dealing with an
+// uncacheable request
+MSHR *mshr = pkt->req->isUncacheable() ? nullptr :
+mshrQueue.findMatch(blk_addr, pkt->isSecure());
+
+// Software prefetch handling:
+// To keep the core from waiting on data it won't look at
+// anyway, send back a response with dummy data. Miss handling
+// will continue asynchronously. Unfortunately, the core will
+// insist upon freeing original Packet/Request, so we have to
+// create a new pair with a different lifecycle. Note that this
+// processing happens before any MSHR munging on the behalf of
+// this request because this new Request will be the one stored
+// into the MSHRs, not the original.
+if (pkt->cmd.isSWPrefetch()) {
+assert(pkt->needsResponse());
+assert(pkt->req->hasPaddr());
+assert(!pkt->req->isUncacheable());
+
+// There's no reason to add a prefetch as an additional target
+// to an existing MSHR. If an outstanding request is already
+// in progress, there is nothing for the prefetch to do.
+// If this is the case, we don't even create a request at all.
+PacketPtr pf = nullptr;
+
+if (!mshr) {
+// copy the request and create a new SoftPFReq packet
+RequestPtr req = new Request(pkt->req->getPaddr(),
+ pkt->req->getSize(),
+ pkt->req->getFlags(),
+ pkt->req->masterId());
+pf = new Packet(req, pkt->cmd);
+pf->allocate();
+assert(pf->getAddr() == pkt->getAddr());
+assert(pf->getSize() == pkt->getSize());
+}
+
+pkt->makeTimingResponse();
+
+// request_time is used here, taking into account lat and the delay
+// charged if the packet comes from the xbar.
+cpuSidePort->schedTimingResp(pkt, request_time, true);
+
+// If an outstanding request is in progress (we found an
+// MSHR) this is set to null
+pkt = pf;
+}
+
+if (mshr) {
+/// MSHR hit
+/// @note writebacks will be checked in getNextMSHR()
+/// for any conflicting requests to the same block
+
+//@todo remove hw_pf here
+
+// 

[gem5-dev] Change in gem5/gem5[master]: mem-cache: Refactor the cache recvTimingReq function

[Nikos Nikoleris (Gerrit)]

Hello Daniel Carvalho, Jason Lowe-Power,

I'd like you to reexamine a change. Please visit

https://gem5-review.googlesource.com/10424

to look at the new patch set (#4).

Change subject: mem-cache: Refactor the cache recvTimingReq function
..

mem-cache: Refactor the cache recvTimingReq function

The recvTimingReq function in the cache handles timing requests. Over
time, recvTimingReq has grown in complexity and code size. This change
factors out some of its functionality in two separate functions. The
new functions handle timing requests that hit and timing requests that
miss separately.

Change-Id: I09902d648d7272f0f9ec2851fa6376f7305ba418
---
M src/mem/cache/cache.cc
M src/mem/cache/cache.hh
2 files changed, 223 insertions(+), 192 deletions(-)


--
To view, visit https://gem5-review.googlesource.com/10424
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: I09902d648d7272f0f9ec2851fa6376f7305ba418
Gerrit-Change-Number: 10424
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 recvTimingReq function

[Nikos Nikoleris (Gerrit)]

Hello Daniel Carvalho, Jason Lowe-Power,

I'd like you to reexamine a change. Please visit

https://gem5-review.googlesource.com/10424

to look at the new patch set (#3).

Change subject: mem-cache: Refactor the cache recvTimingReq function
..

mem-cache: Refactor the cache recvTimingReq function

The recvTimingReq function in the cache handles timing requests. Over
time, recvTimingReq has grown in complexity and code size. This change
factors out some of its functionality in two separate functions. The
new functions handle timing requests that hit and timing requests that
miss separately.

Change-Id: I09902d648d7272f0f9ec2851fa6376f7305ba418
---
M src/mem/cache/cache.cc
M src/mem/cache/cache.hh
2 files changed, 28 insertions(+), 5 deletions(-)


--
To view, visit https://gem5-review.googlesource.com/10424
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: I09902d648d7272f0f9ec2851fa6376f7305ba418
Gerrit-Change-Number: 10424
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 recvTimingReq function

[Nikos Nikoleris (Gerrit)]

Hello Daniel Carvalho,

I'd like you to reexamine a change. Please visit

https://gem5-review.googlesource.com/10424

to look at the new patch set (#2).

Change subject: mem-cache: Refactor the cache recvTimingReq function
..

mem-cache: Refactor the cache recvTimingReq function

The recvTimingReq function in the cache handles timing requests. Over
time, recvTimingReq has grown in complexity and code size. This change
factors out some of its functionality in two separate functions. The
new functions handle timing requests that hit and timing requests that
miss separately.

Change-Id: I09902d648d7272f0f9ec2851fa6376f7305ba418
---
M src/mem/cache/cache.cc
M src/mem/cache/cache.hh
2 files changed, 223 insertions(+), 192 deletions(-)


--
To view, visit https://gem5-review.googlesource.com/10424
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: I09902d648d7272f0f9ec2851fa6376f7305ba418
Gerrit-Change-Number: 10424
Gerrit-PatchSet: 2
Gerrit-Owner: Nikos Nikoleris 
Gerrit-Reviewer: Daniel Carvalho 
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 recvTimingReq function

[Nikos Nikoleris (Gerrit)]

Nikos Nikoleris has uploaded this change for review. (  
https://gem5-review.googlesource.com/10424



Change subject: mem-cache: Refactor the cache recvTimingReq function
..

mem-cache: Refactor the cache recvTimingReq function

The recvTimingReq function in the cache handles timing requests. Over
time, recvTimingReq has grown in complexity and code size. This change
factors out some of its functionality in two separate functions. The
new functions handle timing requests that hit and timing requests that
miss separately.

Change-Id: I09902d648d7272f0f9ec2851fa6376f7305ba418
---
M src/mem/cache/cache.cc
M src/mem/cache/cache.hh
2 files changed, 247 insertions(+), 210 deletions(-)



diff --git a/src/mem/cache/cache.cc b/src/mem/cache/cache.cc
index c0d4401..79e7613 100644
--- a/src/mem/cache/cache.cc
+++ b/src/mem/cache/cache.cc
@@ -643,6 +643,231 @@
 }

 void
+Cache::handleTimingReqHit(PacketPtr pkt, CacheBlk *blk, Tick request_time)
+{
+// should never be satisfying an uncacheable access as we
+// flush and invalidate any existing block as part of the
+// lookup
+assert(!pkt->req->isUncacheable());
+
+// track time of availability of next prefetch, if any
+Tick next_pf_time = MaxTick;
+
+// hit (for all other request types)
+
+if (prefetcher && (prefetchOnAccess ||
+   (blk && blk->wasPrefetched( {
+if (blk)
+blk->status &= ~BlkHWPrefetched;
+
+// Don't notify on SWPrefetch
+if (!pkt->cmd.isSWPrefetch()) {
+assert(!pkt->req->isCacheMaintenance());
+next_pf_time = prefetcher->notify(pkt);
+}
+}
+
+if (pkt->needsResponse()) {
+pkt->makeTimingResponse();
+// @todo: Make someone pay for this
+pkt->headerDelay = pkt->payloadDelay = 0;
+
+// In this case we are considering request_time that takes
+// into account the delay of the xbar, if any, and just
+// lat, neglecting responseLatency, modelling hit latency
+// just as lookupLatency or or the value of lat overriden
+// by access(), that calls accessBlock() function.
+cpuSidePort->schedTimingResp(pkt, request_time, true);
+} else {
+DPRINTF(Cache, "%s satisfied %s, no response needed\n", __func__,
+pkt->print());
+
+// queue the packet for deletion, as the sending cache is
+// still relying on it; if the block is found in access(),
+// CleanEvict and Writeback messages will be deleted
+// here as well
+pendingDelete.reset(pkt);
+}
+
+if (next_pf_time != MaxTick)
+schedMemSideSendEvent(next_pf_time);
+}
+
+void
+Cache::handleTimingReqMiss(PacketPtr pkt, CacheBlk *blk, Tick forward_time,
+   Tick request_time)
+{
+Addr blk_addr = pkt->getBlockAddr(blkSize);
+
+// ignore any existing MSHR if we are dealing with an
+// uncacheable request
+MSHR *mshr = pkt->req->isUncacheable() ? nullptr :
+mshrQueue.findMatch(blk_addr, pkt->isSecure());
+
+// Software prefetch handling:
+// To keep the core from waiting on data it won't look at
+// anyway, send back a response with dummy data. Miss handling
+// will continue asynchronously. Unfortunately, the core will
+// insist upon freeing original Packet/Request, so we have to
+// create a new pair with a different lifecycle. Note that this
+// processing happens before any MSHR munging on the behalf of
+// this request because this new Request will be the one stored
+// into the MSHRs, not the original.
+if (pkt->cmd.isSWPrefetch()) {
+assert(pkt->needsResponse());
+assert(pkt->req->hasPaddr());
+assert(!pkt->req->isUncacheable());
+
+// There's no reason to add a prefetch as an additional target
+// to an existing MSHR. If an outstanding request is already
+// in progress, there is nothing for the prefetch to do.
+// If this is the case, we don't even create a request at all.
+PacketPtr pf = nullptr;
+
+if (!mshr) {
+// copy the request and create a new SoftPFReq packet
+RequestPtr req = new Request(pkt->req->getPaddr(),
+ pkt->req->getSize(),
+ pkt->req->getFlags(),
+ pkt->req->masterId());
+pf = new Packet(req, pkt->cmd);
+pf->allocate();
+assert(pf->getAddr() == pkt->getAddr());
+assert(pf->getSize() == pkt->getSize());
+}
+
+pkt->makeTimingResponse();
+
+// request_time is used here, taking into account lat and the delay
+// charged if the packet comes from the xbar.
+cpuSidePort->schedTimingResp(pkt, request_time, true);
+
+// If an outstanding request is in progress (we