> On June 21, 2013, 6:33 p.m., Steve Reinhardt wrote: > > In general this looks like a positive direction, but I'm concerned about > > keeping the python tag class and the cache template instance in sync. For > > example, it looks to me like if I specify a fully associative cache (via > > the cache's assoc parameter), but pass in an LRU tag SimObject, then the > > dynamic_cast in the tag initializer of the Cache constructor will fail, and > > I'll get a segfault in the constructor at 'tags->setCache(this);'. Not > > very user-friendly at best! > > > > One possibility would be to use dynamic_casts in BaseCacheParams::create() > > to figure out the type of the tags object and use the right Cache template > > instance accordingly. You lose the magic of letting the system pick the > > best tag instance for you (in the case where that makes sense) but at least > > you avoid the potential for a mismatch between the two types. > > > > Another possibility is to change the python perspective from there being > > two independent classes (Cache and LRU/FALRU) to an inheritance model, > > where you instantiate LRUCache or FALRUCache in python, and those map to > > Cache<LRU> or Cache<FALRU> in the C++. Then the python tag params become > > additional params on the derived Cache object.
Good point. I'm inclined to go with the inheritance (LRUCache etc in Python). Anyone else got any preference? - Andreas ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/1931/#review4455 ----------------------------------------------------------- On June 20, 2013, 5:47 p.m., Andreas Hansson wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/1931/ > ----------------------------------------------------------- > > (Updated June 20, 2013, 5:47 p.m.) > > > Review request for Default. > > > Repository: gem5 > > > Description > ------- > > Changeset 9791:0946f340b959 > --------------------------- > mem: Reorganize cache tags and make them a SimObject > > This patch reorganizes the cache tags to allow more flexibility to > implement new replacement policies. The base tags class is now a > clocked object so that derived classes can use a clock if they need > one. Also having deriving from SimObject allows specialized Tag > classes to be swapped in/out in .py files. > > The cache set is now templatized to allow it to contain customized > cache blocks with additional informaiton. This involved moving code to > the .hh file and removing cacheset.cc. > > The statistics belonging to the cache tags are now including ".tags" > in their name. Hence, the stats need an update to reflect the change > in naming. > > Note that this follows on: http://reviews.gem5.org/r/1875/ > > > Diffs > ----- > > src/mem/cache/tags/lru.cc ff17ab994003 > src/mem/cache/tags/lru.hh ff17ab994003 > src/mem/cache/tags/fa_lru.cc ff17ab994003 > src/mem/cache/tags/fa_lru.hh ff17ab994003 > src/mem/cache/tags/cacheset.cc ff17ab994003 > src/mem/cache/tags/cacheset.hh ff17ab994003 > src/mem/cache/tags/base.cc ff17ab994003 > src/mem/cache/tags/base.hh ff17ab994003 > src/mem/cache/tags/Tags.py PRE-CREATION > src/mem/cache/tags/SConscript ff17ab994003 > src/mem/cache/cache_impl.hh ff17ab994003 > src/mem/cache/cache.hh ff17ab994003 > src/mem/cache/base.cc ff17ab994003 > src/mem/cache/BaseCache.py ff17ab994003 > > Diff: http://reviews.gem5.org/r/1931/diff/ > > > Testing > ------- > > After some sedding of the stats (for tag stats include .tags in the name) all > regressions pass > > > Thanks, > > Andreas Hansson > > _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
