-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/3647/#review8824
-----------------------------------------------------------


I like this approach! A couple of comments,though.

1. I think you need to add an __init__.py file to each directory (ruby, 
topologies, network) for this to work. Did you forget to hg add them?
2. Since we don't have good test coverage, did you eyeball that the rest of the 
scripts in configs/ruby didn't need to be updated similarly? Looking that them, 
I think AMD_Base_Constructor.py and GPU_VIPER_Region.py should be updated too. 
I don't *think* there are any others, but I could be wrong.

Thanks for fixing this the "right" way. Maybe next time someone can factor out 
all of the common code across all of these files and make them more "modular".

- Jason Lowe-Power


On Oct. 12, 2016, 8:55 a.m., Andreas Hansson wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.gem5.org/r/3647/
> -----------------------------------------------------------
> 
> (Updated Oct. 12, 2016, 8:55 a.m.)
> 
> 
> Review request for Default.
> 
> 
> Repository: gem5
> 
> 
> Description
> -------
> 
> Changeset 11670:48761052b362
> ---------------------------
> ruby: Fix regressions and make Ruby configs Python packages
> 
> This patch moves the addition of network options into the Ruby module
> to avoid the regressions all having to add it explicitly. Doing this
> exposes an issue in our current config system though, namely the fact
> that addtoPath is relative to the Python script being executed. Since
> both example and regression scripts use the Ruby module we would end
> up with two different (relative) paths being added. Instead we take a
> first step at turning the config modules into Python packages, simply
> by adding a __init__.py in the configs/ruby, configs/topologies and
> configs/network subdirectories.
> 
> As a result, we can now add the top-level configs directory to the
> Python search path, and then use the package names in the various
> modules. The example scripts are also updated, and the messy
> path-deducing variations in the scripts are unified.
> 
> 
> Diffs
> -----
> 
>   tests/configs/simple-timing-mp-ruby.py 220fa4099b9a 
>   tests/configs/simple-timing-ruby.py 220fa4099b9a 
>   tests/configs/memtest-ruby.py 220fa4099b9a 
>   tests/configs/o3-timing-mp-ruby.py 220fa4099b9a 
>   tests/configs/o3-timing-ruby.py 220fa4099b9a 
>   tests/configs/pc-simple-timing-ruby.py 220fa4099b9a 
>   tests/configs/rubytest-ruby.py 220fa4099b9a 
>   tests/configs/simple-atomic-mp-ruby.py 220fa4099b9a 
>   configs/ruby/GPU_VIPER.py 220fa4099b9a 
>   configs/ruby/GPU_VIPER_Baseline.py 220fa4099b9a 
>   configs/ruby/MOESI_AMD_Base.py 220fa4099b9a 
>   configs/ruby/Ruby.py 220fa4099b9a 
>   tests/configs/gpu-randomtest-ruby.py 220fa4099b9a 
>   tests/configs/gpu-ruby.py 220fa4099b9a 
>   configs/example/ruby_direct_test.py 220fa4099b9a 
>   configs/example/ruby_gpu_random_test.py 220fa4099b9a 
>   configs/example/ruby_mem_test.py 220fa4099b9a 
>   configs/example/ruby_random_test.py 220fa4099b9a 
>   configs/example/se.py 220fa4099b9a 
>   configs/ruby/GPU_RfO.py 220fa4099b9a 
>   configs/example/apu_se.py 220fa4099b9a 
>   configs/example/fs.py 220fa4099b9a 
>   configs/example/garnet_synth_traffic.py 220fa4099b9a 
> 
> Diff: http://reviews.gem5.org/r/3647/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Andreas Hansson
> 
>

_______________________________________________
gem5-dev mailing list
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev

Reply via email to