> On Oct. 11, 2012, 12:50 a.m., Andreas Hansson wrote: > > src/mem/protocol/MESI_CMP_directory-msg.sm, line 83 > > <http://reviews.gem5.org/r/1443/diff/3/?file=30215#file30215line83> > > > > Why is it not symmetric with the reads?
While a check can be added in functionalWrite(), it is not necessary. The cases in which the data block may not exist (from an actual physical implementation prespective), the protocol will not read the data that a functionalWrite() may write to the block. But that's not true for a functionalRead(). The data block is present in all the messages, but only a few of them hold the actual data. Hence, the check is required. > On Oct. 11, 2012, 12:50 a.m., Andreas Hansson wrote: > > src/mem/protocol/MESI_CMP_directory-msg.sm, line 75 > > <http://reviews.gem5.org/r/1443/diff/3/?file=30215#file30215line75> > > > > A comment on what (or what is not) a coherence request of type PUTX? A comment on what is a PUTX appears with its definition. > On Oct. 11, 2012, 12:50 a.m., Andreas Hansson wrote: > > src/mem/protocol/MESI_CMP_directory-msg.sm, line 98 > > <http://reviews.gem5.org/r/1443/diff/3/?file=30215#file30215line98> > > > > Same as previously, a short comment on why Data dataExclusive and > > memory data (what is data/mem data difference?). There should be comments about these message types where they are defined. But those comments are not helpful. I am editing those so that differentiation can be made between types. > On Oct. 11, 2012, 12:50 a.m., Andreas Hansson wrote: > > src/mem/protocol/MESI_CMP_directory-msg.sm, line 110 > > <http://reviews.gem5.org/r/1443/diff/3/?file=30215#file30215line110> > > > > Again, why the asymmetry with reads? Same comment as above. > On Oct. 11, 2012, 12:50 a.m., Andreas Hansson wrote: > > src/mem/ruby/buffers/MessageBuffer.hh, line 152 > > <http://reviews.gem5.org/r/1443/diff/3/?file=30232#file30232line152> > > > > Why a pointer reference? This is a mistake on my part. > On Oct. 11, 2012, 12:50 a.m., Andreas Hansson wrote: > > src/mem/ruby/buffers/MessageBuffer.hh, line 158 > > <http://reviews.gem5.org/r/1443/diff/3/?file=30232#file30232line158> > > > > a const pointer ref? Why? > > > > Is it not possible that the write turns the packet into a response? I am removing the reference. > On Oct. 11, 2012, 12:50 a.m., Andreas Hansson wrote: > > src/mem/ruby/buffers/MessageBuffer.cc, line 451 > > <http://reviews.gem5.org/r/1443/diff/3/?file=30233#file30233line451> > > > > Could this be iterators as well? Feels a bit off to mix counter based > > and iterator based iteration. I am not a fan of iterators. But my knowledge of C++ tells me that map and list datatypes do not support counter based traversal. With vectors, I tend to do counter based traversal only. > On Oct. 11, 2012, 12:50 a.m., Andreas Hansson wrote: > > src/mem/ruby/buffers/MessageBuffer.cc, line 496 > > <http://reviews.gem5.org/r/1443/diff/3/?file=30233#file30233line496> > > > > 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. In case of functional write, the order does not matter since all the messages in the memory system are updated. For functional reads, any packet that holds the data should be holding the correct data since we allow only exclusive writes. > On Oct. 11, 2012, 12:50 a.m., Andreas Hansson wrote: > > src/mem/ruby/slicc_interface/RubyRequest.cc, line 28 > > <http://reviews.gem5.org/r/1443/diff/3/?file=30245#file30245line28> > > > > 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. I would not say memtest is broken, but I agree that it is restrictive. Since one of the purposes of the patch is to clear the regression tests, I have to produce code that would satisfy the tester. As and when the tester is changed, I would redo the implementation to take care of the changes. - Nilay ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/1443/#review3561 ----------------------------------------------------------- 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
