----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/327/#review841 -----------------------------------------------------------
Overall, this patch looks good to me as well. I just have a couple minor questions. Though your comment says this patch removes libruby, the diff seems to indicate that the file still remains. Am I reading that correctly? Also it seems that in several places you unecessarily generate the line address, even though the line address already exists in the ruby request (see comments below). src/mem/ruby/storebuffer/storebuffer.cc <http://reviews.m5sim.org/r/327/#comment1201> Instead of masking the physical address, can we just use the m_LineAddress? src/mem/ruby/storebuffer/storebuffer.cc <http://reviews.m5sim.org/r/327/#comment1202> Same here. Can we just use the m_LineAddress? src/mem/ruby/storebuffer/storebuffer.cc <http://reviews.m5sim.org/r/327/#comment1203> And here src/mem/ruby/system/Sequencer.cc <http://reviews.m5sim.org/r/327/#comment1199> Why is this line address conversion necessary? Isn't m_LineAddress already set correctly in the constructor? src/mem/ruby/system/Sequencer.cc <http://reviews.m5sim.org/r/327/#comment1200> Can you just use the m_LineAddress variable of ruby_request instead of converting the paddr to a line address. - Brad On 2011-01-25 09:15:23, Nilay Vaish wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.m5sim.org/r/327/ > ----------------------------------------------------------- > > (Updated 2011-01-25 09:15:23) > > > Review request for Default. > > > Summary > ------- > > Remove CacheMsg class from SLICC > The goal of the patch is to do away with the CacheMsg class currently in use > in coherence protocols. In place of CacheMsg, the RubyRequest class will used. > This class is already present in libruby.hh. In fact, objects of class > CacheMsg are generated by copying values from a RubyRequest object. > > The changes relating to removal of libruby have been moved to separate patch. > > > Diffs > ----- > > src/cpu/testers/rubytest/RubyTester.hh 31a04e5ac4be > src/mem/protocol/MESI_CMP_directory-L1cache.sm 31a04e5ac4be > src/mem/protocol/MI_example-cache.sm 31a04e5ac4be > src/mem/protocol/MOESI_CMP_directory-L1cache.sm 31a04e5ac4be > src/mem/protocol/MOESI_CMP_token-L1cache.sm 31a04e5ac4be > src/mem/protocol/MOESI_CMP_token-dir.sm 31a04e5ac4be > src/mem/protocol/MOESI_hammer-cache.sm 31a04e5ac4be > src/mem/protocol/RubySlicc_Exports.sm 31a04e5ac4be > src/mem/protocol/RubySlicc_Profiler.sm 31a04e5ac4be > src/mem/protocol/RubySlicc_Types.sm 31a04e5ac4be > src/mem/ruby/SConscript 31a04e5ac4be > src/mem/ruby/common/Address.hh 31a04e5ac4be > src/mem/ruby/common/Address.cc 31a04e5ac4be > src/mem/ruby/common/DataBlock.hh 31a04e5ac4be > src/mem/ruby/common/DataBlock.cc 31a04e5ac4be > src/mem/ruby/filters/BlockBloomFilter.cc 31a04e5ac4be > src/mem/ruby/filters/BulkBloomFilter.cc 31a04e5ac4be > src/mem/ruby/filters/LSB_CountingBloomFilter.cc 31a04e5ac4be > src/mem/ruby/filters/MultiGrainBloomFilter.cc 31a04e5ac4be > src/mem/ruby/filters/NonCountingBloomFilter.cc 31a04e5ac4be > src/mem/ruby/libruby.hh 31a04e5ac4be > src/mem/ruby/libruby.cc 31a04e5ac4be > src/mem/ruby/libruby_internal.hh 31a04e5ac4be > src/mem/ruby/profiler/AccessTraceForAddress.cc 31a04e5ac4be > src/mem/ruby/profiler/AddressProfiler.hh 31a04e5ac4be > src/mem/ruby/profiler/AddressProfiler.cc 31a04e5ac4be > src/mem/ruby/profiler/Profiler.hh 31a04e5ac4be > src/mem/ruby/profiler/Profiler.cc 31a04e5ac4be > src/mem/ruby/recorder/CacheRecorder.hh 31a04e5ac4be > src/mem/ruby/recorder/CacheRecorder.cc 31a04e5ac4be > src/mem/ruby/recorder/TraceRecord.hh 31a04e5ac4be > src/mem/ruby/recorder/TraceRecord.cc 31a04e5ac4be > src/mem/ruby/recorder/Tracer.hh 31a04e5ac4be > src/mem/ruby/recorder/Tracer.cc 31a04e5ac4be > src/mem/ruby/slicc_interface/RubyRequest.hh PRE-CREATION > src/mem/ruby/slicc_interface/RubyRequest.cc PRE-CREATION > src/mem/ruby/slicc_interface/RubySlicc_Profiler_interface.hh 31a04e5ac4be > src/mem/ruby/slicc_interface/RubySlicc_Util.hh 31a04e5ac4be > src/mem/ruby/slicc_interface/SConscript 31a04e5ac4be > src/mem/ruby/storebuffer/stb_interface.cc 31a04e5ac4be > src/mem/ruby/storebuffer/storebuffer.cc 31a04e5ac4be > src/mem/ruby/system/CacheMemory.hh 31a04e5ac4be > src/mem/ruby/system/CacheMemory.cc 31a04e5ac4be > src/mem/ruby/system/DMASequencer.hh 31a04e5ac4be > src/mem/ruby/system/DMASequencer.cc 31a04e5ac4be > src/mem/ruby/system/PerfectCacheMemory.hh 31a04e5ac4be > src/mem/ruby/system/RubyPort.hh 31a04e5ac4be > src/mem/ruby/system/RubyPort.cc 31a04e5ac4be > src/mem/ruby/system/Sequencer.hh 31a04e5ac4be > src/mem/ruby/system/Sequencer.cc 31a04e5ac4be > src/mem/ruby/system/SparseMemory.cc 31a04e5ac4be > src/mem/slicc/parser.py 31a04e5ac4be > > Diff: http://reviews.m5sim.org/r/327/diff > > > Testing > ------- > > > Thanks, > > Nilay > >
_______________________________________________ m5-dev mailing list m5-dev@m5sim.org http://m5sim.org/mailman/listinfo/m5-dev
