> On March 20, 2015, 9:15 p.m., Joel Hestness wrote: > > src/mem/ruby/slicc_interface/AbstractController.hh, line 209 > > <http://reviews.gem5.org/r/2702/diff/1/?file=44261#file44261line209> > > > > This doesn't seem like a good idea. This means that every generated > > controller will now have a pointer to the RubySystem, which may encourage > > later misuse of the system within the controllers. > > > > I think we should be moving in the other direction instead. > > Specifically, Ruby's warmup and cooldown processes are > > simulated-system-wide activities, NOT RubySystem instance-specific. In a > > single simulation, I could imagine there being multiple simulated systems > > with separate RubySystem instances, but warmup and cooldown activities may > > necessarily touch components of both RubySystems (e.g. a shared backing > > memory being flushed to). Thus, you can't have one instance warming > > up/cooling down, but the other instance isn't. It doesn't make sense to > > have separate variables for each instance. > > > > I'd strongly prefer that m_warmup_enabled and m_cooldown_enabled be > > made into static (class) variables of the RubySystem (this has bugged me > > for a long time). This would eliminate the need for RubySystem pointers in > > the abstract system, because these variables could be accessed statically > > just like the block size. It would also improve the encapsulation of the > > RubySystem object, which currently has these variables publicly-visible > > (yuck). > > > > This patch enables functionality that has never existed before, so I'd > > prefer that we fix the RubySystem variable visibility before pushing this > > patch.
I can totally understand why you want to fix the encapsulation, and if someone else wants to make that fix, I'm happy to make the minor changes to update this patch to work with it. I really just wanted to get this out there for anyone else who wants to restore ruby checkpoints using the default memory type, since I imagine I might not be the only one. - Lena ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/2702/#review5954 ----------------------------------------------------------- On March 21, 2015, 7:11 p.m., Lena Olson wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/2702/ > ----------------------------------------------------------- > > (Updated March 21, 2015, 7:11 p.m.) > > > Review request for Default. > > > Repository: gem5 > > > Description > ------- > > Changeset 10743:d3ead01d8747 > --------------------------- > ruby: allow restoring from checkpoint when using DRAMCtrl > > Restoring from a checkpoint with ruby + the DRAMCtrl memory model > was not working, because ruby and DRAMCtrl disagreed on the > current tick during warmup. Since there is no reason to do > timing requests during warmup, use functional requests instead. > > > Diffs > ----- > > src/mem/ruby/slicc_interface/AbstractController.hh 655ff3f6352d > src/mem/ruby/slicc_interface/AbstractController.cc 655ff3f6352d > > Diff: http://reviews.gem5.org/r/2702/diff/ > > > Testing > ------- > > Can now restore checkpoints in X86+MOESI hammer+DRAMCtrl. It also still > works with simple mem, which was working before. > > > Thanks, > > Lena Olson > > _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
