----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/827/#review1461 -----------------------------------------------------------
Other than fixing one comment, this patch looks good to me. By the way, have you tested an existing directory protocol with the memtester using functional accesses? I can't remember if the regression tester turns on functional accesses for the ruby-memtester, and I suspect that even if it does, it may not run long enough to catch all bugs. I just want to make sure that nothing breaks with this change, though I don't anticipate a problem. src/mem/ruby/system/RubyPort.cc <http://reviews.m5sim.org/r/827/#comment1881> Could you clarify this comment a bit. What is the second if/else statement you are referring to? Also I don't believe you can say that if you reach the else statement, that the block exists in the cache hierarchy. For directory protocols, one could reach the else statement with memory containing the only valid copy. src/mem/ruby/system/RubyPort.cc <http://reviews.m5sim.org/r/827/#comment1882> Thanks for adding the spacing! Much easier to read. - Brad On 2011-08-15 15:29:06, Lisa Hsu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.m5sim.org/r/827/ > ----------------------------------------------------------- > > (Updated 2011-08-15 15:29:06) > > > Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and > Nathan Binkert. > > > Summary > ------- > > Functional Accesses: Update states to support Broadcast/Snooping protocols. > > In the current implementation of Functional Accesses, it's very hard to > implement broadcast or snooping protocols where the memory has no idea if it > has exclusive access to a cache block or not. Without this knowledge, making > sure the RW vs. RO permissions are right are next to impossible. So we add a > new state called Backing_Store to enable the conveyance that this is the > backup storage for a block, so that it can be written if it is the only > possibly RW block in the system, or written even if there is another RW block > in the ssytem. > > > Diffs > ----- > > src/mem/protocol/RubySlicc_Exports.sm 4a6c166f61f7 > src/mem/ruby/system/RubyPort.cc 4a6c166f61f7 > src/mem/slicc/symbols/StateMachine.py 4a6c166f61f7 > > Diff: http://reviews.m5sim.org/r/827/diff > > > Testing > ------- > > Tested on a protocol on 10 random seeds to 1M accesses using ruby_mem_test.py > with 25% functional accesses. > > > Thanks, > > Lisa > > _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
