----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/2627/#review5840 -----------------------------------------------------------
Ship it! LGTM, also. I don't think it should be addressed in this patch, but I'm making notes here as a vision for how I think we should proceed with these backing store and protocol developments: Really what we're capturing with the backing store option is whether a Ruby coherence protocol requires the backing store, but *not* that the RubySystem needs it for any fundamental reason. In other words, requiring a backing store is a property of a particular coherence protocol implementation rather than the overall RubySystem. We've noted previously that protocol implementations can require backing store if they are not known to correctly buffer data in the cache controllers, or if they implement some functionality that cannot be easily modeled without a backing store (e.g. slim GPU atomics). The last couple changes to backing stores are major steps in the right direction: we've reduced the backing store specifier to a single option, rather than the prior structure, which disaggregated the variable and required that the programmer set the option on each of the RubySequencers (previously 'access_phys_mem'). However, the ultimate solution *should* be that each coherence prot ocol implementation specifies whether it requires the backing store. To this end, I propose that the next step for development on this should be to create an abstract Python wrapper object for Ruby protocol implementations that embeds this "requires backing store" property as a variable. All Ruby protocols could descend from that wrapper object, and we could handle the protocol as a Python object in config files rather than using the ugly Python exec calling convention we have in Ruby.py currently. If the protocol requires the backing store, Ruby should check for the requirement (e.g. accessor function in the abstract Python protocol object) and set up the backing store without the need for any command line parameters. If a developer decides to validate a protocol as "data correct", s/he would simply set the "requires backing store" variable to False for that protocol when the validation is completed. - Joel Hestness On Feb. 5, 2015, 4:02 p.m., Jason Power wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/2627/ > ----------------------------------------------------------- > > (Updated Feb. 5, 2015, 4:02 p.m.) > > > Review request for Default. > > > Repository: gem5 > > > Description > ------- > > Changeset 10681:b2dc4aee1a0d > --------------------------- > Ruby: Update backing store option to propagate through to all RubyPorts > > Previously, the user would have to manually set access_backing_store=True > on all RubyPorts (Sequencers) in the config files. > Now, instead there is one global option that each RubyPort checks on > initialization. > > > Diffs > ----- > > configs/ruby/Ruby.py 7639c17357dc > src/mem/ruby/system/DMASequencer.hh 7639c17357dc > src/mem/ruby/system/DMASequencer.cc 7639c17357dc > src/mem/ruby/system/RubyPort.cc 7639c17357dc > src/mem/ruby/system/RubySystem.py 7639c17357dc > src/mem/ruby/system/Sequencer.py 7639c17357dc > src/mem/ruby/system/System.hh 7639c17357dc > src/mem/ruby/system/System.cc 7639c17357dc > > Diff: http://reviews.gem5.org/r/2627/diff/ > > > Testing > ------- > > > Thanks, > > Jason Power > > _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
