> 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. > > Lena Olson wrote: > 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.
After some more digging, all generated controllers have access to the RubySystem with the globally-visible g_system_ptr. If you're not willing to investigate moving the warmup/cooldown variables around, you need to at least use g_system_ptr instead of adding this pointer. - Joel ----------------------------------------------------------- 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
