> On May 26, 2015, 5:11 p.m., Nilay Vaish wrote: > > I went through both the patches. Unlikely that I am going to agree > > these changes since we are dropping the functionality of connecting buffers > > in python. > > Joel Hestness wrote: > Thanks for reviewing this quickly. Before I spend the time to address the > rest of your changes, I'd prefer to know more about why we're "connecting" > buffers to ports in Python code. Connecting buffers to ports in Python is > very confusing (buffers do not expose a port interface), and the code in > pyobject.cc doesn't actually do anything with the port to which the buffer is > connected. Can you elaborate why we do that and why it shouldn't be dropped?
I would be easy to move buffer-to-port connections back into Python code to get them printed in config.ini files. I do feel like the existing way of doing this is confusing, because buffers do not expose a port interface, so I'll be looking for clearer ways to get the same effect. I strongly prefer Python configurability of buffers rather than allowing sizing in the .sm files. I'm willing to spend some time to make it happen. Can you let me know if you support moving MessageBuffers to Python like this, or if you would prefer we do it another way? > On May 26, 2015, 5:11 p.m., Nilay Vaish wrote: > > src/mem/ruby/slicc_interface/AbstractController.cc, line 70 > > <http://reviews.gem5.org/r/2845/diff/1/?file=45415#file45415line70> > > > > Don't you need to set the receiver and the ordering property? Code for this is now auto-generated to be consistent with other MessageBuffers (in the constructor and init() functions). > On May 26, 2015, 5:11 p.m., Nilay Vaish wrote: > > src/mem/ruby/slicc_interface/AbstractController.cc, line 294 > > <http://reviews.gem5.org/r/2845/diff/1/?file=45415#file45415line294> > > > > Do we not need to functionally write to the memory? Code for this is now auto-generated to be consistent with other MessageBuffers (in functionalWriteBuffers(PacketPtr& pkt)). > On May 26, 2015, 5:11 p.m., Nilay Vaish wrote: > > src/mem/ruby/network/simple/SimpleNetwork.cc, line 124 > > <http://reviews.gem5.org/r/2845/diff/1/?file=45412#file45412line124> > > > > This is not right. Yep, this is what I'll need to tackle next. - Joel ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/2845/#review6397 ----------------------------------------------------------- On May 25, 2015, 11:51 p.m., Joel Hestness wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/2845/ > ----------------------------------------------------------- > > (Updated May 25, 2015, 11:51 p.m.) > > > Review request for Default. > > > Repository: gem5 > > > Description > ------- > > Changeset 10850:0fc919c768b2 > --------------------------- > 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/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
