Hi Andreas, I think we're probably more in tune than these emails would indicate... I was just not clear enough about what I meant (perhaps even in my own head). I agree that users and model developers should think in terms of cycles and not ticks. The whole ClockedObject thing is a big step in supporting that, and the pattern of only using nextCycle() or clockEdge() or whatever we end up calling it when scheduling events is very appropriate. As an aside, and strictly for future consideration, I'd be fine with changing the scheduling API to make it even easier to use clock edges and harder to use ticks as units for scheduling events within ClockedObjects.
Of course, underneath that pretty clock-oriented facade, everything really does run on ticks ;-). My concern is that if we start accommodating or even promoting the use of Cycle as an *absolute* measure of time, then we're introducing confusion, since with DVFS you can't even convert a cycle counter to an actual time value. There may be situations where you want to count cycles, but we should be very clear that a cycle counter is not a measurement of absolute time. I guess what I'm trying to push is that we work hard to have domains in which we always consistently use cycles (e.g., scheduling events in ClockedObjects) and other domains in which we always consistently use ticks (e.g., stats that track absolute measurements of time), and make it easy to follow this pattern and hard to violate it. In my earlier email I was thinking that if we push this hard enough we might be able to avoid using fancy type tricks, though now that I've looked at the code some more, maybe the right answer is to do this in addition to some typing things like what has been proposed. A particular subtlety that is causing me some concern is that while it's valid to convert between relative cycles and relative ticks based on a specific clock period, it's invalid to do that with absolute cycle and tick values. I think the most significant issue is distinguishing between relative cycle-based delays/latencies and absolute cycle counts, since these are the things that could be inconsistent. Distinguishing between absolute and relative tick values is not as critical since they don't change (an absolute tick value really is just a relative number of ticks since time 0, which isn't true for cycles), plus it would be too hard to go fix up all the existing code. So based on these rambling thoughts, here's a proposal: - Let's create a uint64_t typedef called CycleCount (or maybe CycleCounter). - In ClockedObject, let's replace the current "Tick cycle" with "CycleCount cycleCount", and "curCycle()" with "curCycleCount()". - For internal latency parameters, let's keep them as ints, and by convention any latency parameter related to a ClockedObject should be in terms of cycles. I'm reluctant to create a Cycle typedef because you'd probably want it to be 64 bits just in case, but that's overkill for nearly all latency parameters which will typically be handfuls of cycles. - Let's change "tickToCycles()" (which is changed to "tickToCycle()" in Andreas's patch) to "ticksToCycles()", which I think is slightly more indicative of it being a relative conversion and not an absolute conversion. Actually it would be nice to get rid of it entirely, and I'm glad to see that Andreas's patch eliminates all but one use of it... unfortunately there is still one use remaining though, that at the very least does not appear totally trivial to eliminate. This is just a proposal... let me know what you think, Am I overthinking this? Steve On Mon, Jul 30, 2012 at 1:16 AM, Andreas Hansson <[email protected]>wrote: > Hi Steve, > > I am 100% with you on the notion of making a connection between clock (and > power state) changes and the collection of stats. A basic solution would be > to have a more flexible implementation of listeners for the stats that then > dump the relevant stats on frequency changes etc. There has been some work > done in our team along these lines and it would be good to discuss the > design in a bit more detail and understand what use-cases we have to > accommodate. > > When it comes to the cycles vs ticks, I would rather argue to deemphasise > ticks :). The cycles are used (and should be imho) for most scheduling of > events. There are very few objects that should have events that do not > coincide with clock edges, and having a more firm notion of clock edges > would definitely make it easier to have a sort of design pattern for how to > schedule events, I.e. either now (delta delay) or a cycle in the future. > > I am keen to explore the options here, but I'm inclined to hang on to the > cycles. Any thoughts or other ides on this topic? > > Andreas > > ________________________________________ > From: Steve Reinhardt [[email protected]] > Sent: Saturday, July 28, 2012 8:13 PM > To: gem5 Developer List > Cc: Andreas Hansson; Ali Saidi; Nathan Binkert > Subject: Re: [gem5-dev] Review Request: Clock: Rework clocks to avoid > tick-to-cycle transformations > > On Sat, Jul 28, 2012 at 10:30 AM, Ali Saidi <[email protected]<mailto: > [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 > > > -- IMPORTANT NOTICE: The contents of this email and any attachments are > confidential and may also be privileged. If you are not the intended > recipient, please notify the sender immediately and do not disclose the > contents to any other person, use it for any purpose, or store or copy the > information in any medium. Thank you. > > _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
