What's going on with this patch? I don't believe it's been committed but it seems like it should. I've also got some patches waiting behind this because they used to touch CacheMsg and I don't want to mess Nilay up, so I've been waiting to serialize behind this.
Lisa On Wed, Feb 9, 2011 at 1:28 PM, Brad Beckmann <brad.beckm...@amd.com> wrote: > This is an automatically generated e-mail. To reply, visit: > http://reviews.m5sim.org/r/327/ > > 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/diff/6/?file=10042#file10042line183> > (Diff > revision 6) > > StoreBuffer::checkForLoadHit(RubyRequest request) > > 183 > > if ((req.paddr & m_block_mask) != lineaddr) > > 183 > > if ((req.m_PhysicalAddress.getAddress() & m_block_mask) != lineaddr) > > 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/diff/6/?file=10042#file10042line233> > (Diff > revision 6) > > StoreBuffer::returnMatchedData(RubyRequest request) > > 233 > > if ((it->m_request.paddr & m_block_mask) == lineaddr) { > > 233 > > if ((it->m_request.m_PhysicalAddress.getAddress() & m_block_mask) == > lineaddr) { > > Same here. Can we just use the m_LineAddress? > > > > src/mem/ruby/storebuffer/storebuffer.cc<http://reviews.m5sim.org/r/327/diff/6/?file=10042#file10042line308> > (Diff > revision 6) > > StoreBuffer::complete(uint64_t id) > > 308 > > if ((from_buffer.m_request.paddr & m_block_mask) == lineaddr && > > 308 > > if ((from_buffer.m_request.m_PhysicalAddress.getAddress() & > m_block_mask) == lineaddr && > > And here > > > > src/mem/ruby/system/Sequencer.cc<http://reviews.m5sim.org/r/327/diff/6/?file=10051#file10051line232> > (Diff > revision 6) > > Sequencer::insertRequest(SequencerRequest* request) > > 230 > > line_addr.makeLineAddress(); > > 231 > > Address line_addr(ruby_request->m_LineAddress); > > 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/diff/6/?file=10051#file10051line455> > (Diff > revision 6) > > Sequencer::hitCallback(SequencerRequest* srequest, > > 451 > > Address request_line_address(ruby_request.paddr); > > 453 > > Address request_line_address(ruby_request->m_PhysicalAddress); > > Can you just use the m_LineAddress variable of ruby_request instead of > converting the paddr to a line address. > > > - Brad > > On January 25th, 2011, 9:15 a.m., Nilay Vaish wrote: > Review request for Default. > By Nilay Vaish. > > *Updated 2011-01-25 09:15:23* > Description > > 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) > > View Diff <http://reviews.m5sim.org/r/327/diff/> > > _______________________________________________ > m5-dev mailing list > m5-dev@m5sim.org > http://m5sim.org/mailman/listinfo/m5-dev > > _______________________________________________ m5-dev mailing list m5-dev@m5sim.org http://m5sim.org/mailman/listinfo/m5-dev
