I finally read this diff. My comments are inline. In the future,
please use "hg email" which is found in the patchbomb extension. It
makes it far easier to comment on diffs since I can just then reply to
the e-mail without having to jump through hoops to get it into my
mailer.
Nate
> diff -r 369b61762d7b -r e5a3ba0bbe88 tests/configs/memtest-ruby.py
> --- a/tests/configs/memtest-ruby.py Mon Aug 31 16:38:22 2009 -0500
> +++ b/tests/configs/memtest-ruby.py Mon Aug 31 16:39:59 2009 -0500
> @@ -29,13 +29,15 @@
> import m5
> from m5.objects import *
>
> +#MAX CORES IS 8 with the fals sharing method
typo: false
> +assert(nb_cores > 0 and nb_cores <= 8)
> +assert(nb_cores == int(nb_cores))
> +assert(config_file != 'none')
>
> -#MAX CORES IS 8 with the fals sharing method
> -nb_cores = 8
> cpus = [ MemTest() for i in xrange(nb_cores) ]
>
> import ruby_config
> -ruby_memory = ruby_config.generate("MI_example-homogeneous.rb", nb_cores)
> +ruby_memory = ruby_config.generate(config_file, nb_cores)
>
> # system simulated
> system = System(cpu = cpus, funcmem = PhysicalMemory(),
why did you move nb_cores to run.py? I think that's the wrong place
for that. run.py is supposed to only run the script, nothing more.
If you want some place to stick ruby stuff, create another file that
all ruby tests can import.
> diff -r 369b61762d7b -r e5a3ba0bbe88 tests/quick/50.memtest/test.py
> --- a/tests/quick/50.memtest/test.py Mon Aug 31 16:38:22 2009 -0500
> +++ b/tests/quick/50.memtest/test.py Mon Aug 31 16:39:59 2009 -0500
> @@ -25,6 +25,8 @@
> # OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
> #
> # Authors: Ron Dreslinski
> +import m5
> +from m5.objects import *
>
> MemTest.max_loads=1e5
> MemTest.progress_interval=1e4
Is this actually necessary?
> diff -r 369b61762d7b -r e5a3ba0bbe88 tests/run.py
> --- a/tests/run.py Mon Aug 31 16:38:22 2009 -0500
> +++ b/tests/run.py Mon Aug 31 16:39:59 2009 -0500
> @@ -38,6 +38,7 @@
>
> # single "path" arg encodes everything we need to know about test
> (category, name, isa, opsys, config) = sys.argv[1].split('/')[-5:]
> +(build, protocol) = sys.argv[1].split('/')[:2]
There's a better way to do this below.
>
> # find path to directory containing this file
> tests_root = os.path.dirname(__file__)
> @@ -62,7 +63,15 @@
>
> # build configuration
> sys.path.append(joinpath(tests_root, 'configs'))
> -execfile(joinpath(tests_root, 'configs', config + '.py'))
> +nb_cores = 8
> +config_file = 'none'
> +if (protocol == 'LIBRUBY_MI_example'):
> + config_file = 'MI_example-homogeneous.rb'
> +elif (protocol == 'LIBRUBY_MOESI_CMP_directory'):
> + config_file = 'TwoLevel_SplitL1UnifiedL2.rb'
> +
> +globals = {'nb_cores':nb_cores, 'config_file':config_file}
The added code above should go into some sort of Ruby file. Also the
protocol variable is just something that you guys did and isn't how
the protocol is really stored. The right way to do it is add
'PROTOCOL' to export_vars in src/mem/protocol/SConsopts (See
src/mem/ruby/SConsopts for an example of how this is done.) You can
then use m5.build_env['PROTOCOL'] to get the actual protocol name.
(just grep for build_env in the tree to see examples).
> +execfile(joinpath(tests_root, 'configs', config + '.py'), globals, globals)
Given that you shouldn't be sticking nb_cores and config_file here, I
don't think you should change this line.
_______________________________________________
m5-dev mailing list
m5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/m5-dev
