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


Thanks for your patch!

I think this config script is independant enough that you should consider not 
using the Options helper in configs/common. I would argue that it just creates 
confusion due to the large number of unused options. I would suggest that you 
make it as independent as possible of the functionality in config/common/ and 
make it completely stand alone instead. However, don't consider these comments 
to be blockers for submission, they are just nice-to-have improvements.


util/tlm/tlm_elastic.py (lines 41 - 44)
<http://reviews.gem5.org/r/3477/#comment7282>

    I'd suggest not using any of these components. You'll just end up creating 
a gazillion of options that you'll never use in the end.



util/tlm/tlm_elastic.py (line 96)
<http://reviews.gem5.org/r/3477/#comment7283>

    You already force options.cpu_type to 'trace', which means that you 
statically already know that your CPU will be the TraceCPU and the memory mode 
will be timing.



util/tlm/tlm_elastic.py (line 97)
<http://reviews.gem5.org/r/3477/#comment7285>

    You shouldn't need to care about this for the trace cpu.



util/tlm/tlm_elastic.py (lines 100 - 101)
<http://reviews.gem5.org/r/3477/#comment7284>

    cpu=TraceCPU(cpu_id=0),
    mem_mode='timing',



util/tlm/tlm_elastic.py (line 126)
<http://reviews.gem5.org/r/3477/#comment7286>

    This shouldn't be needed since you're not in FS mode (and you're simulating 
a trace CPU).


- Andreas Sandberg


On June 14, 2016, 10:06 p.m., Matthias Jung wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.gem5.org/r/3477/
> -----------------------------------------------------------
> 
> (Updated June 14, 2016, 10:06 p.m.)
> 
> 
> Review request for Default and Andreas Sandberg.
> 
> 
> Repository: gem5
> 
> 
> Description
> -------
> 
> This patch adds an example configuration for elastic trace playing into the
> SystemC world, similar to the already existing traffic generator example in
> /util/tlm.
> 
> 
> Diffs
> -----
> 
>   util/tlm/README fc247b9c42b6 
>   util/tlm/tlm_elastic.py PRE-CREATION 
> 
> Diff: http://reviews.gem5.org/r/3477/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Matthias Jung
> 
>

_______________________________________________
gem5-dev mailing list
[email protected]
http://m5sim.org/mailman/listinfo/gem5-dev

Reply via email to