----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/1443/#review3561 -----------------------------------------------------------
src/mem/protocol/MESI_CMP_directory-L1cache.sm <http://reviews.gem5.org/r/1443/#comment3551> Very minor...the others just have a one-liner return getCacheEntry(addr).DataBlk; Perhaps stick to the same? src/mem/protocol/MESI_CMP_directory-msg.sm <http://reviews.gem5.org/r/1443/#comment3552> A comment on what (or what is not) a coherence request of type PUTX? src/mem/protocol/MESI_CMP_directory-msg.sm <http://reviews.gem5.org/r/1443/#comment3553> Why is it not symmetric with the reads? src/mem/protocol/MESI_CMP_directory-msg.sm <http://reviews.gem5.org/r/1443/#comment3554> Same as previously, a short comment on why Data dataExclusive and memory data (what is data/mem data difference?). src/mem/protocol/MESI_CMP_directory-msg.sm <http://reviews.gem5.org/r/1443/#comment3555> Again, why the asymmetry with reads? src/mem/ruby/buffers/MessageBuffer.hh <http://reviews.gem5.org/r/1443/#comment3556> Why a pointer reference? src/mem/ruby/buffers/MessageBuffer.hh <http://reviews.gem5.org/r/1443/#comment3557> a const pointer ref? Why? Is it not possible that the write turns the packet into a response? src/mem/ruby/buffers/MessageBuffer.cc <http://reviews.gem5.org/r/1443/#comment3558> Could this be iterators as well? Feels a bit off to mix counter based and iterator based iteration. src/mem/ruby/buffers/MessageBuffer.cc <http://reviews.gem5.org/r/1443/#comment3562> Could there be order dependent behaviour here or somewhere else? The iteration over a map scares me quite a bit! In general, we should avoid it as the order is not defined. src/mem/ruby/network/Network.hh <http://reviews.gem5.org/r/1443/#comment3559> Not the same type as the other place... src/mem/ruby/network/Network.hh <http://reviews.gem5.org/r/1443/#comment3560> Not the same type (was const Packet*&) src/mem/ruby/network/simple/SimpleNetwork.cc <http://reviews.gem5.org/r/1443/#comment3561> now int and not size_t? an iterator would be even nicer to see src/mem/ruby/slicc_interface/RubyRequest.cc <http://reviews.gem5.org/r/1443/#comment3563> Just a note on the memtester...it is as far as I am concerned broken in its current implementation. The way it interacts with the shadow memory is too restrictive. The ordering model allows many more legal permutations than what we currently do by comparing with the functional memory. I'm not sure I would design for a broken memtest to pass. - Andreas Hansson On Oct. 8, 2012, 5:47 p.m., Nilay Vaish wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/1443/ > ----------------------------------------------------------- > > (Updated Oct. 8, 2012, 5:47 p.m.) > > > Review request for Default. > > > Description > ------- > > Changeset 9283:0f843ef51892 > --------------------------- > ruby: improved support for functional accesses > This patch adds support to different entities in the ruby memory system > for more reliable functional read/write accesses. Only the simple network > has been augmented as of now. Later on Garnet will also support functional > accesses. > The patch adds functional access code to all the different types of messages > that protocols can send around. These messages are functionally accessed > by going through the buffers maintained by the network entities. > The patch also rectifies some of the bugs found in coherence protocols while > testing the patch. > > With this patch applied, functional writes always succeed. But functional > reads can still fail. > > > Diffs > ----- > > configs/example/ruby_mem_test.py a5ede748a1d9 > src/mem/protocol/MESI_CMP_directory-L1cache.sm a5ede748a1d9 > src/mem/protocol/MESI_CMP_directory-L2cache.sm a5ede748a1d9 > src/mem/protocol/MESI_CMP_directory-dir.sm a5ede748a1d9 > src/mem/protocol/MESI_CMP_directory-msg.sm a5ede748a1d9 > src/mem/protocol/MI_example-cache.sm a5ede748a1d9 > src/mem/protocol/MI_example-dir.sm a5ede748a1d9 > src/mem/protocol/MI_example-msg.sm a5ede748a1d9 > src/mem/protocol/MOESI_CMP_directory-L1cache.sm a5ede748a1d9 > src/mem/protocol/MOESI_CMP_directory-L2cache.sm a5ede748a1d9 > src/mem/protocol/MOESI_CMP_directory-dir.sm a5ede748a1d9 > src/mem/protocol/MOESI_CMP_directory-msg.sm a5ede748a1d9 > src/mem/protocol/MOESI_CMP_token-L1cache.sm a5ede748a1d9 > src/mem/protocol/MOESI_CMP_token-dir.sm a5ede748a1d9 > src/mem/protocol/MOESI_CMP_token-msg.sm a5ede748a1d9 > src/mem/protocol/MOESI_hammer-cache.sm a5ede748a1d9 > src/mem/protocol/MOESI_hammer-dir.sm a5ede748a1d9 > src/mem/protocol/MOESI_hammer-msg.sm a5ede748a1d9 > src/mem/protocol/Network_test-msg.sm a5ede748a1d9 > src/mem/protocol/RubySlicc_Exports.sm a5ede748a1d9 > src/mem/protocol/RubySlicc_MemControl.sm a5ede748a1d9 > src/mem/ruby/buffers/MessageBuffer.hh a5ede748a1d9 > src/mem/ruby/buffers/MessageBuffer.cc a5ede748a1d9 > src/mem/ruby/buffers/MessageBufferNode.hh a5ede748a1d9 > src/mem/ruby/network/Network.hh a5ede748a1d9 > src/mem/ruby/network/simple/PerfectSwitch.cc a5ede748a1d9 > src/mem/ruby/network/simple/SimpleNetwork.hh a5ede748a1d9 > src/mem/ruby/network/simple/SimpleNetwork.cc a5ede748a1d9 > src/mem/ruby/network/simple/Switch.hh a5ede748a1d9 > src/mem/ruby/network/simple/Switch.cc a5ede748a1d9 > src/mem/ruby/slicc_interface/AbstractController.hh a5ede748a1d9 > src/mem/ruby/slicc_interface/Message.hh a5ede748a1d9 > src/mem/ruby/slicc_interface/NetworkMessage.hh a5ede748a1d9 > src/mem/ruby/slicc_interface/RubyRequest.hh a5ede748a1d9 > src/mem/ruby/slicc_interface/RubyRequest.cc a5ede748a1d9 > src/mem/ruby/slicc_interface/RubySlicc_Util.hh a5ede748a1d9 > src/mem/ruby/system/MemoryControl.hh a5ede748a1d9 > src/mem/ruby/system/MemoryControl.cc a5ede748a1d9 > src/mem/ruby/system/RubyMemoryControl.hh a5ede748a1d9 > src/mem/ruby/system/RubyMemoryControl.cc a5ede748a1d9 > src/mem/ruby/system/System.cc a5ede748a1d9 > src/mem/slicc/ast/TypeDeclAST.py a5ede748a1d9 > src/mem/slicc/symbols/StateMachine.py a5ede748a1d9 > src/mem/slicc/symbols/SymbolTable.py a5ede748a1d9 > src/mem/slicc/symbols/Type.py a5ede748a1d9 > > Diff: http://reviews.gem5.org/r/1443/diff/ > > > Testing > ------- > > > Thanks, > > Nilay Vaish > > _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
