----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/1095/#review2323 -----------------------------------------------------------
It looks much better. I just a few more comments for improvement. They all should be pretty easy to take care of. src/mem/ruby/network/Network.hh <http://reviews.gem5.org/r/1095/#comment2744> Minor comment: Instead of defining the emtpy function as "{return;};", I believe the common convention is to leave it empty "{}" src/mem/ruby/network/topaz/SConscript <http://reviews.gem5.org/r/1095/#comment2745> For all new files, please modify the copywrite owner to the correct institution. src/mem/ruby/network/topaz/TopazNetwork.hh <http://reviews.gem5.org/r/1095/#comment2747> Update copywrite holder src/mem/ruby/network/topaz/TopazNetwork.hh <http://reviews.gem5.org/r/1095/#comment2748> Remove all the "//BEGIN TOPAZ" and "//END TOPAZ" comments. They seem needless and confusing. src/mem/ruby/network/topaz/TopazNetwork.hh <http://reviews.gem5.org/r/1095/#comment2749> Now that you've added more Topaz files to this patch, do you still need to include the separate TPZSimulator header file? If so, I suspect you make this work by using scons EXTRAS and turning on the USE_TOPAZ command line variable. Can you briefly describe that process here? src/mem/ruby/network/topaz/TopazNetwork.py <http://reviews.gem5.org/r/1095/#comment2746> Add copywrite and license to this file src/mem/ruby/network/topaz/TopazSwitch.hh <http://reviews.gem5.org/r/1095/#comment2750> Update comment src/mem/ruby/network/topaz/TopazSwitchFlow.hh <http://reviews.gem5.org/r/1095/#comment2751> update comment src/mem/ruby/profiler/Profiler.cc <http://reviews.gem5.org/r/1095/#comment2752> Why did you have to include this file? src/mem/ruby/profiler/Profiler.cc <http://reviews.gem5.org/r/1095/#comment2753> Why did you remove this line? It seems like the changes you make to Profiler.cc are unnecessary. - Brad Beckmann On March 19, 2012, 12:11 p.m., Valentin Puente wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/1095/ > ----------------------------------------------------------- > > (Updated March 19, 2012, 12:11 p.m.) > > > Review request for Default, Nilay Vaish and Brad Beckmann. > > > Description > ------- > > Interface to integrate TOPAZ network simulator > (http://code.google.com/p/tpzsimul/) within GEM5-RUBY. Does not replace > ruby's interconnection network simulators (garnet and simple-network). It > just adds the hookups required to connect ruby and TOPAZ. You have to > download as "ext" TOPAZ from an external repo. > > To have a better perspective of the changes, you can follow the guide > provided in http://code.google.com/p/tpzsimul/wiki/GEM5Integration. In order > to integrate the compilation process of both simulators we have to slightly > modify main SConstruct and ruby SCons files. All the remaining changes are > constrained to Ruby. Main modifications have been done in > "src/mem/ruby/network/simple/PerfectSwitch.cc" > > > Diffs > ----- > > SConstruct be4990a2c764 > build_opts/ALPHA_Network_Topaz_test PRE-CREATION > build_opts/ALPHA_directory_topaz PRE-CREATION > build_opts/ALPHA_token_topaz PRE-CREATION > configs/ruby/Ruby.py be4990a2c764 > src/mem/ruby/buffers/MessageBuffer.hh be4990a2c764 > src/mem/ruby/buffers/MessageBuffer.cc be4990a2c764 > src/mem/ruby/network/Network.hh be4990a2c764 > src/mem/ruby/network/Topology.cc be4990a2c764 > src/mem/ruby/network/topaz/SConscript PRE-CREATION > src/mem/ruby/network/topaz/SConsopts PRE-CREATION > src/mem/ruby/network/topaz/TopazNetwork.hh PRE-CREATION > src/mem/ruby/network/topaz/TopazNetwork.cc PRE-CREATION > src/mem/ruby/network/topaz/TopazNetwork.py PRE-CREATION > src/mem/ruby/network/topaz/TopazSwitch.hh PRE-CREATION > src/mem/ruby/network/topaz/TopazSwitch.cc PRE-CREATION > src/mem/ruby/network/topaz/TopazSwitchFlow.hh PRE-CREATION > src/mem/ruby/network/topaz/TopazSwitchFlow.cc PRE-CREATION > src/mem/ruby/profiler/Profiler.cc be4990a2c764 > > Diff: http://reviews.gem5.org/r/1095/diff/ > > > Testing > ------- > > Regression test seems to be working for the ruby protocols provided > > > Thanks, > > Valentin Puente > > _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
