Review Board 2.0.15


cache: enable multiple stores per cycle

Review Request #2273 - Created May 13, 2014 and updated

Information
Steve Reinhardt
gem5
default
Reviewers
Default
Changeset 10225:cf108c6a0aa0
---------------------------
cache: enable multiple stores per cycle

Fix cases when we have back to back stores, and 2nd store cannot be
sent to cache because ACK for 1st store is sent at weird Tick:
curTick() + 1, which effectively schedule the ACK to be processed in
the next cycle.

With this change, a potential live lock is possible: When MSHR file is
full, but prefetcher tells us that it has some data. This data should
only be processed when mshrQueue is NOT full.  Otherwise, we may have
a livelock due to event sent from the prefetcher prevents the cache to
be drained out.

This work was done while Binh was an intern at AMD Research.

   
Ship it!
Posted (May 14, 2014, 7:55 a.m.)
This looks fine to me, but I'm going to guess Andreas will have a comment. This patch fixes the lovelock you mentioned, correct?
  1. Yea, getting rid of the '+ 1' in PacketQueue::scheduleSend is needed to do two stores in one cycle, an the change in the prefetcher test fixes the livelock that that induces.
    
    Unfortunately this patch was developed a while ago, so the specifics are a bit fuzzy... I don't know if the challenge to doing two stores in one cycle is specific to x86/TSO or if that occurs with any ISA.
  2. I'm not overly thrilled that we are removing all notions of delay in the ports (and it makes the notion of 4-phase more challenging). The +1 was a poor-mans version of a delta delay, and acts as a form of safety check in terms of causality. As gem5 does not have update/notify (in general) I don't see any other way besides the +1. Removing it all together essentially opens up for infinite throughput. Is that really what we want?
    
    Is this a "lazy" way of avoiding having to code up an FSM on the CPU side to deal with the intermediate states?