----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/2738/#review6301 -----------------------------------------------------------
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. src/mem/ruby/system/System.hh (line 78) <http://reviews.gem5.org/r/2738/#comment5437> You are returning the same variable from both the functions. - Nilay Vaish On April 15, 2015, 4:40 p.m., Joel Hestness wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/2738/ > ----------------------------------------------------------- > > (Updated April 15, 2015, 4:40 p.m.) > > > Review request for Default. > > > Repository: gem5 > > > Description > ------- > > Changeset 10788:2b38017adcb6 > --------------------------- > 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 3c6a78d23507 > src/mem/ruby/slicc_interface/AbstractController.hh 3c6a78d23507 > src/mem/ruby/slicc_interface/AbstractController.cc 3c6a78d23507 > src/mem/ruby/system/Sequencer.cc 3c6a78d23507 > src/mem/ruby/system/System.hh 3c6a78d23507 > src/mem/ruby/system/System.cc 3c6a78d23507 > > 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
