> On April 29, 2015, 9:59 a.m., Steve Reinhardt wrote: > > src/mem/cache/blk.hh, line 424 > > <http://reviews.gem5.org/r/2711/diff/3/?file=44635#file44635line424> > > > > I don't think this needs to be a template anymore, does it? It's only > > instantiated once with <Cache> as the parameter. I suggest getting rid of > > the template and also the WrappedBlkVisitor typedef below. The new class > > could use either name. > > Andreas Hansson wrote: > Not possible since the block cannot include the Cache. The template > parameter is really just there to break the circular dependency.
I see. But since the definition only uses 'CacheBlk &', there isn't a dependency on the full CacheBlk definition, so I think the circular dependency could be avoided without the template just by moving the definition to cache.hh, right? I see where that leads to some follow-on issues, since you need the CacheBlkVisitor base class, but so does CacheBlkIsDirtyVisitor, and as it stands the latter does rely on the CacheBlk definition. However, there's really no need for CacheBlkIsDirtyVisitor::operator() to be defined in the header... you could move that method (or in fact the whole CacheBlkIsDirtyVisitor class declaration) into cache_impl.hh right above the isDirty() method. Then you could move both CacheBlkVisitor and the untemplated CacheBlkVisitorWrapper/WrappedBlkVisitor into cache.hh. Not trying to make your life difficult, honest... if you want to defer all this to some future patch that's fine with me. But it would be nice to make it happen, since the template is conceptually unnecessary. - Steve ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/2711/#review6095 ----------------------------------------------------------- On April 29, 2015, 10:04 a.m., Andreas Hansson wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/2711/ > ----------------------------------------------------------- > > (Updated April 29, 2015, 10:04 a.m.) > > > Review request for Default. > > > Repository: gem5 > > > Description > ------- > > Changeset 10805:9b1222365250 > --------------------------- > mem: Remove templates in cache model > > This patch changes the cache implementation to rely on virtual methods > rather than using the replacement policy as a template argument. > > There is no impact on the simulation performance, and overall the > changes make it easier to modify (and subclass) the cache and/or > replacement policy. > > > Diffs > ----- > > src/mem/cache/tags/lru.cc df2aa91dba5b > src/mem/cache/tags/random_repl.hh df2aa91dba5b > src/mem/cache/tags/random_repl.cc df2aa91dba5b > src/mem/cache/base.cc df2aa91dba5b > src/mem/cache/blk.hh df2aa91dba5b > src/mem/cache/cache.hh df2aa91dba5b > src/mem/cache/tags/base_set_assoc.cc df2aa91dba5b > src/mem/cache/tags/fa_lru.hh df2aa91dba5b > src/mem/cache/tags/fa_lru.cc df2aa91dba5b > src/mem/cache/tags/lru.hh df2aa91dba5b > src/mem/cache/tags/base.hh df2aa91dba5b > src/mem/cache/tags/base_set_assoc.hh df2aa91dba5b > src/mem/cache/cache.cc df2aa91dba5b > src/mem/cache/cache_impl.hh df2aa91dba5b > > Diff: http://reviews.gem5.org/r/2711/diff/ > > > Testing > ------- > > > Thanks, > > Andreas Hansson > > _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
