> On 2011-08-16 22:47:57, Brad Beckmann wrote: > > 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.
I haven't, though it occurred to me last night. I'm planning to do a few quick tests, but like you I don't anticipate problems (famous last words). :) > On 2011-08-16 22:47:57, Brad Beckmann wrote: > > src/mem/ruby/system/RubyPort.cc, line 369 > > <http://reviews.m5sim.org/r/827/diff/1/?file=14659#file14659line369> > > > > 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. I suppose the comment isn't clear. It's not meant to mean that there is a *valid* copy in the cache hierarchy, just that it exists in some form, since the if statement should cover the case where it doesn't exist at all (is Invalid or Not Present) throughout all cache controllers but exists in memory. I also didn't mean there's a second if/else, just the two cases separated by the if/else. The two cases (if and else) are separated because if you always consider memory a readable block, then in the case where it is stale and the loop gets to memory first, you don't want it to be the read value, but in the case that it doesn't exist in any of the cache controllers, you definitely want to return the block in memory. In other words, you only want to return the value in memory sometimes, so you have to separate it into the if/else, which should not be collapsed into one large statement. I'm not sure if I'm answering your question - maybe you can drop by the cube if I'm missing your point. - Lisa ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/827/#review1461 ----------------------------------------------------------- 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
