----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/3158/#review7474 -----------------------------------------------------------
src/mem/cache/cache.cc (line 1154) <http://reviews.gem5.org/r/3158/#comment6332> 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); src/mem/packet.hh (line 148) <http://reviews.gem5.org/r/3158/#comment6334> 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 src/mem/packet.cc (line 93) <http://reviews.gem5.org/r/3158/#comment6333> do we really want NeedsExclusive set here? - Steve Reinhardt On Nov. 2, 2015, 2: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, 2: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
