> On May 4, 2014, 11:56 p.m., Steve Reinhardt wrote: > > src/python/m5/simulate.py, line 117 > > <http://reviews.gem5.org/r/2246/diff/1/?file=39626#file39626line117> > > > > for all these 'if hasattr()' changes (here and below, and in > > simulate.py): seems like it would be much cleaner to make sure whatever > > dummy objects are being inserted into the config tree support dummy > > versions of these calls rather than add all these distributed checks all > > over the place. Maybe create a special DummySimObject class or something? > > I don't fully understand the use case here or perhaps I could be more > > specific. > > Steve Reinhardt wrote: > BTW, is this change independent of the command-line parameter stuff? If > so, then it should be a separate changeset. > > Geoffrey Blake wrote: > Internally we had this discussion and it was deemed a worse maintenance > headache having a DummySimobject C++ class over being handled purely in the > python code. It can be a separate changeset as well > > Steve Reinhardt wrote: > You didn't invite me to your discussion ;). It seems to me like we've > established what the SimObject interface is, and we've already added dummy > versions of these methods to the base SimObject class so that we don't have > to put all these tests in, even though many derived classes don't implement > them. So a DummySimObject class ought to be pretty trivial, and putting in > all these tests now seems like a step backwards to me. > > As I said initially, I don't really understand the use case for these > "dummy objects" anyway, or how you can call simulate() when you don't have > all the objects in place yet, so maybe I'm missing something, but so far I > remain unconvinced. > > Geoffrey Blake wrote: > The dummy simobject is being used as a means to provide naming resolution > only. In the rewrite of the config scripts, it was a desired feature to have > functions that created whole subsystems that we could call multiple times to > instantiate the desired system: > > cluster1 = makeCpuAndCaches() > cluster2 = makeCpuAndCaches() > > SimObjects cluster1 and 2 would have identical sets of Simobjects as > children, but their final names would be unique (cluster1.cpu0.icache, > cluster2.cpu0.icache etc). In the end Objects cluster1 and cluster2 aren't > instantiated (I have forgotten how now), but the simulated system still works > correctly because we made sure to connect up all the ports properly. Making > an actual DummySimobject class is not hard, I did that in a previous > iteration.
My reasoning here was that we didn't really want to have a sim object that was instantiated by did nothing, wasn't connected to anything and only existed to provide containers for configuration. - Ali ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/2246/#review5082 ----------------------------------------------------------- On April 23, 2014, 12:22 p.m., Andreas Hansson wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/2246/ > ----------------------------------------------------------- > > (Updated April 23, 2014, 12:22 p.m.) > > > Review request for Default. > > > Repository: gem5 > > > Description > ------- > > Changeset 10198:f749128e94c8 > --------------------------- > config: Add hooks to enable new config sys > > This patch adds helper functions to SimObject.py, params.py and > simulate.py to enable the new configuration system. Functions like > enumerateParams() in SimObject lets the config system auto-generate > command line options for simobjects to be modified on the command > line. > > Params in params.py have __call__() added > to their definition to allow the argparse module to use them > as a type to check command input is in the proper format. > > simulate.py is changed to allow for empty simobjects to exist in the hierarchy > to allow a system to be built in parts to be stitched together later. > > > Diffs > ----- > > src/python/m5/SimObject.py e40b35147270 > src/python/m5/params.py e40b35147270 > src/python/m5/simulate.py e40b35147270 > src/python/m5/stats/__init__.py e40b35147270 > > Diff: http://reviews.gem5.org/r/2246/diff/ > > > Testing > ------- > > > Thanks, > > Andreas Hansson > > _______________________________________________ gem5-dev mailing list gem5-dev@gem5.org http://m5sim.org/mailman/listinfo/gem5-dev
