[gem5-dev] Change in gem5/gem5[master]: mem-cache: Remove writebacks packet list

2019-05-08 Thread Daniel Carvalho (Gerrit)
Daniel Carvalho has submitted this change and it was merged. (  
https://gem5-review.googlesource.com/c/public/gem5/+/18209 )


Change subject: mem-cache: Remove writebacks packet list
..

mem-cache: Remove writebacks packet list

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. Since atomic mode does
not have a writebuffer, this kind of conflict wouldn't be noticed.

Therefore, instead of evicting multiple blocks at once, we
do it one by one.

Change-Id: I2fc2f9eb0f26248ddf91adbe987d158f5a2e592b
Signed-off-by: Daniel R. Carvalho 
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/18209
Tested-by: kokoro 
Reviewed-by: Nikos Nikoleris 
Maintainer: Nikos Nikoleris 
---
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, 143 insertions(+), 192 deletions(-)

Approvals:
  Nikos Nikoleris: Looks good to me, approved; Looks good to me, approved
  kokoro: Regressions pass



diff --git a/src/mem/cache/base.cc b/src/mem/cache/base.cc
index 8d7d193..8929343 100644
--- a/src/mem/cache/base.cc
+++ b/src/mem/cache/base.cc
@@ -342,20 +342,11 @@
 // the delay provided by the crossbar
 Tick forward_time = clockEdge(forwardLatency) + pkt->headerDelay;

+// Note that lat is passed by reference here. The function
+// access() will set the lat value.
 Cycles lat;
 CacheBlk *blk = nullptr;
-bool satisfied = false;
-{
-PacketList writebacks;
-// Note that lat is passed by reference here. The function
-// access() will set the lat value.
-satisfied = access(pkt, blk, lat, writebacks);
-
-// After the evicted blocks are selected, they must be forwarded
-// to the write buffer to ensure they logically precede anything
-// happening below
-doWritebacks(writebacks, clockEdge(lat + forwardLatency));
-}
+bool satisfied = access(pkt, blk, lat);

 // Here we charge the headerDelay that takes into account the latencies
 // of the bus, if the packet comes from it.
@@ -457,8 +448,6 @@
 miss_latency;
 }

-PacketList writebacks;
-
 bool is_fill = !mshr->isForward &&
 (pkt->isRead() || pkt->cmd == MemCmd::UpgradeResp ||
  mshr->wasWholeLineWrite);
@@ -475,7 +464,7 @@

 const bool allocate = (writeAllocator && mshr->wasWholeLineWrite) ?
 writeAllocator->allocate() : mshr->allocOnFill();
-blk = handleFill(pkt, blk, writebacks, allocate);
+blk = handleFill(pkt, blk, allocate);
 assert(blk != nullptr);
 ppFill->notify(pkt);
 }
@@ -531,13 +520,9 @@

 // if we used temp block, check to see if its valid and then clear it  
out

 if (blk == tempBlock && tempBlock->isValid()) {
-evictBlock(blk, writebacks);
+evictBlock(blk, clockEdge(forwardLatency) + pkt->headerDelay);
 }

-const Tick forward_time = clockEdge(forwardLatency) + pkt->headerDelay;
-// copy writebacks to write buffer
-doWritebacks(writebacks, forward_time);
-
 DPRINTF(CacheVerbose, "%s: Leaving with %s\n", __func__, pkt->print());
 delete pkt;
 }
@@ -555,8 +540,7 @@
 Cycles lat = lookupLatency;

 CacheBlk *blk = nullptr;
-PacketList writebacks;
-bool satisfied = access(pkt, blk, lat, writebacks);
+bool satisfied = access(pkt, blk, lat);

 if (pkt->isClean() && blk && blk->isDirty()) {
 // A cache clean opearation is looking for a dirty
@@ -566,17 +550,12 @@
 DPRINTF(CacheVerbose, "%s: packet %s found block: %s\n",
 __func__, pkt->print(), blk->print());
 PacketPtr wb_pkt = writecleanBlk(blk, pkt->req->getDest(),  
pkt->id);

-writebacks.push_back(wb_pkt);
 pkt->setSatisfied();
+doWritebacksAtomic(wb_pkt);
 }

-// handle writebacks resulting from the access here to ensure they
-// logically precede anything happening below
-doWritebacksAtomic(writebacks);
-assert(writebacks.empty());
-
 if (!satisfied) {
-lat += handleAtomicReqMiss(pkt, blk, writebacks);
+lat += handleAtomicReqMiss(pkt, blk);
 }

 // Note that we don't invoke the prefetcher at all in atomic mode.
@@ -590,9 +569,6 @@
 // immediately rather than calling requestMemSideBus() as we do
 // there).

-// do any writebacks resulting from the response handling
-doWritebacksAtomic(writebacks);
-
 // if 

[gem5-dev] Change in gem5/gem5[master]: mem-cache: Remove writebacks packet list

2019-05-03 Thread Daniel Carvalho (Gerrit)

Hello kokoro, Jason Lowe-Power, Nikos Nikoleris,

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

https://gem5-review.googlesource.com/c/public/gem5/+/18209

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

Change subject: mem-cache: Remove writebacks packet list
..

mem-cache: Remove writebacks packet list

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. Since atomic mode does
not have a writebuffer, this kind of conflict wouldn't be noticed.

Therefore, instead of evicting multiple blocks at once, we
do it one by one.

Change-Id: I2fc2f9eb0f26248ddf91adbe987d158f5a2e592b
Signed-off-by: Daniel R. Carvalho 
---
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, 143 insertions(+), 192 deletions(-)


--
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: 4
Gerrit-Owner: Daniel Carvalho 
Gerrit-Reviewer: Daniel Carvalho 
Gerrit-Reviewer: Jason Lowe-Power 
Gerrit-Reviewer: Nikos Nikoleris 
Gerrit-Reviewer: kokoro 
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: Remove writebacks packet list

2019-04-30 Thread Daniel Carvalho (Gerrit)

Hello kokoro, Jason Lowe-Power, Nikos Nikoleris,

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

https://gem5-review.googlesource.com/c/public/gem5/+/18209

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

Change subject: mem-cache: Remove writebacks packet list
..

mem-cache: Remove writebacks packet list

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. Since atomic mode does
not have a writebuffer, this kind of conflict wouldn't be noticed.

Therefore, instead of evicting multiple blocks at once, we
do it one by one.

Change-Id: I2fc2f9eb0f26248ddf91adbe987d158f5a2e592b
Signed-off-by: Daniel R. Carvalho 
---
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, 157 insertions(+), 190 deletions(-)


--
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: 3
Gerrit-Owner: Daniel Carvalho 
Gerrit-Reviewer: Daniel Carvalho 
Gerrit-Reviewer: Jason Lowe-Power 
Gerrit-Reviewer: Nikos Nikoleris 
Gerrit-Reviewer: kokoro 
Gerrit-MessageType: newpatchset
___
gem5-dev mailing list
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev