----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/561/#review940 -----------------------------------------------------------
Hi Tushar, I have a few minor comments below, but my major question is why does the config file create two versions of physical memory? Do they really need to be accessed? I suspect when we create this patch 1.5 years ago, that was required. However since then, we added a feature that prevents the rubyport from accessing physical memory when it is unecessary. I think this patch should use that feature. Also once we add this patch to the repo, we should add the network test + garnet to the regression tester. I'll add that to my list of outstanding regression tests. configs/example/ruby_network_test.py <http://reviews.m5sim.org/r/561/#comment1310> The network test doesn't actually do any functional checks, correct? If so, it seems that we can remove this line and instead tell the rubyport not to access physical memory. For instance: "ruby_port.access_phys_mem = False" src/cpu/testers/networktest/networktest.cc <http://reviews.m5sim.org/r/561/#comment1309> Expand this comment a bit. This is not straightforward. src/cpu/testers/networktest/networktest.cc <http://reviews.m5sim.org/r/561/#comment1307> Please remove commented line. Also if the network tester doesn't do any functional operation, why is funcmem specified in the config file? src/mem/protocol/Network_test-cache.sm <http://reviews.m5sim.org/r/561/#comment1306> Please remove comment src/mem/protocol/Network_test-cache.sm <http://reviews.m5sim.org/r/561/#comment1305> Please remove commented line src/mem/protocol/Network_test-cache.sm <http://reviews.m5sim.org/r/561/#comment1308> You should add a comment here because it is not obvious what you're doing. src/mem/protocol/Network_test-dir.sm <http://reviews.m5sim.org/r/561/#comment1304> This comment should be removed. src/mem/protocol/Network_test-dir.sm <http://reviews.m5sim.org/r/561/#comment1303> Please remove the commented out lines. - Brad On 2011-03-11 09:33:50, Tushar Krishna wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.m5sim.org/r/561/ > ----------------------------------------------------------- > > (Updated 2011-03-11 09:33:50) > > > Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, Nathan > Binkert, and Brad Beckmann. > > > Summary > ------- > > This patch adds the network tester for simple and garnet networks. > The tester code is in testers/networktest. > The tester can be invoked by configs/example/ruby_network_test.py. > A dummy coherence protocol called Network_test is also addded for > network-only simulations and testing. The protocol takes in messages from the > tester and just pushes them into the network in the appropriate vnet, without > storing any state. > > > Diffs > ----- > > build_opts/ALPHA_SE_Network_test PRE-CREATION > configs/example/ruby_network_test.py PRE-CREATION > configs/ruby/Network_test.py PRE-CREATION > src/cpu/testers/networktest/NetworkTest.py PRE-CREATION > src/cpu/testers/networktest/SConscript PRE-CREATION > src/cpu/testers/networktest/networktest.hh PRE-CREATION > src/cpu/testers/networktest/networktest.cc PRE-CREATION > src/mem/protocol/Network_test-cache.sm PRE-CREATION > src/mem/protocol/Network_test-dir.sm PRE-CREATION > src/mem/protocol/Network_test-msg.sm PRE-CREATION > src/mem/protocol/Network_test.slicc PRE-CREATION > src/mem/protocol/SConsopts 159c07f22c8e > src/mem/ruby/network/garnet/BaseGarnetNetwork.hh 159c07f22c8e > src/mem/ruby/network/garnet/BaseGarnetNetwork.cc 159c07f22c8e > src/mem/ruby/network/garnet/BaseGarnetNetwork.py 159c07f22c8e > src/mem/ruby/network/garnet/fixed-pipeline/GarnetNetwork_d.cc 159c07f22c8e > src/mem/ruby/network/garnet/fixed-pipeline/NetworkInterface_d.cc > 159c07f22c8e > src/mem/ruby/network/garnet/flexible-pipeline/NetworkInterface.cc > 159c07f22c8e > src/mem/ruby/system/Sequencer.hh 159c07f22c8e > src/mem/ruby/system/Sequencer.cc 159c07f22c8e > src/mem/ruby/system/Sequencer.py 159c07f22c8e > > Diff: http://reviews.m5sim.org/r/561/diff > > > Testing > ------- > > Testing done with garnet and simple networks. I will update config > assumptions etc on the wiki. > > > Thanks, > > Tushar > > _______________________________________________ m5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/m5-dev
