> On June 24, 2015, 4:54 p.m., Joel Hestness wrote: > > configs/ruby/Ruby.py, line 92 > > <http://reviews.gem5.org/r/2845/diff/4/?file=46640#file46640line92> > > > > Splitting Brad's comments for easier tracking: "Overall, can this code > > be moved into the Switch and Liny .py files rather than have it here in > > Ruby.py?" > > Joel Hestness wrote: > Actually, I don't think it make sense to move these into the links, > switches or the SimpleNetwork. By setting up buffers here, the user can > modify buffer parameters in the configs and not require recompilation. If we > move this setup into, for example, SimpleNetwork.py - which knows how buffers > need to be instantiated and connected, then a user would need find the > buffers they want to modify in the config files here, after the call to > network.setup_buffers(), and then override their parameters. OR they would > have to modify the parameters in one of the SimObject declarations and then > recompile the simulator. At least with the setup as-is, it is easy to > identify and modify buffer parameters. > > Would it be ok to leave this? > > Brad Beckmann wrote: > On can still modify the buffer parameters even if you move this code into > the Switch and Link .py files. All this code is doing is expanding each link > into it's virtual channels. To modify the MessageBuffers parameters, the > user should be modifying the topology file. That is where they know which > switch/link is which. > > Also let's rename the network.message_buffers network.int_link_buffers or > network.interface_buffers. That way folks know the difference between the > message buffers assoicated with the network and those associated with the > switches. > > Joel Hestness wrote: > >> On can still modify the buffer parameters even if you move this code > into the Switch and Link .py files. All this code is doing is expanding each > link into it's virtual channels. To modify the MessageBuffers parameters, > the user should be modifying the topology file. That is where they know > which switch/link is which. > > I apologize, but I'm must be missing what you mean by "moving this code > into Switch and Link .py files". I can sort of see how links could parent > their MessageBuffers, since the network just passes its buffers through to > the int_links. However, I think that change is outside the scope of this > patch, since the aim of this patch is promoting MessageBuffers to SimObjects > rather than changing how links use MessageBuffers. > > Moving buffers into Switch.py is confusing because not every switch port > has buffers for VCs. Only ports between switches, and int_links/out_links > require buffers. Thus, the topology matters in deciding where to instantiate > MessageBuffers. However, the topology does NOT define the VC architecture. It > only defines the connections between network components. The SimpleNetwork is > the only component that knows about switch connections and VC architecture. > > Can you clarify what you mean here? > > > >> Also let's rename the network.message_buffers network.int_link_buffers > or network.interface_buffers. That way folks know the difference between the > message buffers assoicated with the network and those associated with the > switches. > > This also confuses me. Buffers associated with the network links are > children of the network, while buffers associated with switches are children > of the switches. I can change the buffer vector's name to > network.int_link_buffers, but that won't clarify anything the way you > suggest. Was there something more you were hoping for here? > > Brad Beckmann wrote: > All I'm asking is to move the first and third loops from Ruby.py to > SimpleLink.py and the second loop from Ruby.py to SimpleNetwork.py. I'm not > asking for a big change here. I'm certainly not asking to change how links > use MessageBuffers. > > When you say VC architecture, are you referring to something other than > the number of VCs? The number of VCs is a parameter from the RubyNetwork and > is easily accessible to the SimpleLink and SimpleNetwork (inherits from > RubyNetwork). > > The VC design is part of the protocol. Often Topology files are protocol > specific, so if you want to do something fancy with VC buffers, it seems like > the Topology file is were to do it. > > What is a "switch port" and which switch port does not have > MessageBuffers for VCs? > > Joel Hestness wrote: > Sorry. I'm still confused about what you're requesting here. > > In the setup_buffers function, there are 8 total loops, 4 of which are > leaf loops that expand ports to VCs. Are you requesting that I move these > port-to-VC expanding loops into other .py files? If so, the trouble is that > Python VectorParams can only be assigned once, so we'd still need to maintain > the network_buffers and router_buffers lists as local variables somewhere and > accumulate the buffer instances before assigning them to > network.int_link_buffers and router.buffers. > > If you're requesting something else, I'd need some more direction. I > can't figure out which loops (and code) you're referring to that be should > moved.
Ok, I'll simplify my request. Please move the entire setup_buffers function to Network.py. My main goal here is not to add another function to Ruby.py unless it is absolutely necessary. As an example, see the attachIO function in src/dev/x86/Pc.py. That is essentially what I'm looking for. The both functions are rather straight-forward and are simply connecting things together that are specific to a particular class. Does that make sense? - Brad ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/2845/#review6591 ----------------------------------------------------------- On June 20, 2015, 4:14 p.m., Joel Hestness wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/2845/ > ----------------------------------------------------------- > > (Updated June 20, 2015, 4:14 p.m.) > > > Review request for Default. > > > Repository: gem5 > > > Description > ------- > > Changeset 10859:d0dddb8fc9de > --------------------------- > 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 > ----- > > configs/ruby/Ruby.py d02b45a554b5 > src/mem/protocol/RubySlicc_Defines.sm d02b45a554b5 > src/mem/ruby/SConscript d02b45a554b5 > src/mem/ruby/network/MessageBuffer.hh d02b45a554b5 > src/mem/ruby/network/MessageBuffer.cc d02b45a554b5 > src/mem/ruby/network/MessageBuffer.py PRE-CREATION > src/mem/ruby/network/SConscript d02b45a554b5 > src/mem/ruby/network/simple/SimpleNetwork.hh d02b45a554b5 > src/mem/ruby/network/simple/SimpleNetwork.cc d02b45a554b5 > src/mem/ruby/network/simple/SimpleNetwork.py d02b45a554b5 > src/mem/ruby/network/simple/Switch.hh d02b45a554b5 > src/mem/ruby/network/simple/Switch.cc d02b45a554b5 > src/mem/ruby/slicc_interface/AbstractController.hh d02b45a554b5 > src/mem/ruby/slicc_interface/AbstractController.cc d02b45a554b5 > src/mem/slicc/ast/ObjDeclAST.py d02b45a554b5 > src/mem/slicc/symbols/StateMachine.py d02b45a554b5 > src/python/swig/pyobject.cc d02b45a554b5 > > 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
