> 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. > > Brad Beckmann wrote: > 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. > > Joel Hestness wrote: > Ok. Can you elaborate on what you mean by "Controller MessageBuffers > configured differently than the Network MessageBuffers"? Differently in what > ways? If all MessageBuffers are instantiated in Python, that should allow > arbitrary configuration for either use, no?
Sure, I should have been more clear. The key difference is that the Network MessageBuffers will be instantiated by the network Python code and it will be easy for the src/mem/ruby/network/*.py files to set the proper default sizes for these buffers. Meanwhile the controller Python code is autogenerated. Thus to set proper default values for these buffers we need to be able to specify them in the .sm files. - 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
