----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/1791/#review4155 -----------------------------------------------------------
Looks very sensible. I assume we also have to bump the stats for all the regressions? My only request would be to make the description more descriptive, e.g include MSHR, response latency perhaps. - Andreas Hansson On March 23, 2013, 2:55 p.m., Mitch Hayenga wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/1791/ > ----------------------------------------------------------- > > (Updated March 23, 2013, 2:55 p.m.) > > > Review request for Default. > > > Description > ------- > > Fixes a cache latency calculation bug. > > The classic memory system can improperly calculate response latency under > certain circumstances. When a cache miss to a lower level occurs, the > response time to packets in the MSRH is computed by as follows: > > completion_time = curTick() + responseLatency * clockPeriod() + > (transfer_offset ? pkt->busLastWordDelay : > pkt->busFirstWordDelay); > > However, the "whenReady" field of the cacheblock is only set to: > blk->whenReady = curTick() + pkt->busLastWordDelay; > > This expression lacks the responseLatency component. This means younger > accesses that occur on (or after) the cycle of the fill from the lower level > cache can be observed sooner than they should be. Additionally, and how I > found this, it means multiple senior stores dispatched in order by the CPU to > the *same* address may be acknowledged out of order. This can break some > CPUs' LSQ implementations that assume stores issued in program order (to the > same address) will be acknowledged in the order they were dispatched to the > memory system. > > Note #1: It seems the non-LRU caches/tags completely ignore the whenReady > field. Those will be fixed in a later patch. > Note #2: I don't have commit access, someone else will have to push this. > > > Diffs > ----- > > src/mem/cache/cache_impl.hh 0a4b702628bd > > Diff: http://reviews.gem5.org/r/1791/diff/ > > > Testing > ------- > > 1 billion instructions on spec2k6's gcc across various simpoints on an out of > order cpu. > > > Thanks, > > Mitch Hayenga > > _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
