> On 2011-05-03 17:45:41, Brad Beckmann wrote: > > Can we change the name of Time in base/time.hh instead of Time in Ruby? > > Right now this patch touches 50+ Ruby files and a bunch of lines within > > those files just to change Time to RTime. It seems that far fewer changes > > would be required to change the name of base/time.hh's version of Time. > > Nathan Binkert wrote: > While I wouldn't be opposed to changing the time in base/time.hh if we > needed both, but don't we need to move ruby away from its own version of Time > anyway? Shouldn't we be using Tick? > > Also, we've generally never shied away from changes that can be done with > a one line sed/perl script. > > Gabe Black wrote: > I was about to say something along these lines. This would be a good > chance to consolidate Ruby and M5 systems a little. It might be a good idea > to do the simple find/replace change on its own so the non-simple > non-find/replace stuff doesn't get lost as noise. I don't have deep feelings > one way or the other, though. > > Korey Sewell wrote: > This should be two diffs: 1 for rename and 1 for stat code. But since > they are related to getting dumpstats working I combined them both for the > sake of discussion. > > Time isnt equivalent Tick or I would've happily made that change :). > > Time is really the Ruby Cycle count so if you change it to Tick then all > over the place you would have to convert to cycles when making comparisons > for request latencies and various parameters in Ruby. In the interim, maybe > changing Time to Cycle would work. > > Alternatively, you could change Time to Tick, and then force all the > latencies throughout Ruby to be expressed in Ticks. This would have the nice > effect of setting the latencies for Caches and memory objects with a real > time (e.g. "1ns") instead of relative time (e.g. 1 cycle). > > Brad Beckmann wrote: > Eventually, we absolutely need Ruby to move away from its own version of > time and we are in the process to do just that. Right now both Nilay and > Somayeh are working on two pretty complicated and substantial changes that > are on that path. In particular, once Ruby supports functional accesses, > then it can supply data directly to the CPUs. Once Ruby data is functional > memory, we can create cache warmup traces that include valid data. Once we > have a cache warmup methodology that includes data and works more seamlessly > with unserialization, we can remove the Ruby event queue APIs. Once we do > that, we can get rid of Ruby's notion of time...As you can tell, this is a > long process and we still have a ways to go, but that is the plan. > > My hesitation with this patch is that it seems premature and it adds more > rebase/merging work for those of us who have other patches under development > that really need touch these same files and lines. > > Overall, does this patch need to be checked in right now? For instance, > do we need it to move Ruby stats to M5 stats or will moving to M5 stats > negate having to make this change? >
Seems like a reasonable objection. I've generally had success with running the search/replace routines directly on my patches when this sort of change happens, but if that doesn't work, then I wouldn't be opposed to renaming Time to TimeOfDay or RealTime or something like that. I would like to make sure that Korey can continue working. - Nathan ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/675/#review1195 ----------------------------------------------------------- On 2011-05-03 11:20:58, Korey Sewell wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.m5sim.org/r/675/ > ----------------------------------------------------------- > > (Updated 2011-05-03 11:20:58) > > > Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and > Nathan Binkert. > > > Summary > ------- > > ruby-stats: support for dump_stats instruction > *** > NOTE: The core changes for this diff are in Profiler.cc/hh, > stat_control.hh/cc, and pseudo_inst.cc > **** > This is a first pass toward getting dump-stats functionality to work > "cleanly" for Ruby. As is, the patch works, but there needs to be discussion > over: > - Changing Ruby typedef "Time" to "RTime" because it conflicts with the Time > class defined in base/time.hh (The majority of files are renames)... Is there > a better name than "RTime"? > > - Where is the right place for this RubyStatEvent code? I hesitated to do any > real cosmetic changes because of what impending stat changes might do. I have > two thoughts: > (1) If Ruby Stats will be registered like old M5 stats, then this code would > nicely fold into the old "statEvent" code in sim_control.cc. "Fold into" > maybe too strong of a phrase even, as most of it should just naturally "work". > (2) If Ruby Stats are not registered, then maybe placing this code into the > RubySystem class. I realized late that the "Profiler" and "Network" have two > different stats that they track so the RubySystem would be the right place > along with calling the namespace "RubyStats". > > > Diffs > ----- > > SConstruct 3f49ed206f46 > src/cpu/testers/rubytest/RubyTester.hh 3f49ed206f46 > src/cpu/testers/rubytest/RubyTester.cc 3f49ed206f46 > src/mem/ruby/buffers/MessageBuffer.hh 3f49ed206f46 > src/mem/ruby/buffers/MessageBuffer.cc 3f49ed206f46 > src/mem/ruby/buffers/MessageBufferNode.hh 3f49ed206f46 > src/mem/ruby/common/Consumer.hh 3f49ed206f46 > src/mem/ruby/common/Global.hh 3f49ed206f46 > src/mem/ruby/common/TypeDefines.hh 3f49ed206f46 > src/mem/ruby/eventqueue/RubyEventQueue.hh 3f49ed206f46 > src/mem/ruby/eventqueue/RubyEventQueue.cc 3f49ed206f46 > src/mem/ruby/network/garnet/fixed-pipeline/GarnetNetwork_d.hh 3f49ed206f46 > src/mem/ruby/network/garnet/fixed-pipeline/GarnetNetwork_d.cc 3f49ed206f46 > src/mem/ruby/network/garnet/fixed-pipeline/InputUnit_d.hh 3f49ed206f46 > src/mem/ruby/network/garnet/fixed-pipeline/NetworkInterface_d.hh > 3f49ed206f46 > src/mem/ruby/network/garnet/fixed-pipeline/OutVcState_d.hh 3f49ed206f46 > src/mem/ruby/network/garnet/fixed-pipeline/VirtualChannel_d.hh 3f49ed206f46 > src/mem/ruby/network/garnet/fixed-pipeline/flit_d.hh 3f49ed206f46 > src/mem/ruby/network/garnet/flexible-pipeline/FlexibleConsumer.hh > 3f49ed206f46 > src/mem/ruby/network/garnet/flexible-pipeline/GarnetNetwork.hh 3f49ed206f46 > src/mem/ruby/network/garnet/flexible-pipeline/GarnetNetwork.cc 3f49ed206f46 > src/mem/ruby/network/garnet/flexible-pipeline/InVcState.hh 3f49ed206f46 > src/mem/ruby/network/garnet/flexible-pipeline/InVcState.cc 3f49ed206f46 > src/mem/ruby/network/garnet/flexible-pipeline/NetworkInterface.hh > 3f49ed206f46 > src/mem/ruby/network/garnet/flexible-pipeline/NetworkInterface.cc > 3f49ed206f46 > src/mem/ruby/network/garnet/flexible-pipeline/NetworkLink.hh 3f49ed206f46 > src/mem/ruby/network/garnet/flexible-pipeline/NetworkLink.cc 3f49ed206f46 > src/mem/ruby/network/garnet/flexible-pipeline/OutVcState.hh 3f49ed206f46 > src/mem/ruby/network/garnet/flexible-pipeline/OutVcState.cc 3f49ed206f46 > src/mem/ruby/network/garnet/flexible-pipeline/Router.hh 3f49ed206f46 > src/mem/ruby/network/garnet/flexible-pipeline/Router.cc 3f49ed206f46 > src/mem/ruby/network/garnet/flexible-pipeline/flit.hh 3f49ed206f46 > src/mem/ruby/network/garnet/flexible-pipeline/flit.cc 3f49ed206f46 > src/mem/ruby/network/simple/Throttle.hh 3f49ed206f46 > src/mem/ruby/profiler/Profiler.hh 3f49ed206f46 > src/mem/ruby/profiler/Profiler.cc 3f49ed206f46 > src/mem/ruby/profiler/StoreTrace.hh 3f49ed206f46 > src/mem/ruby/profiler/StoreTrace.cc 3f49ed206f46 > src/mem/ruby/recorder/CacheRecorder.hh 3f49ed206f46 > src/mem/ruby/recorder/CacheRecorder.cc 3f49ed206f46 > src/mem/ruby/recorder/TraceRecord.hh 3f49ed206f46 > src/mem/ruby/recorder/TraceRecord.cc 3f49ed206f46 > src/mem/ruby/recorder/Tracer.hh 3f49ed206f46 > src/mem/ruby/recorder/Tracer.cc 3f49ed206f46 > src/mem/ruby/slicc_interface/AbstractCacheEntry.hh 3f49ed206f46 > src/mem/ruby/slicc_interface/Message.hh 3f49ed206f46 > src/mem/ruby/slicc_interface/RubySlicc_Util.hh 3f49ed206f46 > src/mem/ruby/system/AbstractReplacementPolicy.hh 3f49ed206f46 > src/mem/ruby/system/LRUPolicy.hh 3f49ed206f46 > src/mem/ruby/system/MemoryControl.cc 3f49ed206f46 > src/mem/ruby/system/MemoryNode.hh 3f49ed206f46 > src/mem/ruby/system/PseudoLRUPolicy.hh 3f49ed206f46 > src/mem/ruby/system/Sequencer.hh 3f49ed206f46 > src/mem/ruby/system/Sequencer.cc 3f49ed206f46 > src/mem/ruby/system/System.hh 3f49ed206f46 > src/mem/ruby/system/System.cc 3f49ed206f46 > src/mem/ruby/system/TimerTable.hh 3f49ed206f46 > src/mem/ruby/system/TimerTable.cc 3f49ed206f46 > src/mem/ruby/system/WireBuffer.cc 3f49ed206f46 > src/sim/pseudo_inst.cc 3f49ed206f46 > src/sim/stat_control.hh 3f49ed206f46 > src/sim/stat_control.cc 3f49ed206f46 > > Diff: http://reviews.m5sim.org/r/675/diff > > > Testing > ------- > > > Thanks, > > Korey > > _______________________________________________ m5-dev mailing list m5-dev@m5sim.org http://m5sim.org/mailman/listinfo/m5-dev
