On Mon, 27 Apr 2015, Brad Beckmann wrote:
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/2749/#review6086
-----------------------------------------------------------
Why make such a large and ugly change to fix a seemingly simple problem
in configuration? The Ruby Tester has always been a single tester, with
multiple ports, rather than a set of testers. This patch makes a set of
testers, but then uses an ugly combination of static variables and
conditions to fall back on the single tester logic.
I knowingly did not change the configuration files. If we create multiple
cpus when we run actual simulations, why not create multiple testers? I
do not want to add checks to python configuration files that change
behavior of the code when a tester is being invoked. I think this
difference in behavior is the reason why bugs creep into the testers
concerned with ruby.
As far as use of a static variable is concerned, since the original code
always created a single tester variable, I do not see why a static
variable is a problem. If you are still not convinced, I can convert the
check table to a sim object and add a pointer to it to the tester python
object.
We have a patch in our patch queue that we will soon post that I believe fixes
this problem in a much better way.
I will not believe your claim unless I see the code myself. And if your
patch puts in a check that the code should behave differently when a
tester is invoked, in my opinion, that is not better at all.
src/cpu/testers/rubytest/RubyTester.hh (line 140)
<http://reviews.gem5.org/r/2749/#comment5292>
We should avoid static variables. At some point, we could be interested in
running multiple instances of Ruby with the random tester.
Since ruby_random_test.py anyway did not used to create more than one
tester, I do not see why having a static variable is problem. There are
several things that would need to change in ruby to run multiple
instances.
src/cpu/testers/rubytest/RubyTester.cc (line 96)
<http://reviews.gem5.org/r/2749/#comment5293>
This is pretty ugly hack. The tester is not meant to be created per cpu.
There is a single tester and check table that is used to coordinate the races.
Again if we are creating multiple cpus when we run simulations, we should
be creating multiple testers. I do not want to test in a fashion
different from the fashion in which we run actual simulations.
--
Nilay
_______________________________________________
gem5-dev mailing list
[email protected]
http://m5sim.org/mailman/listinfo/gem5-dev
