> On May 12, 2015, 9:24 p.m., Jason Power wrote: > > src/mem/ruby/structures/CacheMemory.hh, line 137 > > <http://reviews.gem5.org/r/2802/diff/1/?file=45052#file45052line137> > > > > Can you add a comment here describing what this function does? I don't > > find it self explanatory from the function name/parameters. > > > > E.g., what is the "idx" parameter? What is the min/max value (0 to > > getNumBlocks()-1, I assume)? Also, could you specify the mapping from idx > > to way/set (in the comment)? It's in the code, but the code is not easily > > parsable, at least to me. > > > > Also, are you really getting the index, or are you getting the address > > at a specific index? Maybe a more precise name is needed as well. > > Brad Beckmann wrote: > I added the following comment to the function. Does this work for you: > > // Given an unique cache block identifier (idx): return the valid address > // stored by the cache block. If the block is invalid/notpresent, the > // function returns the 0 address > > Jason Power wrote: > Sorry to be picky here, but I still don't quite understand. Is a cache > block identifier a common thing? I personally don't know what it is based on > just that phrase. Is each block in the cache assigned a number from 0 to > number of blocks? Which block is 0, which is 1, or does this not matter for > some reason? From what I can tell you're taking what's usually refered to as > sets/ways and linearizing it. If you could explain how it's linearized it > would make this clear. > > Brad Beckmann wrote: > This feature is not meant to be complicated. Yes, the input idx > parameter is just an integer between 0 and the number of blocks. The > function returns the tag at that "linearized" location in the cache. > > There is no special logic to "linearize" the cache. The way and set is > simply determined using the arithmetic implemented in the function. There > really is no magic going on outside this function. > > Nilay Vaish wrote: > I think the function should be renamed since it is not returning the tag > but the address. > How about getAddressAtIdx()?
Sure. The unique information provided by the function is a tag, but renaming it Address better describes the returned data structure. This did not make it into the patch that Tony posted a few days ago, but it will be updated in the next posted round of patches. - Brad ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/2802/#review6209 ----------------------------------------------------------- On May 15, 2015, 8:12 p.m., Tony Gutierrez wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/2802/ > ----------------------------------------------------------- > > (Updated May 15, 2015, 8:12 p.m.) > > > Review request for Default. > > > Repository: gem5 > > > Description > ------- > > Changeset 10863:a19dfa9c5391 > --------------------------- > ruby: speed up function used for cache walks > > This patch adds a few helpful functions that allow .sm files to directly > invalidate all cache blocks using a trigger queue rather than rely on each > individual cache block to be invalidated via requests from the mandatory > queue. > > > Diffs > ----- > > src/mem/ruby/structures/CacheMemory.cc > 9b424e7adac570f9837cde60d619b6ec3c211854 > src/mem/protocol/RubySlicc_Types.sm > 9b424e7adac570f9837cde60d619b6ec3c211854 > src/mem/ruby/structures/CacheMemory.hh > 9b424e7adac570f9837cde60d619b6ec3c211854 > > Diff: http://reviews.gem5.org/r/2802/diff/ > > > Testing > ------- > > > Thanks, > > Tony Gutierrez > > _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
