----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/2167/#review5218 -----------------------------------------------------------
src/mem/cache/tags/base_set_assoc.cc <http://reviews.gem5.org/r/2167/#comment4757> It seems sensible to make it an unsigned param and only check for 0. src/mem/cache/tags/base_set_assoc.cc <http://reviews.gem5.org/r/2167/#comment4758> Same here, instead of an Int param it should probably be Unsigned. src/mem/cache/tags/base_set_assoc.cc <http://reviews.gem5.org/r/2167/#comment4759> Is Cycles, so it can never be < 0 src/mem/cache/tags/base_set_assoc.cc <http://reviews.gem5.org/r/2167/#comment4760> A bit pedantic perhaps, but (slightly stupid) static code analysers get unhappy with the member variables not being initialised in the initialisation list. Could we add a default value, or move the assignment up to the initialisation list? Some style-related suggestions, for the rest it looks great. Feel free to punt this to a later patch. - Andreas Hansson On July 21, 2014, 6:45 p.m., Anthony Gutierrez wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/2167/ > ----------------------------------------------------------- > > (Updated July 21, 2014, 6:45 p.m.) > > > Review request for Default. > > > Repository: gem5 > > > Description > ------- > > this patch implements a new tags class that uses a random replacement policy. > these tags prefer to evict invalid blocks first, if none are available a > replacement candidate is chosen at random. > > this patch factors out the common code in the LRU class and creates a new > abstract class: the BaseSetAssoc class. any set associative tag class must > implement the functionality related to the actual replacement policy in the > following methods: > > accessBlock() > findVictim() > insertBlock() > invalidate() > > > Diffs > ----- > > src/mem/cache/base.cc 23384aa97d8533f6e3f812d015dccaab3c0267af > src/mem/cache/cache.cc 23384aa97d8533f6e3f812d015dccaab3c0267af > src/mem/cache/tags/SConscript 23384aa97d8533f6e3f812d015dccaab3c0267af > src/mem/cache/tags/Tags.py 23384aa97d8533f6e3f812d015dccaab3c0267af > src/mem/cache/tags/base_set_assoc.hh PRE-CREATION > src/mem/cache/tags/base_set_assoc.cc PRE-CREATION > src/mem/cache/tags/lru.hh 23384aa97d8533f6e3f812d015dccaab3c0267af > src/mem/cache/tags/lru.cc 23384aa97d8533f6e3f812d015dccaab3c0267af > src/mem/cache/tags/random_repl.hh PRE-CREATION > src/mem/cache/tags/random_repl.cc PRE-CREATION > > Diff: http://reviews.gem5.org/r/2167/diff/ > > > Testing > ------- > > Regressions pass > > > Thanks, > > Anthony Gutierrez > > _______________________________________________ gem5-dev mailing list gem5-dev@gem5.org http://m5sim.org/mailman/listinfo/gem5-dev
