config: Break out base options for usage with NULL ISA
Review Request #3683 - Created Oct. 20, 2016 and submitted
Information | |
---|---|
Andreas Hansson | |
gem5 | |
default | |
Reviewers | |
Default | |
Changeset 11688:3b7d93ec1cf1 --------------------------- 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 ISA, and thus no 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.
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.
-
configs/common/Options.py (Diff revision 1) -
Weird. I guess I understand why this is needed, but it's super strange to have to import from the package this file is part of. Something else that should be solved someday, I guess.
Thanks for breaking out the configs. Seems only garnet_synth_traffic.py is modified to call Options.addBaseOptions(parser), does other testers need to be modified?
-
configs/common/FSConfig.py (Diff revision 1) -
Should it be "from common import PlatformConfig" here as well, just like in Options.py? This script might cause a problem if /gem5/configs/common is not added to path.
Description: |
|
|||||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Diff: |
Revision 2 (+87 -74) |
Ship It!