> On 2011-01-03 10:14:44, Steve Reinhardt wrote: > > src/mem/protocol/MOESI_CMP_token-L1cache.sm, line 972 > > <http://reviews.m5sim.org/r/326/diff/2/?file=8135#file8135line972> > > > > These functions that take multiple pointers just so the called function > > can test which one is valid are pretty unwieldy and also seem to cause a > > lot of redundant testing. Can we factor out the check so we have a single > > test to set one valid entry_ptr variable, then use that in all these calls? > > I see there are some similar calls in MOESI_CMP_directory too, though not > > as many. > > Nilay Vaish wrote: > You should check my comment in the MOESI Hammer request why we cannot set > just one valid entry pointer. I am of the view that we should generate the > arguments for getCacheEntry() internally in SLICC. > > Steve Reinhardt wrote: > I understand that you can't universally assume that there's just one > valid pointer... I'm not asking to go all the way back to that. But it > appears to me that there are a lot of places (regions of code) where you know > that only one of N pointers is valid, yet you have to repeatedly pass all N > pointers to each low-level function to figure out which one to use. It would > be both more efficient and easier to read if, in those regions, there was one > function up front that identified and selected the one valid pointer (maybe > sticking it in a local variable), then the low-level functions were all > passed that one valid pointer. Auto-generating params in SLICC would solve > the readability problem but not the efficiency problem.
This is possible with a local variable. In fact I was thinking of letting SLICC generate such a local variable in each action. But then I decided I would let this issue be taken up during the review process. I can make this change. The problem is I am not sure if the currently proposed changes would be acceptable to some one (read Brad) who has used SLICC extensively. - Nilay ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/326/#review588 ----------------------------------------------------------- On 2010-12-31 17:33:04, Nilay Vaish wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.m5sim.org/r/326/ > ----------------------------------------------------------- > > (Updated 2010-12-31 17:33:04) > > > Review request for Default. > > > Summary > ------- > > This request for reviewing the updates to implementation of the MOESI CMP > token protocol. These updates have been carried out so as to conform with the > changes made to CacheMemory and TBETable classes, and to SLICC. > > > Diffs > ----- > > src/mem/protocol/MOESI_CMP_token-L1cache.sm UNKNOWN > src/mem/protocol/MOESI_CMP_token-L2cache.sm UNKNOWN > src/mem/protocol/MOESI_CMP_token-dir.sm UNKNOWN > > Diff: http://reviews.m5sim.org/r/326/diff > > > Testing > ------- > > Changes have been tested using ruby_random_test.py for 1,000,000 loads and 20 > different seeds for random number generator. > > > Thanks, > > Nilay > >
_______________________________________________ m5-dev mailing list m5-dev@m5sim.org http://m5sim.org/mailman/listinfo/m5-dev
