> 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 > > Andreas Hansson wrote: > I would prefer the addition of "IsEviction" to be in a separate follow-on > patch. > > Steve Reinhardt wrote: > Any reason? Since you're just introducing IsWriteback in this patch, if > we're going to replace it with IsEviction, seems preferable to me not to > introduce IsWriteback in the first place. > > It would be nice to have a Packet::isCleanEviction() method regardless, I > think. > > Andreas Hansson wrote: > I guess I'm just not a massive fan of the attributes, but I'll give your > suggestion a spin. Note that in the updated patch neither WritebackDirty, nor > WritebackClean has NeedsExclusive set.
The flags are added/changed, but I must confess I'm not a massive fan of the "isCleanEviction" method as I find it confusing. The name suggests it is only checking if the cmd == CleanEvict (like the other similarly named methods), but it also includes cmd == WritebackClean. Suggestions? - Andreas ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/3158/#review7474 ----------------------------------------------------------- On Nov. 4, 2015, 5:53 p.m., Andreas Hansson wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/3158/ > ----------------------------------------------------------- > > (Updated Nov. 4, 2015, 5:53 p.m.) > > > Review request for Default. > > > Repository: gem5 > > > Description > ------- > > Changeset 11198:7445f17936f5 > --------------------------- > 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 > ----- > > src/mem/coherent_xbar.cc 2d1d51615e0e > src/mem/packet.hh 2d1d51615e0e > src/mem/packet.cc 2d1d51615e0e > src/mem/snoop_filter.cc 2d1d51615e0e > src/mem/cache/Cache.py 2d1d51615e0e > src/mem/cache/base.hh 2d1d51615e0e > src/mem/cache/cache.hh 2d1d51615e0e > src/mem/cache/cache.cc 2d1d51615e0e > configs/common/Caches.py 2d1d51615e0e > configs/common/O3_ARM_v7a.py 2d1d51615e0e > src/mem/abstract_mem.cc 2d1d51615e0e > > 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
