> On May 17, 2015, 8:46 a.m., Nilay Vaish wrote: > > Would it make sense to declare the two variables: warmup and cooldown as > > global > > instead of having them a static members of RubySystem? If we want to move > > towards > > having multiple ruby systems, I would prefer all static variables in ruby > > system > > be simply made global.
Right now, only RubySystems perform warmup and cooldown activity, so these should be static members of RubySystem. If other components add warmup/cooldown functionality, then it may make sense to move them to global scope, but that's not for this patch. > On May 17, 2015, 8:46 a.m., Nilay Vaish wrote: > > src/mem/ruby/system/System.hh, line 78 > > <http://reviews.gem5.org/r/2738/diff/1/?file=44478#file44478line78> > > > > You are returning the same variable from both the functions. Thanks for catching that. Fixed. - Joel ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/2738/#review6301 ----------------------------------------------------------- On May 18, 2015, 3:49 p.m., Joel Hestness wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/2738/ > ----------------------------------------------------------- > > (Updated May 18, 2015, 3:49 p.m.) > > > Review request for Default. > > > Repository: gem5 > > > Description > ------- > > Changeset 10832:e1ce7cc2abbd > --------------------------- > ruby: Fix RubySystem warm-up and cool-down scope > > The processes of warming up and cooling down Ruby caches are simulation-wide > processes, not just RubySystem instance-specific processes. Thus, the warm-up > and cool-down variables should be globally visible to any Ruby components > participating in either process. Make these variables static members and track > the warm-up and cool-down processes as appropriate. > > This patch also has two side benefits: > 1) It removes references to the RubySystem g_system_ptr, which are problematic > for allowing multiple RubySystem instances in a single simulation. Warmup and > cooldown variables being static (global) reduces the need for > instance-specific > dereferences through the RubySystem. > 2) From the AbstractController, it removes local RubySystem pointers, which > are > used inconsistently with other uses of the RubySystem: 11 other uses reference > the RubySystem with the g_system_ptr. Only sequencers have local pointers. > > > Diffs > ----- > > src/mem/ruby/network/MessageBuffer.cc fbdaa08aaa42 > src/mem/ruby/slicc_interface/AbstractController.hh fbdaa08aaa42 > src/mem/ruby/slicc_interface/AbstractController.cc fbdaa08aaa42 > src/mem/ruby/system/Sequencer.cc fbdaa08aaa42 > src/mem/ruby/system/System.hh fbdaa08aaa42 > src/mem/ruby/system/System.cc fbdaa08aaa42 > > Diff: http://reviews.gem5.org/r/2738/diff/ > > > Testing > ------- > > 1) Patience when hearing statements of the form "if you'd like something done > (an easy and better way), do it yourself". The review process is meant to > avoid making other people fix your changes. > 2) Regressions pass > 3) Checkpoint restore into Ruby works with all memory controllers (used > MI_example and MOESI_hammer) > > > Thanks, > > Joel Hestness > > _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
