Nilay,
The basic problem here is that fs.py, se.py, and ruby_fs.py include a lot
of useful functionality that people don't want to duplicate. I don't think
asking users to maintain separate config files is the right answer.
Instead we need to make these config files more modular. Also we need to
avoid unnecessarily moving more unrelated config parameters into common
files. Jason has some ideas on how to do that and I think he'll post a
message soon on that topic.
The goal to share more configuration code with the regression tester is
great. However, I think we need to do that by sharing more modules between
the two, not moving more information into the few modules that already are.
Brad
-----Original Message-----
From: Nilay Vaish [mailto:[email protected]]
Sent: Friday, March 30, 2012 9:33 AM
To: Beckmann, Brad
Cc: Steve Reinhardt; gem5 Developer List; Gabe Black
Subject: RE: [gem5-dev] Review Request: Config: Change the way options
are added
Brad
* The functions for adding options introduced by the patch could have
been
placed in se.py and fs.py. I did not do that because ruby_fs.py and
fs.py have
options in common, and to avoid duplication those options were placed in
Options.py. And the options in se.py were also moved to Options.py so
that
a common file has all the options in it. We can always move these
functions
back in to their respective files.
* Ruby's options are being added separately via function calls, so I did
not see
any need to change it. I certainly feel that a lot of duplication exists
across
scripts in configs/ruby directory, but it would require some thinking on
how
to combine the scripts together.
* These configuration files are examples which can be used to write
config
files for a specific need. I don't know if the files in config/ change
more often
that those in src/. But assuming that's true, any patches for these
files that
users maintain will need to be rebased more often than other patches. I
suggest that users should maintain separate config files instead of
modifying
se/fs.py, so that they are not affected.
* One of the things that I felt is worth spending time on, is to make
regression tests use se/fs.py by setting options properly instead of
redoing
the work of se/fs.py. And I think it is with this aim that I posted the
patch.
--
Nilay
On Thu, 29 Mar 2012, Beckmann, Brad wrote:
The reason I protest to this change is because I feel it is a step in
the wrong direction. As Steve says, the biggest pain point for us to
maintain patches that work on top of gem5 is the configuration scripts.
This changeset would centralize several system specific options into
Option.py which will likely cause unnecessary conflicts when future
parameters are added and seems to be contrast to the desire for more
modularity. Furthermore, the changeset uses a confusing policy where
most system specific parameters are put in Options.py but Ruby
protocol options still remain in their respective py config files.
Finally, there are a few minor semantic changes in this changeset that
just don't seem really worth it.
Sorry to push back so much on this, but my recent experience with
rebasing config files have made me leery of these sorts of changes.
Brad
From: Steve Reinhardt [mailto:[email protected]]
Sent: Wednesday, March 28, 2012 5:13 PM
To: gem5 Developer List
Cc: Nilay Vaish; Beckmann, Brad; Gabe Black
Subject: Re: [gem5-dev] Review Request: Config: Change the way options
are added
On Wed, Mar 28, 2012 at 4:11 PM, Gabe Black
<[email protected]<mailto:[email protected]>> wrote:
I'm assuming Steve and Ali have already weighed the pros and cons which
is why they were ok with it going in.
Just to weigh in on this point... I approved it because it looked
reasonable (i.e., not unreasonable), but I did not do an extensive
cost/benefit analysis. I didn't really think about the impact on
existing scripts.
I will mention that config script modularity is a real problem we
have, and it would be nice to see steps toward making things more
modular (without having lots of duplicate code). I don't think this
change is a step in the wrong direction, but I'm not sure if it's a
step forward or just sideways.
Steve
_______________________________________________
gem5-dev mailing list
[email protected]
http://m5sim.org/mailman/listinfo/gem5-dev
