I think that, as you said, broke other topologies, let alone the gem5-gpu. The original fix didn't let MeshDirCorners run. Is that not the case anymore?
Nilay's most recent response is still not clear to me: If we don't modify Simulation.py or fs/se.py or Ruby.py, then what options are there? Some parameter check has to come from one of those files. The entire system is configured and instantiated from those 3 files, If not one of those 3, then which one? Reverting to the original diff would require more changes to handle the other topologies, but it won't be in a direction that removes topology-specific checks, so its not clear if doing that would be useful, or resolve the concern that was raised. On Mon, Apr 21, 2014 at 10:06 AM, Jason Power <[email protected]> wrote: > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/2224/ > > On April 10th, 2014, 4:58 p.m. UTC, *Nilay Vaish* wrote: > > > configs/ruby/Ruby.py<http://reviews.gem5.org/r/2224/diff/2/?file=39527#file39527line102> > (Diff > revision 2) > > def create_topology(controllers, options): > > 102 > > if options.topology == "Cluster": > > I am not in favour of adding such ad hoc checks to the config file. > > On April 15th, 2014, 3:13 a.m. UTC, *Malek Musleh* wrote: > > I am not quite sure what you mean. There are other instances in the config > scripts where string comparisons are done, so there is a long precedent for > that (including a patch that was recently committed). Do you prefer doing > something like if type(topology) == Cluster? If so, I tried that initially, > but it required adding additional imports to the top of Ruby.py, and created > some other complications that prevents it from running. > > I think a solution is to go back to your first diff and change the line in > Cluster.py to: > __init__(self, controllers=[],... > > > - Jason > > On April 10th, 2014, 4:17 p.m. UTC, Malek Musleh wrote: > Review request for Default. > By Malek Musleh. > > *Updated April 10, 2014, 4:17 p.m.* > *Repository: * gem5 > Description > > Changeset 10158:97c7a8d9ea1e > --------------------------- > ruby: cluster topology config fix > > This patch adds a missing member function, and updates the ruby config file > to include the list of controllers for the Cluster topology. Previously the > list of controllers was not being passed to the topology. > > Diffs > > - configs/ruby/Ruby.py (5c2ecad1a3c9) > - configs/topologies/Cluster.py (5c2ecad1a3c9) > - src/mem/ruby/slicc_interface/AbstractController.hh (5c2ecad1a3c9) > > View Diff <http://reviews.gem5.org/r/2224/diff/> > _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
