From 82e2a3557672864f0ea3ae64dad61681546aaf07 Mon Sep 17 00:00:00 2001 From: Steve Reinhardt Date: Sun, 22 Jul 2007 21:43:38 -0700 Subject: [PATCH] Replace lowerMSHRPending flag with more robust scheme based on following Packet senderState links. --HG-- extra : convert_revision : 9027d59bd7242aa0e4275bf94d8b1fb27bd59d79 --- src/mem/cache/cache.hh | 3 +- src/mem/cache/cache_impl.hh | 22 +++------------ src/mem/cache/miss/mshr.cc | 48 ++++++++++++++++++++++++++++++-- src/mem/cache/miss/mshr.hh | 5 ++++ src/mem/cache/miss/mshr_queue.cc | 16 +++-------- src/mem/packet.hh | 3 -- 6 files changed, 60 insertions(+), 37 deletions(-) diff --git a/src/mem/cache/cache.hh b/src/mem/cache/cache.hh index 7dfe9e8f1..57028a05e 100644 --- a/src/mem/cache/cache.hh +++ b/src/mem/cache/cache.hh @@ -190,8 +190,7 @@ class Cache : public BaseCache * @param new_state The new coherence state for the block. */ void handleSnoop(PacketPtr ptk, BlkType *blk, - bool is_timing, bool is_deferred, - bool lower_mshr_pending); + bool is_timing, bool is_deferred); /** * Create a writeback request for the given block. diff --git a/src/mem/cache/cache_impl.hh b/src/mem/cache/cache_impl.hh index 82410afe1..efd7f4588 100644 --- a/src/mem/cache/cache_impl.hh +++ b/src/mem/cache/cache_impl.hh @@ -754,7 +754,7 @@ Cache::handleResponse(PacketPtr pkt) } else { // response to snoop request DPRINTF(Cache, "processing deferred snoop...\n"); - handleSnoop(target->pkt, blk, true, true, false); + handleSnoop(target->pkt, blk, true, true); } mshr->popTarget(); @@ -917,8 +917,7 @@ Cache::doTimingSupplyResponse(PacketPtr req_pkt, template void Cache::handleSnoop(PacketPtr pkt, BlkType *blk, - bool is_timing, bool is_deferred, - bool lower_mshr_pending) + bool is_timing, bool is_deferred) { assert(pkt->isRequest()); @@ -930,9 +929,6 @@ Cache::handleSnoop(PacketPtr pkt, BlkType *blk, if (is_timing) { Packet *snoopPkt = new Packet(pkt, true); // clear flags snoopPkt->setExpressSnoop(); - if (lower_mshr_pending) { - snoopPkt->setLowerMSHRPending(); - } snoopPkt->senderState = new ForwardResponseRecord(pkt, this); cpuSidePort->sendTiming(snoopPkt); if (snoopPkt->memInhibitAsserted()) { @@ -1019,16 +1015,6 @@ Cache::snoopTiming(PacketPtr pkt) Addr blk_addr = pkt->getAddr() & ~(Addr(blkSize-1)); MSHR *mshr = mshrQueue.findMatch(blk_addr); - // If a lower cache has an operation on this block pending (not - // yet in service) on the MSHR, then the upper caches need to know - // about it, as this means that the pending operation logically - // succeeds the current snoop. It's not sufficient to record - // whether the MSHR *is* in service, as this misses the window - // where the lower cache has completed the request and the - // response is on its way back up the hierarchy. - bool lower_mshr_pending = - (mshr && (!mshr->inService) || pkt->lowerMSHRPending()); - // Let the MSHR itself track the snoop and decide whether we want // to go ahead and do the regular cache snoop if (mshr && mshr->handleSnoop(pkt, order++)) { @@ -1075,7 +1061,7 @@ Cache::snoopTiming(PacketPtr pkt) } } - handleSnoop(pkt, blk, true, false, lower_mshr_pending); + handleSnoop(pkt, blk, true, false); } @@ -1090,7 +1076,7 @@ Cache::snoopAtomic(PacketPtr pkt) } BlkType *blk = tags->findBlock(pkt->getAddr()); - handleSnoop(pkt, blk, false, false, false); + handleSnoop(pkt, blk, false, false); return hitLatency; } diff --git a/src/mem/cache/miss/mshr.cc b/src/mem/cache/miss/mshr.cc index b9dfdf729..9b05aea3f 100644 --- a/src/mem/cache/miss/mshr.cc +++ b/src/mem/cache/miss/mshr.cc @@ -63,7 +63,8 @@ MSHR::TargetList::TargetList() inline void -MSHR::TargetList::add(PacketPtr pkt, Tick readyTime, Counter order, bool cpuSide) +MSHR::TargetList::add(PacketPtr pkt, Tick readyTime, + Counter order, bool cpuSide) { if (cpuSide) { if (pkt->needsExclusive()) { @@ -73,6 +74,12 @@ MSHR::TargetList::add(PacketPtr pkt, Tick readyTime, Counter order, bool cpuSide if (pkt->cmd == MemCmd::UpgradeReq) { hasUpgrade = true; } + + MSHR *mshr = dynamic_cast(pkt->senderState); + if (mshr != NULL) { + assert(!mshr->downstreamPending); + mshr->downstreamPending = true; + } } push_back(Target(pkt, readyTime, order, cpuSide)); @@ -97,6 +104,20 @@ MSHR::TargetList::replaceUpgrades() } +void +MSHR::TargetList::clearDownstreamPending() +{ + Iterator end_i = end(); + for (Iterator i = begin(); i != end_i; ++i) { + MSHR *mshr = dynamic_cast(i->pkt->senderState); + if (mshr != NULL) { + assert(mshr->downstreamPending); + mshr->downstreamPending = false; + } + } +} + + void MSHR::allocate(Addr _addr, int _size, PacketPtr target, Tick whenReady, Counter _order) @@ -109,6 +130,7 @@ MSHR::allocate(Addr _addr, int _size, PacketPtr target, isCacheFill = false; _isUncacheable = target->req->isUncacheable(); inService = false; + downstreamPending = false; threadNum = 0; ntargets = 1; // Don't know of a case where we would allocate a new MSHR for a @@ -121,6 +143,28 @@ MSHR::allocate(Addr _addr, int _size, PacketPtr target, data = NULL; } + +bool +MSHR::markInService() +{ + assert(!inService); + if (isSimpleForward()) { + // we just forwarded the request packet & don't expect a + // response, so get rid of it + assert(getNumTargets() == 1); + popTarget(); + return true; + } + inService = true; + if (!downstreamPending) { + // let upstream caches know that the request has made it to a + // level where it's going to get a response + targets->clearDownstreamPending(); + } + return false; +} + + void MSHR::deallocate() { @@ -164,7 +208,7 @@ MSHR::allocateTarget(PacketPtr pkt, Tick whenReady, Counter _order) bool MSHR::handleSnoop(PacketPtr pkt, Counter _order) { - if (!inService || (pkt->isExpressSnoop() && pkt->lowerMSHRPending())) { + if (!inService || downstreamPending) { // Request has not been issued yet, or it's been issued // locally but is buffered unissued at some downstream cache // which is forwarding us this snoop. Either way, the packet diff --git a/src/mem/cache/miss/mshr.hh b/src/mem/cache/miss/mshr.hh index 06ef6e113..e850a8633 100644 --- a/src/mem/cache/miss/mshr.hh +++ b/src/mem/cache/miss/mshr.hh @@ -81,6 +81,7 @@ class MSHR : public Packet::SenderState bool isReset() { return !needsExclusive && !hasUpgrade; } void add(PacketPtr pkt, Tick readyTime, Counter order, bool cpuSide); void replaceUpgrades(); + void clearDownstreamPending(); }; /** A list of MSHRs. */ @@ -117,6 +118,8 @@ class MSHR : public Packet::SenderState /** True if the request is uncacheable */ bool _isUncacheable; + bool downstreamPending; + bool pendingInvalidate; bool pendingShared; @@ -163,6 +166,8 @@ public: void allocate(Addr addr, int size, PacketPtr pkt, Tick when, Counter _order); + bool markInService(); + /** * Mark this MSHR as free. */ diff --git a/src/mem/cache/miss/mshr_queue.cc b/src/mem/cache/miss/mshr_queue.cc index 4d3cf30e1..50a28fb3c 100644 --- a/src/mem/cache/miss/mshr_queue.cc +++ b/src/mem/cache/miss/mshr_queue.cc @@ -179,20 +179,12 @@ MSHRQueue::moveToFront(MSHR *mshr) void MSHRQueue::markInService(MSHR *mshr) { - assert(!mshr->inService); - if (mshr->isSimpleForward()) { - // we just forwarded the request packet & don't expect a - // response, so get rid of it - assert(mshr->getNumTargets() == 1); - mshr->popTarget(); + if (mshr->markInService()) { deallocate(mshr); - return; + } else { + readyList.erase(mshr->readyIter); + inServiceEntries += 1; } - mshr->inService = true; - readyList.erase(mshr->readyIter); - //mshr->readyIter = NULL; - inServiceEntries += 1; - //readyList.pop_front(); } void diff --git a/src/mem/packet.hh b/src/mem/packet.hh index 779ea49a2..036bd3fd7 100644 --- a/src/mem/packet.hh +++ b/src/mem/packet.hh @@ -257,7 +257,6 @@ class Packet : public FastAlloc Shared, // Special control flags ExpressSnoop, - LowerMSHRPending, // not yet in service NUM_PACKET_FLAGS }; @@ -323,8 +322,6 @@ class Packet : public FastAlloc // Special control flags void setExpressSnoop() { flags[ExpressSnoop] = true; } bool isExpressSnoop() { return flags[ExpressSnoop]; } - void setLowerMSHRPending() { flags[LowerMSHRPending] = true; } - bool lowerMSHRPending() { return flags[LowerMSHRPending]; } // Network error conditions... encapsulate them as methods since // their encoding keeps changing (from result field to command