> On July 27, 2015, 10:29 p.m., Nilay Vaish wrote: > > src/mem/slicc/ast/ObjDeclAST.py, lines 60-64 > > <http://reviews.gem5.org/r/2845/diff/6/?file=48498#file48498line60> > > > > Do we need this code?
No. I've removed this. > On July 27, 2015, 10:29 p.m., Nilay Vaish wrote: > > src/mem/ruby/network/simple/SimpleNetwork.py, lines 48-77 > > <http://reviews.gem5.org/r/2845/diff/6/?file=48492#file48492line48> > > > > I read Brad's comment on this function. I think the original choice > > was right. I don't see why we want to compile this into the binary. If > > configs/ruby/Ruby.py is becoming a problem, we should just split that file. > > Brad Beckmann wrote: > The function manipulates members of the network. It belongs in the > network class. That is simply good object-oriented design. I've felt the same way as Nilay about this, but followed Brad's suggestion in hopes of getting this patch out the door. This code can be quite problematic to leave in SimpleNetwork.py and compile into the binary: Suppose someone wanted to set the buffer size or randomization on these buffers, which seem like a reasonable ideas. Either they would have to modify this code and recompile the simulator, or they would have to hack up Ruby.py\/another config file to (a) reinstantiate all these buffers with appropriate parameters, (b) append them in local lists, and (c) reassign the network and switch vector params with those lists. Yes, having this in SimpleNetwork is good object-oriented design, but it is likely to cause headaches and kinda defeats the purpose of having simulation configs. @Brad: Are you strongly opposed to housing this in Ruby.py? > On July 27, 2015, 10:29 p.m., Nilay Vaish wrote: > > src/python/swig/pyobject.cc, line 109 > > <http://reviews.gem5.org/r/2845/diff/6/?file=48500#file48500line109> > > > > Can you confirm that these checks on name1 and name2 are still required? Seems these are no longer required, since we are now casting to MessageBuffers instead of AbstractControllers and SLICC generated code now connects buffers to memories. I've removed these checks. - Joel ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/2845/#review6835 ----------------------------------------------------------- On July 25, 2015, 3:45 p.m., Joel Hestness wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/2845/ > ----------------------------------------------------------- > > (Updated July 25, 2015, 3:45 p.m.) > > > Review request for Default. > > > Repository: gem5 > > > Description > ------- > > Changeset 10936:85b34fa1e87a > --------------------------- > 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) Distinguishes buffer-specific parameters from buffer-to-network parameters. > Specifically, buffer size, randomization, ordering, recycle latency, and ports > are all specific to a MessageBuffer, while the virtual network ID and type are > intrinsics of how the buffer is connected to network ports. The former are > specified in the Python object, while the latter are specified in the > controller *.sm files. Unlike buffer-specific parameters, which may need to > change depending on the simulated system structure, buffer-to-network > parameters can be specified statically for most or all different simulated > systems. > > > Diffs > ----- > > src/mem/ruby/network/simple/Switch.hh cafae9abd4e4 > src/mem/ruby/network/simple/Switch.cc cafae9abd4e4 > src/mem/ruby/network/simple/Throttle.cc cafae9abd4e4 > src/mem/ruby/slicc_interface/AbstractController.hh cafae9abd4e4 > src/mem/ruby/slicc_interface/AbstractController.cc cafae9abd4e4 > src/mem/slicc/ast/ObjDeclAST.py cafae9abd4e4 > src/mem/slicc/symbols/StateMachine.py cafae9abd4e4 > src/python/swig/pyobject.cc cafae9abd4e4 > src/mem/ruby/network/simple/SimpleNetwork.py cafae9abd4e4 > src/mem/ruby/network/simple/SimpleNetwork.cc cafae9abd4e4 > src/mem/ruby/network/simple/SimpleNetwork.hh cafae9abd4e4 > src/mem/ruby/network/MessageBuffer.py PRE-CREATION > src/mem/ruby/network/SConscript cafae9abd4e4 > src/mem/ruby/network/simple/PerfectSwitch.cc cafae9abd4e4 > src/mem/ruby/network/MessageBuffer.cc cafae9abd4e4 > configs/ruby/Ruby.py cafae9abd4e4 > src/mem/protocol/RubySlicc_Defines.sm cafae9abd4e4 > src/mem/ruby/SConscript cafae9abd4e4 > src/mem/ruby/network/MessageBuffer.hh cafae9abd4e4 > > 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
