----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/2702/#review5954 -----------------------------------------------------------
src/mem/ruby/slicc_interface/AbstractController.hh <http://reviews.gem5.org/r/2702/#comment5200> 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. src/mem/ruby/slicc_interface/AbstractController.cc <http://reviews.gem5.org/r/2702/#comment5199> White space around parens should be consistent with rest of file. - Joel Hestness On March 20, 2015, 7:48 p.m., Lena Olson wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/2702/ > ----------------------------------------------------------- > > (Updated March 20, 2015, 7:48 p.m.) > > > Review request for Default. > > > Repository: gem5 > > > Description > ------- > > Changeset 10743:b643c607990a > --------------------------- > 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. This caused problems with timing > requests. 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
