On Sat, Jul 28, 2012 at 10:30 AM, Ali Saidi <[email protected]> wrote: > > > > On July 27, 2012, 6:50 a.m., Ali Saidi wrote: > > > this sort of thing worries me because of the possibilities of > introducing subtle bugs, but it looks like a good change long term. > > > > Andreas Hansson wrote: > > Agreed. I think for DVFS it is practically a must as the fixed > relation between cycles and ticks will no longer be true. Ultimately it > would be a blessing if C++11 units could help out, but that is probably > quite a few years away :) > > > > The Cycle typedef should be a good starting point to at least > indicate what the intention is. In a future patch I will also change the > name of the ClockedObject::clock member to period to try and avoid > confusion. > > > > Ali Saidi wrote: > > i guess a third option would be to wrap these things in a struct so > that we can get better type checking. > > > > > > Nathan Binkert wrote: > > what part worries you? > > > > Ali Saidi wrote: > > someone will assign ticks = clocks. > > > > Andreas Hansson wrote: > > I can: > > > > 1) Add the Cycle typedef to at least enable the code to "document > itself" in what it is expecting. The compiler will obviously not check for > you. > > 2) Create a wrapper class for either one or both Tick and Cycle with > an explicit constructor and a uint casting operator. > > 3) Leave this patch as is for now. > > > > Preferences? > > > > In all cases...fix it properly the day c++11 is a minimum build > requirement. > > > > Once again thanks to Nate for getting this patch going :) > > I think (1) is the minimum, and (2) if you want bonus points ;) > > Anyone else have an opinion?
I'd like to explore a different option, which is to de-emphasize 'cycles' as a unit of time. Looking at the current patch, it appears that the only places we really use the notion of cycles as a unit (or I think equivalently care about what the current "cycle number" is) is for some statistics tracking (like number of idle cycles), and we're not particularly consistent about that (like lastRunningCycle in InOrder, which in spite of the name was tracking ticks rather than cycles). Given that we currently don't do any frequency scaling, if the short-term concern is only statistics compatibility, then we could move the stats that are actually currently tracked in cycles to being tracked in terms of ticks, followed by a single divide by the clock period when stats are dumped. Of course we won't be able to do that with frequency scaling, but in that case, do we really want these stats reported in cycles (when we won't really know what that means) or would we want them reported in absolute time? Or perhaps both? Even if we want them computed in terms of cycles, it might be more efficient to maintain counters in terms of ticks, then accumulate to cycle-based stats when the frequency changes (using the old frequency as the divider), then do a final accumulation when stats are dumped. This strategy would also work for non-stats things like a non-constant TSC; I'm sure that RDTSC is called rarely enough to make fixing up a value periodically cheaper than keeping it constantly correct. We could keep this relatively painless by adding this mechanism to the base ClockedObject; for example, we could keep a list of callbacks that need to happen whenever the frequency is changed, and have a stats base class like CycleCounter that automatically registers the appropriate callback. We may well end up needing this callback system regardless of how we handle this cycle issue, so I don't think it's necessarily extra overhead. Maybe this is too reactionary, but I think it has two advantages: - it keeps ClockedObject simpler because we only need to add the 'tick' var and not 'cycle' - if we de-emphasize the use of cycles as a unit, then we don't have to worry so much about people getting them confused with ticks. Thoughts? Steve _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
