----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/3156/#review7473 -----------------------------------------------------------
src/mem/cache/cache.hh (line 225) <http://reviews.gem5.org/r/3156/#comment6328> please add 'Atomic' to the end of this function name so we know it's just for atomic mode src/mem/cache/cache.hh (line 291) <http://reviews.gem5.org/r/3156/#comment6331> it would be much more in the style of Packet to add an AlwaysAlloc value to Packet::Attribute (and a corresponding alwaysAlloc() method to test it), then add that attribute to the appropriate commands in MemCmd::commandInfo[] in packet.cc. Then this test would just be clusivity == Enums::mostly_incl || pkt->alwaysAlloc() That would also make it clear that the decision needs to be made for each command type; with lists like this it's always easy to think you have it covered but leave one out. src/mem/cache/cache.hh (line 294) <http://reviews.gem5.org/r/3156/#comment6330> why not MemCmd::Writeback? src/mem/cache/cache.hh (line 295) <http://reviews.gem5.org/r/3156/#comment6329> doesn't ReadReq include most reads? seems like mostly_excl would not be mostly exclusive then. src/mem/cache/cache.cc (line 207) <http://reviews.gem5.org/r/3156/#comment6327> with these three lines showing up repeatedly, might be worth defining a helper function on Cache, e.g., Cache::invalidate(CacheBlk *) src/mem/cache/cache.cc (line 275) <http://reviews.gem5.org/r/3156/#comment6326> so is it the case that tempBlock could never have been allocated for a writable block before? I don't see anything that prevents that. Did we always just get lucky? src/mem/cache/cache.cc (line 1097) <http://reviews.gem5.org/r/3156/#comment6325> I'm confused as to why the writeback of tempBlock is treated differently from all the other writebacks. src/mem/cache/cache.cc (line 2352) <http://reviews.gem5.org/r/3156/#comment6324> should be '||='? Also, it seems a little odd that we wait until we're issuing the request to look at the target packets and decide whether to allocate or not. Couldn't we look at the commands as we add the targets to the MSHR? - 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/3156/ > ----------------------------------------------------------- > > (Updated Nov. 2, 2015, 2:44 a.m.) > > > Review request for Default. > > > Repository: gem5 > > > Description > ------- > > Changeset 11190:df17c209342a > --------------------------- > mem: Add cache clusivity > > This patch adds a parameter to control the cache clusivity, that is if > the cache is mostly inclusive or exclusive. At the moment there is no > intention to support strict policies, and thus the options are: 1) > mostly inclusive, or 2) mostly exclusive. > > The choice of policy guides the behaviuor on a cache fill, and a new > helper function, allocOnFill, is created to encapsulate the decision > making process. For the timing mode, the decision is annotated on the > MSHR on sending out the downstream packet, and in atomic we directly > pass the decision to handleFill. We (ab)use the tempBlock in cases > where we are not allocating on fill, leaving the rest of the cache > unaffected. Simple and effective. > > This patch also makes it more explicit that multiple caches are > allowed to consider a block writable (this is the case > also before this patch). That is, for a mostly inclusive cache, > multiple caches upstream may also consider the block exclusive. The > caches considering the block writable/exclusive all appear along the > same path to memory, and from a coherency protocol point of view it > works due to the fact that we always snoop upwards in zero time before > querying any downstream cache. > > Note that this patch does not introduce clean writebacks. Thus, for > clean lines we are essentially removing a cache level if it is made > mostly exclusive. For example, lines from the read-only L1 instruction > cache or table-walker cache are always clean, and simply get dropped > rather than being passed to the L2. If the L2 is mostly exclusive and > does not allocate on fill it will thus never hold the line. A follow > on patch adds the clean writebacks. > > The patch changes the L2 of the O3_ARM_v7a CPU configuration to be > mostly exclusive (and stats are affected accordingly). > > > Diffs > ----- > > configs/common/O3_ARM_v7a.py 4daf60db14d7 > src/mem/cache/Cache.py 4daf60db14d7 > src/mem/cache/cache.hh 4daf60db14d7 > src/mem/cache/cache.cc 4daf60db14d7 > src/mem/cache/mshr.hh 4daf60db14d7 > src/mem/cache/mshr.cc 4daf60db14d7 > > Diff: http://reviews.gem5.org/r/3156/diff/ > > > Testing > ------- > > > Thanks, > > Andreas Hansson > > _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
