----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/3017/#review6983 -----------------------------------------------------------
I'm afraid I don't agree with the direction of this patch for a couple major reasons: 1) It essentially adds back the cntrl_ids that were used previously. cntrl_ids were notoriously buggy due to their Python flexibility and C++ redundancy with other numbering schemes. Redundancy in numbering often makes it tricky to know which numbering system to base code on, and the bugs that can be introduced using the incorrect number system are often silent (e.g. incorrectly connecting a network topology gives strange routes, but can still function). The Python node params introduced in this patch provide nearly identical possibility for bugs as the old cntrl_ids, and in general, flat numbering adds ambiguity about which/how numbering should be used. 2) A Python script programmer shouldn't need to handle these IDs. Ruby's internal machine numbering currently handles numbering for single system simulations, and it provides consistent numbering across all protocols (unlike cntrl_ids/node parameters, which can easily be ordered incorrectly in some Python protocol config file). What needs to be changed for multi-system simulations is to make Ruby's numbering non-global, so it can internally track IDs for multiple systems. This might involve changing Python code to get the IDs before C++ SimObject instantiation, but we shouldn't need to expose more numbering in the Python configs. - Joel Hestness On Aug. 9, 2015, 6:28 p.m., Nilay Vaish wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/3017/ > ----------------------------------------------------------- > > (Updated Aug. 9, 2015, 6:28 p.m.) > > > Review request for Default. > > > Repository: gem5 > > > Description > ------- > > Changeset 11059:f95d2c4947c3 > --------------------------- > ruby: provide flat ids to controllers > > * Currently ruby internally computes a flat id for each controller. This id > is > the sum of the all the controllers that precede this controller due to their > machine types or are of same machine type but better version. This moves this > computation to the python configuration code so as to expose it to the config > files. > > * This also means that we no longer need to have static counter variables > associated with each controller class. Hence, the functions related to counts > of machines are being dropped. > > * We can now declare multiple ruby systems and on-chip networks, each of them > can function independently, without any knowledge of others. > > * This patch also paves the way for compiling all the protocols together. > Subsequent patches will enable this. > > > Diffs > ----- > > src/mem/protocol/MOESI_CMP_token-dir.sm 863d314f6356 > src/mem/protocol/MOESI_hammer-dir.sm 863d314f6356 > src/mem/protocol/RubySlicc_Types.sm 863d314f6356 > src/mem/ruby/common/NetDest.hh 863d314f6356 > src/mem/ruby/common/NetDest.cc 863d314f6356 > src/mem/ruby/network/Network.cc 863d314f6356 > src/mem/ruby/network/Topology.cc 863d314f6356 > src/mem/ruby/network/garnet/fixed-pipeline/NetworkInterface_d.cc > 863d314f6356 > src/mem/ruby/network/garnet/flexible-pipeline/NetworkInterface.cc > 863d314f6356 > src/mem/ruby/slicc_interface/AbstractController.hh 863d314f6356 > src/mem/ruby/slicc_interface/AbstractController.cc 863d314f6356 > src/mem/ruby/slicc_interface/Controller.py 863d314f6356 > src/mem/slicc/symbols/StateMachine.py 863d314f6356 > src/mem/slicc/symbols/Type.py 863d314f6356 > configs/ruby/MESI_Three_Level.py 863d314f6356 > configs/ruby/MESI_Two_Level.py 863d314f6356 > configs/ruby/MI_example.py 863d314f6356 > configs/ruby/MOESI_CMP_directory.py 863d314f6356 > configs/ruby/MOESI_CMP_token.py 863d314f6356 > configs/ruby/MOESI_hammer.py 863d314f6356 > configs/ruby/Network_test.py 863d314f6356 > src/mem/protocol/MESI_Three_Level-L0cache.sm 863d314f6356 > src/mem/protocol/MESI_Three_Level-L1cache.sm 863d314f6356 > src/mem/protocol/MOESI_CMP_token-L1cache.sm 863d314f6356 > src/mem/protocol/MOESI_CMP_token-L2cache.sm 863d314f6356 > > Diff: http://reviews.gem5.org/r/3017/diff/ > > > Testing > ------- > > > Thanks, > > Nilay Vaish > > _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
