> On 2011-04-25 09:38:07, Tushar Krishna wrote: > > src/mem/ruby/network/BasicLink.py, line 37 > > <http://reviews.m5sim.org/r/653/diff/1/?file=11716#file11716line37> > > > > could you add a comment somewhere that the bw_multiplier field is only > > used by the simple network?
Thanks for pointing that out. How about we change the name in BasicLink to just bw? Then that bw variable can set the bw_multiplier in the simple network links and the flit size in the garnet links. Actually, I think your comments here and on the endpoint_bandwidth parameter point out that we need to reorganize the bandwidth parameters. How about I do that in a separate subsequent patch? I'll try to get to that soon. > On 2011-04-25 09:38:07, Tushar Krishna wrote: > > src/mem/ruby/network/BasicRouter.hh, line 54 > > <http://reviews.m5sim.org/r/653/diff/1/?file=11717#file11717line54> > > > > typo in "relation" Got it. Thanks. > On 2011-04-25 09:38:07, Tushar Krishna wrote: > > src/mem/ruby/network/Network.py, line 70 > > <http://reviews.m5sim.org/r/653/diff/1/?file=11721#file11721line70> > > > > Some parameters such as topology, number_of_virtual_networks etc are > > used by all networks. > > bandwidth and adaptive routing parameters are only used by simple > > network. > > Either we could move them to a SimpleNetwork.py kind of file, OR > > perhaps we should move the BaseGarnetNetwork.py parameters here into > > Network.py with appropriate comments? I completely agree. As I mentioned above, I'll do this in a separate patch. > On 2011-04-25 09:38:07, Tushar Krishna wrote: > > src/mem/ruby/network/garnet/fixed-pipeline/GarnetLink_d.py, line 40 > > <http://reviews.m5sim.org/r/653/diff/1/?file=11728#file11728line40> > > > > I have been thinking of renaming vcs_per_class to vcs_per_vnet for > > sometime now, to make it consistent with "virtual networks" (instead of > > message class). But that would need a change in BaseGarnetNetwork.py/hh/cc > > too? If it is difficult for you to do, I can do that in a subsequent patch. If you want to do that in a separate patch, that would be great. > On 2011-04-25 09:38:07, Tushar Krishna wrote: > > src/mem/ruby/network/garnet/fixed-pipeline/GarnetNetwork_d.cc, line 175 > > <http://reviews.m5sim.org/r/653/diff/1/?file=11730#file11730line175> > > > > Why is "direction" needed in all the links? Don't the src/dest > > appropriately set up the producers and consumers for the link? > > Is "direction" being used for stats? > > > > To be consistent, the direction of a credit link will be opposite to > > the direction of the corresponding network link. This is a great question and I spent a lot of time trying to think this through. Yes, when you know which is the src and which is the dest, the direction is already implied. However, the links created by the topology are bi-directional (i.e. BasicLink). Then Garnet breaks that into two uni-directional links, and actually as you point out, that is really four links: two main network links and two credit links. The direciton allows us to index into the those two-element arrays of network and credit links. In essence, it allows us to directly map the uni-direcitonal addLink call to the bi-directional python link creation. One possible optimization we could do is instead of explicitly passing the direction of the link, we could implicity create the index value in the make*Link calls by comparing the value of the src and dest. However, I decided against that because it just seemed too complicated. Your comment does point out that I do probably need to improve the comment i n Topology.cc. - Brad ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/653/#review1150 ----------------------------------------------------------- On 2011-04-22 15:16:58, Brad Beckmann wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.m5sim.org/r/653/ > ----------------------------------------------------------- > > (Updated 2011-04-22 15:16:58) > > > Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and > Nathan Binkert. > > > Summary > ------- > > network: convert links & switches to first class C++ SimObjects > > This patch converts links and switches from second class simobjects that were > virtually ignored by the networks (both simple and Garnet) to first class > simobjects that directly correspond to c++ ojbects manipulated by the > topology and network classes. This is especially true for Garnet, where the > links and switches directly correspond to specific C++ objects. > > By making this change, many aspects of the Topology class were simplified. > > > Diffs > ----- > > configs/ruby/MESI_CMP_directory.py 914389024c33 > configs/ruby/MI_example.py 914389024c33 > configs/ruby/MOESI_CMP_directory.py 914389024c33 > configs/ruby/MOESI_CMP_token.py 914389024c33 > configs/ruby/MOESI_hammer.py 914389024c33 > configs/ruby/Network_test.py 914389024c33 > configs/ruby/Ruby.py 914389024c33 > src/mem/protocol/RubySlicc_Exports.sm 914389024c33 > src/mem/ruby/network/BasicLink.hh PRE-CREATION > src/mem/ruby/network/BasicLink.cc PRE-CREATION > src/mem/ruby/network/BasicLink.py PRE-CREATION > src/mem/ruby/network/BasicRouter.hh PRE-CREATION > src/mem/ruby/network/BasicRouter.cc PRE-CREATION > src/mem/ruby/network/BasicRouter.py PRE-CREATION > src/mem/ruby/network/Network.hh 914389024c33 > src/mem/ruby/network/Network.py 914389024c33 > src/mem/ruby/network/SConscript 914389024c33 > src/mem/ruby/network/Topology.hh PRE-CREATION > src/mem/ruby/network/Topology.cc PRE-CREATION > src/mem/ruby/network/garnet/fixed-pipeline/CreditLink_d.hh 914389024c33 > src/mem/ruby/network/garnet/fixed-pipeline/GarnetLink_d.hh PRE-CREATION > src/mem/ruby/network/garnet/fixed-pipeline/GarnetLink_d.cc PRE-CREATION > src/mem/ruby/network/garnet/fixed-pipeline/GarnetLink_d.py PRE-CREATION > src/mem/ruby/network/garnet/fixed-pipeline/GarnetNetwork_d.hh 914389024c33 > src/mem/ruby/network/garnet/fixed-pipeline/GarnetNetwork_d.cc 914389024c33 > src/mem/ruby/network/garnet/fixed-pipeline/GarnetRouter_d.py PRE-CREATION > src/mem/ruby/network/garnet/fixed-pipeline/NetworkLink_d.hh 914389024c33 > src/mem/ruby/network/garnet/fixed-pipeline/NetworkLink_d.cc 914389024c33 > src/mem/ruby/network/garnet/fixed-pipeline/Router_d.hh 914389024c33 > src/mem/ruby/network/garnet/fixed-pipeline/Router_d.cc 914389024c33 > src/mem/ruby/network/garnet/fixed-pipeline/SConscript 914389024c33 > src/mem/ruby/network/garnet/flexible-pipeline/GarnetLink.hh PRE-CREATION > src/mem/ruby/network/garnet/flexible-pipeline/GarnetLink.cc PRE-CREATION > src/mem/ruby/network/garnet/flexible-pipeline/GarnetLink.py PRE-CREATION > src/mem/ruby/network/garnet/flexible-pipeline/GarnetNetwork.hh 914389024c33 > src/mem/ruby/network/garnet/flexible-pipeline/GarnetNetwork.cc 914389024c33 > src/mem/ruby/network/garnet/flexible-pipeline/GarnetRouter.py PRE-CREATION > src/mem/ruby/network/garnet/flexible-pipeline/NetworkLink.hh 914389024c33 > src/mem/ruby/network/garnet/flexible-pipeline/NetworkLink.cc 914389024c33 > src/mem/ruby/network/garnet/flexible-pipeline/Router.hh 914389024c33 > src/mem/ruby/network/garnet/flexible-pipeline/Router.cc 914389024c33 > src/mem/ruby/network/garnet/flexible-pipeline/SConscript 914389024c33 > src/mem/ruby/network/orion/NetworkPower.cc 914389024c33 > src/mem/ruby/network/simple/SimpleNetwork.hh 914389024c33 > src/mem/ruby/network/simple/SimpleNetwork.cc 914389024c33 > src/mem/ruby/network/topologies/Crossbar.py 914389024c33 > src/mem/ruby/network/topologies/Mesh.py 914389024c33 > src/mem/ruby/network/topologies/MeshDirCorners.py 914389024c33 > src/mem/ruby/slicc_interface/AbstractController.hh 914389024c33 > src/mem/ruby/slicc_interface/Controller.py 914389024c33 > > Diff: http://reviews.m5sim.org/r/653/diff > > > Testing > ------- > > > Thanks, > > Brad > > _______________________________________________ m5-dev mailing list m5-dev@m5sim.org http://m5sim.org/mailman/listinfo/m5-dev
