I appreciate Nilay's desire to not have the tester configuration diverge from the simulation configuration. However, the general impression I get here is that we're making the C++ more complicated in order to avoid changes to the Python. Given that the point of putting the configuration in python was to enable additional flexibility, this direction (at least superficially) seems completely backwards.
If we want to have common code that sets up Ruby configurations independent of whether it will be supporting various objects representing compute devices (CPUs/GPUs/whatever) or various ports on a single tester object, then IMO the right approach is to find a way to make the Python more modular, not to do unnatural things in the C++. I haven't looked at the code in question, and in fact I'm not all that involved with the Ruby work here at AMD right now, so this is just a high-level comment based on reading this thread. Steve On Mon, Apr 27, 2015 at 3:26 PM, Brad Beckmann <[email protected]> wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/2749/#review6088 > ----------------------------------------------------------- > > > Responding to Nilay's comments over email: > > I think it would be best if you explained the initial bug that led to this > patch. Going forward, the tester will need to work with protocols that do > not expect simply a number of CPUs. There will be controllers that only > generate instruction fetches, and other controllers that support shared > i-caches and separate d-caches. In these protocols, understanding the > number of CPUs is not good enough. The tester will need to be more > flexible handling ports. > > - Brad Beckmann > > > On April 27, 2015, 7:23 p.m., Nilay Vaish wrote: > > > > ----------------------------------------------------------- > > This is an automatically generated e-mail. To reply, visit: > > http://reviews.gem5.org/r/2749/ > > ----------------------------------------------------------- > > > > (Updated April 27, 2015, 7:23 p.m.) > > > > > > Review request for Default. > > > > > > Repository: gem5 > > > > > > Description > > ------- > > > > Changeset 10811:b826d3a54c0c > > --------------------------- > > cpu: testers: rubytest: fix the test > > > > The test was broken. When run with more than a cpu, it resulted in > error while > > configuring the system. Coherence protocols assume that multiple cpus > are > > present when options.num_cpus > 1. But the test only creates one tester > even > > when we were testing a multicpu system. Now, we will create individual > testers > > for each cpu. > > > > > > Diffs > > ----- > > > > src/cpu/testers/rubytest/RubyTester.hh df2aa91dba5b > > src/cpu/testers/rubytest/RubyTester.cc df2aa91dba5b > > src/cpu/testers/rubytest/RubyTester.py df2aa91dba5b > > tests/configs/rubytest-ruby.py df2aa91dba5b > > configs/example/ruby_random_test.py df2aa91dba5b > > src/cpu/testers/rubytest/Check.hh df2aa91dba5b > > src/cpu/testers/rubytest/Check.cc df2aa91dba5b > > src/cpu/testers/rubytest/CheckTable.hh df2aa91dba5b > > src/cpu/testers/rubytest/CheckTable.cc df2aa91dba5b > > > > Diff: http://reviews.gem5.org/r/2749/diff/ > > > > > > Testing > > ------- > > > > > > Thanks, > > > > Nilay Vaish > > > > > > _______________________________________________ > gem5-dev mailing list > [email protected] > http://m5sim.org/mailman/listinfo/gem5-dev > _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
