> On 2011-04-13 10:28:08, Brad Beckmann wrote: > > configs/example/ruby_mem_test.py, line 97 > > <http://reviews.m5sim.org/r/611/diff/6/?file=11542#file11542line97> > > > > It seems that the following three parameters should not be hardcoded, > > but instead should be set using command line options. Hardcoding the > > atomic parameter to false still makes sense, since Ruby can handle atomic > > requests. Also we should update the comment above.
I did not add that line. I am not even aware what the purpose of that line is. I'll dig into what the implication of setting issue_dmas to false is. I will update the comment. > On 2011-04-13 10:28:08, Brad Beckmann wrote: > > src/cpu/testers/memtest/memtest.cc, line 401 > > <http://reviews.m5sim.org/r/611/diff/6/?file=11545#file11545line401> > > > > Remove commented out line Done! > On 2011-04-13 10:28:08, Brad Beckmann wrote: > > src/mem/protocol/MESI_CMP_directory-L1cache.sm, line 141 > > <http://reviews.m5sim.org/r/611/diff/6/?file=11548#file11548line141> > > > > Why are you adding this function? SLICC already generates a similar > > function: getPermission(). > > > > Overall, I hope/think we can add functional access support without > > requiring any more changes to the protocol specific .sm files beyond the > > changeset: 8086:bf0335d98250 that I checked in a couple months ago. How would you use the function that is generated by SLICC inside the sm file? I am concerned about the visibility of the function. > On 2011-04-13 10:28:08, Brad Beckmann wrote: > > src/mem/ruby/system/CacheMemory.cc, line 270 > > <http://reviews.m5sim.org/r/611/diff/6/?file=11563#file11563line270> > > > > Why are you commenting this line out? If you think it is no longer > > needed, just remove it. If we remove it, can we guarentee that the > > permission is initialized to some value? For instance, do we know whether > > the "entry" constructor will allows initialize the value? I expect the state to decide what the permission should be. > On 2011-04-13 10:28:08, Brad Beckmann wrote: > > src/mem/ruby/system/System.hh, line 132 > > <http://reviews.m5sim.org/r/611/diff/6/?file=11573#file11573line132> > > > > Why are these functions static? I'm concerned that declaring these > > static will unecessarily restrict using Ruby in multiple system simulation. > > Also from my understanding of the code, there is no need to make them > > static. Perhaps I'm missing something. My bad! I think I did that under the impression that static variables can only be referenced in static functions. > On 2011-04-13 10:28:08, Brad Beckmann wrote: > > src/mem/ruby/system/RubyPort.cc, line 302 > > <http://reviews.m5sim.org/r/611/diff/6/?file=11568#file11568line302> > > > > Overall, I really like the comments you added in this file. They > > really help clarify what is going on here. Just one > > minor correction: change "Any copy" to "Any valid copy" Done! > On 2011-04-13 10:28:08, Brad Beckmann wrote: > > src/mem/ruby/system/DirectoryMemory.cc, line 246 > > <http://reviews.m5sim.org/r/611/diff/6/?file=11565#file11565line246> > > > > Please align this statement. I am removing this line. - Nilay ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/611/#review1111 ----------------------------------------------------------- On 2011-04-12 16:35:34, Nilay Vaish wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.m5sim.org/r/611/ > ----------------------------------------------------------- > > (Updated 2011-04-12 16:35:34) > > > Review request for Default. > > > Summary > ------- > > Ruby: Add support for functional accesses > This patch is meant for aiding discussions on implementation of functional > access support in Ruby. > > > Diffs > ----- > > configs/example/ruby_mem_test.py cb1e137ac35e > configs/ruby/MESI_CMP_directory.py cb1e137ac35e > configs/ruby/Ruby.py cb1e137ac35e > src/cpu/testers/memtest/memtest.cc cb1e137ac35e > src/mem/packet.hh cb1e137ac35e > src/mem/packet.cc cb1e137ac35e > src/mem/protocol/MESI_CMP_directory-L1cache.sm cb1e137ac35e > src/mem/protocol/MESI_CMP_directory-L2cache.sm cb1e137ac35e > src/mem/protocol/MESI_CMP_directory-dir.sm cb1e137ac35e > src/mem/protocol/RubySlicc_Types.sm cb1e137ac35e > src/mem/ruby/network/Network.cc cb1e137ac35e > src/mem/ruby/network/Network.py cb1e137ac35e > src/mem/ruby/profiler/Profiler.cc cb1e137ac35e > src/mem/ruby/profiler/Profiler.py cb1e137ac35e > src/mem/ruby/recorder/Tracer.cc cb1e137ac35e > src/mem/ruby/recorder/Tracer.py cb1e137ac35e > src/mem/ruby/system/AbstractMemory.hh PRE-CREATION > src/mem/ruby/system/AbstractMemory.cc PRE-CREATION > src/mem/ruby/system/AbstractMemory.py PRE-CREATION > src/mem/ruby/system/Cache.py cb1e137ac35e > src/mem/ruby/system/CacheMemory.hh cb1e137ac35e > src/mem/ruby/system/CacheMemory.cc cb1e137ac35e > src/mem/ruby/system/DirectoryMemory.hh cb1e137ac35e > src/mem/ruby/system/DirectoryMemory.cc cb1e137ac35e > src/mem/ruby/system/DirectoryMemory.py cb1e137ac35e > src/mem/ruby/system/RubyPort.hh cb1e137ac35e > src/mem/ruby/system/RubyPort.cc cb1e137ac35e > src/mem/ruby/system/RubySystem.py cb1e137ac35e > src/mem/ruby/system/SConscript cb1e137ac35e > src/mem/ruby/system/Sequencer.cc cb1e137ac35e > src/mem/ruby/system/Sequencer.py cb1e137ac35e > src/mem/ruby/system/System.hh cb1e137ac35e > src/mem/ruby/system/System.cc cb1e137ac35e > src/mem/slicc/ast/MemberExprAST.py cb1e137ac35e > > Diff: http://reviews.m5sim.org/r/611/diff > > > Testing > ------- > > I have tested functional accesses with the ratio between functional > and timing accesses for different ratios -- 100:0, 99:1, 90:1, 50:50, > 10:90, 1:99. It is working in all the cases. > > > Thanks, > > Nilay > > _______________________________________________ m5-dev mailing list m5-dev@m5sim.org http://m5sim.org/mailman/listinfo/m5-dev
