> On Nov. 4, 2015, 7:11 a.m., Steve Reinhardt wrote: > > src/mem/packet.hh, line 148 > > <http://reviews.gem5.org/r/3158/diff/2/?file=51193#file51193line148> > > > > maybe instead of IsWriteback, we should add IsEviction, and set that on > > all three commands > > > > then we could: > > - replace evictingBlock() with isEviction() > > - define isWriteback() as isEviction() && hasData() > > - create an isCleanEviction() that is isEviction() && > > !needsExclusive() (assuming we get rid of NeedsExclusive on > > WritebackClean), and use that to replace all the "== CleanEvict || == > > WritebackClean" expressions
I would prefer the addition of "IsEviction" to be in a separate follow-on patch. > On Nov. 4, 2015, 7:11 a.m., Steve Reinhardt wrote: > > src/mem/cache/cache.cc, line 1154 > > <http://reviews.gem5.org/r/3158/diff/2/?file=51191#file51191line1154> > > > > minor, but I think this would be more readable with parens around the > > condition, and with the RHS actually to the right of the '=', e.g.: > > > > tempBlockWriteback = (blk->isDirty() || writebackClean) ? > > writebackBlk(blk) : cleanEvictBlk(blk); > > > > or if not that, then perhaps > > > > bool do_wb = blk->isDirty() || writebackClean; > > tempBlockWriteback = do_wb ? ritebackBlk(blk) : > > cleanEvictBlk(blk); Sure. Will do. > On Nov. 4, 2015, 7:11 a.m., Steve Reinhardt wrote: > > src/mem/packet.cc, line 93 > > <http://reviews.gem5.org/r/3158/diff/2/?file=51194#file51194line93> > > > > do we really want NeedsExclusive set here? I think we will be able to get away without, but this is the exclusive copy in the memory system, and if we see something that is going to change that we drop it. Any particular aversions? - Andreas ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/3158/#review7474 ----------------------------------------------------------- On Nov. 2, 2015, 10:44 a.m., Andreas Hansson wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/3158/ > ----------------------------------------------------------- > > (Updated Nov. 2, 2015, 10:44 a.m.) > > > Review request for Default. > > > Repository: gem5 > > > Description > ------- > > Changeset 11192:22b5a5b7c5a3 > --------------------------- > mem: Add an option to perform clean writebacks from caches > > This patch adds the necessary commands and cache functionality to > allow clean writebacks. This functionality is crucial, especially when > having exclusive (victim) caches. For example, if read-only L1 > instruction caches are not sending clean writebacks, there will never > be any spills from the L1 to the L2. At the moment the cache model > defaults to not sending clean writebacks, and this should possibly be > re-evaluated. > > The implementation of clean writebacks relies on a new packet command > WritebackClean, which acts much like a Writeback (renamed > WritebackDirty), and also much like a CleanEvict. On eviction of a > clean block the cache either sends a clean evict, or a clean > writeback, and if any copies are still cached upstream the clean > evict/writeback is dropped. Similarly, if a clean evict/writeback > reaches a cache where there are outstanding MSHRs for the block, the > packet is dropped. In the typical case though, the clean writeback > allocates a block in the downstream cache, and marks it writable if > the evicted block was writable. > > The patch changes the O3_ARM_v7a L1 cache configuration and the > default L1 caches in config/common/Caches.py > > > Diffs > ----- > > configs/common/Caches.py 4daf60db14d7 > configs/common/O3_ARM_v7a.py 4daf60db14d7 > src/mem/abstract_mem.cc 4daf60db14d7 > src/mem/cache/Cache.py 4daf60db14d7 > src/mem/cache/base.hh 4daf60db14d7 > src/mem/cache/cache.hh 4daf60db14d7 > src/mem/cache/cache.cc 4daf60db14d7 > src/mem/coherent_xbar.cc 4daf60db14d7 > src/mem/packet.hh 4daf60db14d7 > src/mem/packet.cc 4daf60db14d7 > src/mem/snoop_filter.cc 4daf60db14d7 > > Diff: http://reviews.gem5.org/r/3158/diff/ > > > Testing > ------- > > > Thanks, > > Andreas Hansson > > _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
