> On 2010-06-28 17:32:39, Nathan Binkert wrote: > > src/python/m5/simulate.py, line 55 > > <http://reviews.m5sim.org/r/26/diff/3/?file=611#file611line55> > > > > You say *posargs in this code, whereas we use *args everywhere else in > > the python code. (Python itself uses *args, so I'd suggest a change for > > consistency). > > I assume that the *args is for backward compat where people say > > instantiate(root). If so, perhaps you should document this. Actually, is > > it really even worth doing all of this? It's pretty trivial to update a > > script to support this and if people are updating to the latest M5, it's > > not a huge change to ask of them. We don't typically dirty the code with > > backward compat stuff.
Yea, I ended up whacking all of this about two csets later in another patch, so I'm guessing for most people the window of backward compatibility will be the few milliseconds between pulling those csets. So you're probably right, I should just not bother. > On 2010-06-28 17:32:39, Nathan Binkert wrote: > > src/sim/Root.py, line 37 > > <http://reviews.m5sim.org/r/26/diff/3/?file=613#file613line37> > > > > I don't understand why you overload both __new__ and __init__. Seems > > like everything should go in __new__. (i.e. why not just fatal() in > > __new__?) I think you might also need to pass **kwargs to > > SimObject.__new__. > > > > All this said, I'm not sure how __new__ interacts with a metaclass, so > > I could be wrong. Hmm, I think you're right, this design was because I was initially going to print a warning instead of failing on a second instantiation, but then I gave up on that and didn't rethink the design. The comment that's there discusses why I don't pass kwargs to SimObject.__new__... I agree it seems wrong, but that's the way it is. - Steve ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/26/#review52 ----------------------------------------------------------- On 2010-06-27 23:37:29, Steve Reinhardt wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.m5sim.org/r/26/ > ----------------------------------------------------------- > > (Updated 2010-06-27 23:37:29) > > > Review request for Default. > > > Summary > ------- > > sim: make Python Root object a singleton > Enforce that the Python Root SimObject is instantiated only once. > The C++ Root object already panics if more than one is created. > This change avoids the need to track what the root object > is, since it's available from Root._getInstance() (if it exists). > It's now redundant to have the user pass the root object > to functions like instantiate() (though for backwards compatibility > we continue to support this for now with a warning). > > > Diffs > ----- > > configs/common/Simulation.py 5044bb906d5a > configs/example/memtest-ruby.py 5044bb906d5a > configs/example/memtest.py 5044bb906d5a > configs/example/rubytest.py 5044bb906d5a > configs/splash2/cluster.py 5044bb906d5a > configs/splash2/run.py 5044bb906d5a > src/python/m5/simulate.py 5044bb906d5a > src/python/m5/util/__init__.py 5044bb906d5a > src/sim/Root.py 5044bb906d5a > tests/run.py 5044bb906d5a > > Diff: http://reviews.m5sim.org/r/26/diff > > > Testing > ------- > > > Thanks, > > Steve > > _______________________________________________ m5-dev mailing list m5-dev@m5sim.org http://m5sim.org/mailman/listinfo/m5-dev
