----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/3558/#review8505 -----------------------------------------------------------
Thanks for posting the patches Tushar. I think this code is a lot cleaner and i'm interested in trying out the unidirectional links and seperate routing latency for some studies. One major comment: There is a lot of new code for routing algorithms in this patch. I think it would be useful to convert the RoutingUnit to a SimObject and allow the Router SimObject to accept a RoutingUnit SimObject as a parameter. In this way, you could implement multiple "custom" routing algorithms and swap them via python scripts. I marked two spots below where changes would need to be made. src/mem/ruby/network/garnet2.0/NetworkInterface.cc (lines 277 - 283) <http://reviews.gem5.org/r/3558/#comment7383> The spacing is a bit odd here. src/mem/ruby/network/garnet2.0/Router.cc (line 58) <http://reviews.gem5.org/r/3558/#comment7387> If RoutingUnit is a SimObject, this can be set using the Params struct and the RoutingUnit could be specified by the python scripts. src/mem/ruby/network/garnet2.0/RoutingUnit.hh (line 71) <http://reviews.gem5.org/r/3558/#comment7386> Make this method virtual so classes can inherit this one to specify different custom routing computations. src/mem/ruby/network/garnet2.0/SwitchAllocator.hh (lines 82 - 83) <http://reviews.gem5.org/r/3558/#comment7384> The space been the final two > can be removed in C++11. src/mem/ruby/network/garnet2.0/flit.hh (lines 49 - 64) <http://reviews.gem5.org/r/3558/#comment7385> Could all the getter methods and all the setter methods be grouped? That would make this file more readable. - Matthew Poremba On July 13, 2016, 7:04 a.m., Tushar Krishna wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/3558/ > ----------------------------------------------------------- > > (Updated July 13, 2016, 7:04 a.m.) > > > Review request for Default, Andreas Hansson, Brad Beckmann, Jieming Yin, and > Matthew Poremba. > > > Repository: gem5 > > > Description > ------- > > network: garnet2.0 > Revamped version of garnet with more optimized single-cycle routers, > more configurability, and cleaner code. > > > Diffs > ----- > > src/mem/ruby/network/garnet2.0/RoutingUnit.cc PRE-CREATION > src/mem/ruby/network/garnet2.0/SConscript PRE-CREATION > src/mem/ruby/network/garnet2.0/SwitchAllocator.hh PRE-CREATION > src/mem/ruby/network/garnet2.0/SwitchAllocator.cc PRE-CREATION > src/mem/ruby/network/garnet2.0/VirtualChannel.hh PRE-CREATION > src/mem/ruby/network/garnet2.0/VirtualChannel.cc PRE-CREATION > src/mem/ruby/network/garnet2.0/flit.hh PRE-CREATION > src/mem/ruby/network/garnet2.0/flit.cc PRE-CREATION > src/mem/ruby/network/garnet2.0/flitBuffer.hh PRE-CREATION > src/mem/ruby/network/garnet2.0/flitBuffer.cc PRE-CREATION > configs/network/Network.py PRE-CREATION > configs/topologies/Crossbar.py cdb94f2332a6 > configs/topologies/MeshDirCorners_XY.py PRE-CREATION > configs/topologies/Mesh_XY.py PRE-CREATION > configs/topologies/Mesh_westfirst.py PRE-CREATION > configs/topologies/Pt2Pt.py cdb94f2332a6 > src/base/statistics.cc cdb94f2332a6 > src/mem/ruby/network/BasicRouter.py cdb94f2332a6 > src/mem/ruby/network/garnet2.0/CommonTypes.hh PRE-CREATION > src/mem/ruby/network/garnet2.0/Credit.hh PRE-CREATION > src/mem/ruby/network/garnet2.0/Credit.cc PRE-CREATION > src/mem/ruby/network/garnet2.0/CreditLink.hh PRE-CREATION > src/mem/ruby/network/garnet2.0/CrossbarSwitch.hh PRE-CREATION > src/mem/ruby/network/garnet2.0/CrossbarSwitch.cc PRE-CREATION > src/mem/ruby/network/garnet2.0/GarnetLink.hh PRE-CREATION > src/mem/ruby/network/garnet2.0/GarnetLink.cc PRE-CREATION > src/mem/ruby/network/garnet2.0/GarnetLink.py PRE-CREATION > src/mem/ruby/network/garnet2.0/GarnetNetwork.hh PRE-CREATION > src/mem/ruby/network/garnet2.0/GarnetNetwork.cc PRE-CREATION > src/mem/ruby/network/garnet2.0/GarnetNetwork.py PRE-CREATION > src/mem/ruby/network/garnet2.0/InputUnit.hh PRE-CREATION > src/mem/ruby/network/garnet2.0/InputUnit.cc PRE-CREATION > src/mem/ruby/network/garnet2.0/NetworkInterface.hh PRE-CREATION > src/mem/ruby/network/garnet2.0/NetworkInterface.cc PRE-CREATION > src/mem/ruby/network/garnet2.0/NetworkLink.hh PRE-CREATION > src/mem/ruby/network/garnet2.0/NetworkLink.cc PRE-CREATION > src/mem/ruby/network/garnet2.0/OutVcState.hh PRE-CREATION > src/mem/ruby/network/garnet2.0/OutVcState.cc PRE-CREATION > src/mem/ruby/network/garnet2.0/OutputUnit.hh PRE-CREATION > src/mem/ruby/network/garnet2.0/OutputUnit.cc PRE-CREATION > src/mem/ruby/network/garnet2.0/README.txt PRE-CREATION > src/mem/ruby/network/garnet2.0/Router.hh PRE-CREATION > src/mem/ruby/network/garnet2.0/Router.cc PRE-CREATION > src/mem/ruby/network/garnet2.0/RoutingUnit.hh PRE-CREATION > > Diff: http://reviews.gem5.org/r/3558/diff/ > > > Testing > ------- > > > Thanks, > > Tushar Krishna > > _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
