> On May 27, 2015, 12:22 a.m., Brad Beckmann wrote: > > src/mem/ruby/network/simple/SimpleNetwork.cc, line 124 > > <http://reviews.gem5.org/r/2845/diff/1/?file=45412#file45412line124> > > > > I believe Nilay's concern is that you are instantiating a SimObject in > > the C++ code. I could be wrong, but I believe all other SimObjects are > > instantiated by swig via the python configuration files. I believe the > > expected implementation would be to create all these MessageBuffers in the > > Network/Switch .py files. > > Mark Wilkening wrote: > Right now this patch is missing the main point -- "First and foremost, it > exposes MessageBuffers as SimObjects that can be manipulated in Python code. > This allows parameters to be set and checked in Python code to avoid > obfuscating parameters within protocol files." By instantiating > MessageBuffers in C++ code with default parameters we cannot configure them > individually at all. I think the difficult task here is being able to define > all the controller defined MessageBuffers in python and set the > MessageBuffers properly from the Params in the controller C++. I think we > might need to autogenerate protocol specific python controller class > definitions and params, give them defaults but allow network or topology > python code to configure the protocol specific buffers. > > Joel Hestness wrote: > I'm not sure I follow what you're saying. This patch *does* expose all > appropriate MessageBuffers to be manipulated in Python for all generated > controllers. When swig instantiates the MessageBuffers for the Python > SimObjects, their parameters can be set up immediately in the MessageBuffer > constructor without any need for code generation. > > For this patch, I've collected the following feedback, and I plan to try > to address them with further updates to this patch: > 1) From Nilay: If we are going to expose MessageBuffer parameters in > Python, then actually set parameters in the MessageBuffer constructor using > Python parameters. This should be easy. > 2) From Nilay: We need the port connections to be printed to the > config.ini. This patch should not drop that. This should also be easy given > that I had a version of this patch that left the port connections in at one > point. > 3) From Nilay and Brad: Move SimpleNetwork and Switch MessageBuffer > instantiation into Python, since we should disallow C++ instantiation of > SimObjects. I still have not had a chance to look at what this will take.
Joel, I believe part of Mark's question will be answered when you take care of point #3 and remove the C++ instantiation of the Network MessageBuffers. I also believe Mark would like to see the Controller MessageBuffers configured differently than the Network MessageBuffers. That is one of the benefits of the patch http://reviews.gem5.org/r/2814/. I think it makes sense to merge those changes together because we want the network injector buffering to be sized differently that the internal network buffering. - Brad ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/2845/#review6403 ----------------------------------------------------------- On May 27, 2015, 6:13 p.m., Joel Hestness wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/2845/ > ----------------------------------------------------------- > > (Updated May 27, 2015, 6:13 p.m.) > > > Review request for Default. > > > Repository: gem5 > > > Description > ------- > > Changeset 10850:0c31c4ef98c0 > --------------------------- > ruby: Expose MessageBuffers as SimObjects > > Expose MessageBuffers from SLICC controllers as SimObjects that can be > manipulated in Python. This patch has numerous benefits: > 1) First and foremost, it exposes MessageBuffers as SimObjects that can be > manipulated in Python code. This allows parameters to be set and checked in > Python code to avoid obfuscating parameters within protocol files. Further, > now > as SimObjects, MessageBuffer parameters are printed to config output files as > a > way to track parameters across simulations (e.g. buffer sizes) > 2) Cleans up special-case code for responseFromMemory buffers, and aligns > their > instantiation and use with mandatoryQueue buffers. These two special buffers > are the only MessageBuffers that are exposed to components outside of SLICC > controllers, and they're both slave ends of these buffers. They should be > exposed outside of SLICC in the same way, and this patch does it. > 3) Removes the obfuscated port-like connection of MessageBuffers. Ports are > now connected in the init function of the generated controllers. > > > Diffs > ----- > > src/mem/protocol/RubySlicc_Defines.sm e61f847e74fd > src/mem/ruby/network/MessageBuffer.hh e61f847e74fd > src/mem/ruby/network/MessageBuffer.cc e61f847e74fd > src/mem/ruby/network/MessageBuffer.py PRE-CREATION > src/mem/ruby/network/SConscript e61f847e74fd > src/mem/ruby/network/simple/SimpleNetwork.cc e61f847e74fd > src/mem/ruby/network/simple/Switch.cc e61f847e74fd > src/mem/ruby/slicc_interface/AbstractController.hh e61f847e74fd > src/mem/ruby/slicc_interface/AbstractController.cc e61f847e74fd > src/mem/slicc/symbols/StateMachine.py e61f847e74fd > src/python/swig/pyobject.cc e61f847e74fd > > Diff: http://reviews.gem5.org/r/2845/diff/ > > > Testing > ------- > > Regression tests with all Ruby protocols. Protocol file changes are in > separate patch (http://reviews.gem5.org/r/2844/) to keep this one slim. > > > Thanks, > > Joel Hestness > > _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
