> On July 12, 2015, 3:29 p.m., Joel Hestness wrote: > > src/mem/ruby/system/Sequencer.cc, line 323 > > <http://reviews.gem5.org/r/2953/diff/1/?file=47948#file47948line323> > > > > To keep things clean, could we also remove the clearLocked, isLocked, > > and setLocked functions from the CacheMemory, since they are only used > > where you've changed code? > > Nilay Vaish wrote: > Somebody might be using them. It does not hurt us at all to leave them > in place. > > Joel Hestness wrote: > Sure. In that case, could you add a comment near their declarations in > CacheMemory that the suggested functions for handling LL/SC are now in > CacheEntry?
To me, it seems dangerous to leave the functions in CacheMemory. Correct me if I'm wrong, but if you call m_dataCache_ptr->setLocked() and then e->clearLocked() you'll have a major problem. Said another way, if you combine using the cache entry and the cache memory implementations it won't work. My suggestion: Implement CacheMemory::*Locked by calling the AbstractEntry functions. - Jason ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/2953/#review6746 ----------------------------------------------------------- On July 10, 2015, 4:27 p.m., Nilay Vaish wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/2953/ > ----------------------------------------------------------- > > (Updated July 10, 2015, 4:27 p.m.) > > > Review request for Default. > > > Repository: gem5 > > > Description > ------- > > Changeset 10928:2ff7814c0484 > --------------------------- > ruby: handle llsc accesses through CacheEntry, not CacheMemory > > The sequencer takes care of llsc accesses by calling upon functions > from the CacheMemory. This is unnecessary once the required CacheEntry object > is available. Thus some of the calls to findTagInSet() are avoided. > > > Diffs > ----- > > src/mem/ruby/slicc_interface/AbstractCacheEntry.hh 5c76426fd9ee > src/mem/ruby/system/Sequencer.cc 5c76426fd9ee > > Diff: http://reviews.gem5.org/r/2953/diff/ > > > Testing > ------- > > > Thanks, > > Nilay Vaish > > _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
