> On Oct. 20, 2016, 2:11 p.m., Jason Lowe-Power wrote:
> > This is fine with me. However, I think this is just bandaging a broken 
> > system. IMO, we need to greatly simplify (maybe even totally remove) the 
> > command-line parameters in config/common.
> > 
> > What I've been telling people is this: gem5's interface is not the command 
> > line: it is Python scripts. You shouldn't think about "what options should 
> > I pass to gem5" when you are using it, you should instead say "what Python 
> > script should I write". 
> > 
> > My opinion is our scripts in configs/ should do two things:
> >  1. Break out commonalities between all Python configuration scripts. 
> > Things like DDR3 memory, basic cache coherence protocols, specific ARM CPU 
> > configurations, full system setup BS, etc.
> >  2. Give some basic examples of how to use Python configuration scripts, 
> > and give some starting points for new users to pick up gem5.
> > 
> > Sorry, this is off-topic for this patch, but it's something that's been 
> > bothering me for years now. This idea that many of our users have that you 
> > can just call gem5 with the "right" command line paramters to simulate what 
> > you need is harmful. It's harmful both to gem5 development as it makes 
> > these config scripts less maintainable (e.g., Ruby's scripts), and, in the 
> > worst case, produces bad architecture research since users can ignore 
> > important parameters.
> > 
> > /rant.
> > 
> > This isn't really a review of this patch, but I just wanted to say I don't 
> > think it's worth much of our time to try to bandage the current 
> > "Options.py" configuration style. Though, I'm not volunteering to fix this 
> > either. BTW, I think the ARM config scripts in configs/examples/arm are a 
> > step in the right direction, though I think they could be taken even 
> > further to make them cleaner and more maintainable.
> > 
> > Either way, thanks for making these changes, Andreas. We certainly can't 
> > have things that are generating errors for our users in the short term.

I completely agree that this is bandaid. I merely wanted to make sure things 
were somewhat less broken again and that people could use the existing scripts 
without things blowing up.


- Andreas


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


On Oct. 20, 2016, 11:07 a.m., Andreas Hansson wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.gem5.org/r/3683/
> -----------------------------------------------------------
> 
> (Updated Oct. 20, 2016, 11:07 a.m.)
> 
> 
> Review request for Default.
> 
> 
> Repository: gem5
> 
> 
> Description
> -------
> 
> Changeset 11688:60b7de6442f3
> ---------------------------
> config: Break out base options for usage with NULL ISA
> 
> This patch breaks out the most basic configuration options into a set
> of base options, to allow them to be used also by scripts that do not
> involve any actual CPUs or devices. The patch also fixes a few modules
> so that they can be imported in a NULL build, and avoid dragging in
> FSConfig every time Options is imported.
> 
> 
> Diffs
> -----
> 
>   configs/common/CpuConfig.py b3d5f0e9e258 
>   configs/common/FSConfig.py b3d5f0e9e258 
>   configs/common/Options.py b3d5f0e9e258 
>   configs/common/PlatformConfig.py b3d5f0e9e258 
>   configs/example/garnet_synth_traffic.py b3d5f0e9e258 
> 
> Diff: http://reviews.gem5.org/r/3683/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Andreas Hansson
> 
>

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

Reply via email to