From 42bd460d7f91f4d0f76901a8d661d5fe6292b7f2 Mon Sep 17 00:00:00 2001 From: Steve Reinhardt Date: Mon, 10 Nov 2008 14:10:28 -0800 Subject: [PATCH] Cache: Refactor packet forwarding a bit. Makes adding write-through operations easier. --- src/mem/cache/cache_impl.hh | 67 +++++++++++++++++++++++-------------- src/mem/cache/mshr.cc | 7 ++-- src/mem/cache/mshr.hh | 16 +++++---- 3 files changed, 56 insertions(+), 34 deletions(-) diff --git a/src/mem/cache/cache_impl.hh b/src/mem/cache/cache_impl.hh index 40d1c9c2f..c4a19ad5c 100644 --- a/src/mem/cache/cache_impl.hh +++ b/src/mem/cache/cache_impl.hh @@ -613,38 +613,53 @@ Cache::atomicAccess(PacketPtr pkt) if (!access(pkt, blk, lat, writebacks)) { // MISS - PacketPtr busPkt = getBusPacket(pkt, blk, pkt->needsExclusive()); + PacketPtr bus_pkt = getBusPacket(pkt, blk, pkt->needsExclusive()); - bool isCacheFill = (busPkt != NULL); + bool is_forward = (bus_pkt == NULL); - if (busPkt == NULL) { + if (is_forward) { // just forwarding the same request to the next level // no local cache operation involved - busPkt = pkt; + bus_pkt = pkt; } DPRINTF(Cache, "Sending an atomic %s for %x\n", - busPkt->cmdString(), busPkt->getAddr()); + bus_pkt->cmdString(), bus_pkt->getAddr()); #if TRACING_ON CacheBlk::State old_state = blk ? blk->status : 0; #endif - lat += memSidePort->sendAtomic(busPkt); + lat += memSidePort->sendAtomic(bus_pkt); DPRINTF(Cache, "Receive response: %s for addr %x in state %i\n", - busPkt->cmdString(), busPkt->getAddr(), old_state); - - bool is_error = busPkt->isError(); - assert(!busPkt->wasNacked()); - - if (is_error && pkt->needsResponse()) { - pkt->makeAtomicResponse(); - pkt->copyError(busPkt); - } else if (isCacheFill && !is_error) { - blk = handleFill(busPkt, blk, writebacks); - satisfyCpuSideRequest(pkt, blk); - delete busPkt; + bus_pkt->cmdString(), bus_pkt->getAddr(), old_state); + + assert(!bus_pkt->wasNacked()); + + // If packet was a forward, the response (if any) is already + // in place in the bus_pkt == pkt structure, so we don't need + // to do anything. Otherwise, use the separate bus_pkt to + // generate response to pkt and then delete it. + if (!is_forward) { + if (pkt->needsResponse()) { + assert(bus_pkt->isResponse()); + if (bus_pkt->isError()) { + pkt->makeAtomicResponse(); + pkt->copyError(bus_pkt); + } else if (bus_pkt->isRead() || + bus_pkt->cmd == MemCmd::UpgradeResp) { + // we're updating cache state to allow us to + // satisfy the upstream request from the cache + blk = handleFill(bus_pkt, blk, writebacks); + satisfyCpuSideRequest(pkt, blk); + } else { + // we're satisfying the upstream request without + // modifying cache state, e.g., a write-through + pkt->makeAtomicResponse(); + } + } + delete bus_pkt; } } @@ -748,7 +763,10 @@ Cache::handleResponse(PacketPtr pkt) miss_latency; } - if (mshr->isCacheFill && !is_error) { + bool is_fill = !mshr->isForward && + (pkt->isRead() || pkt->cmd == MemCmd::UpgradeResp); + + if (is_fill && !is_error) { DPRINTF(Cache, "Block for addr %x being updated in Cache\n", pkt->getAddr()); @@ -771,7 +789,7 @@ Cache::handleResponse(PacketPtr pkt) if (target->isCpuSide()) { Tick completion_time; - if (blk != NULL) { + if (is_fill) { satisfyCpuSideRequest(target->pkt, blk); // How many bytes past the first request is this one int transfer_offset = @@ -1287,7 +1305,7 @@ Cache::getTimingPacket() PacketPtr tgt_pkt = mshr->getTarget()->pkt; PacketPtr pkt = NULL; - if (mshr->isSimpleForward()) { + if (mshr->isForwardNoResponse()) { // no response expected, just forward packet as it is assert(tags->findBlock(mshr->addr) == NULL); pkt = tgt_pkt; @@ -1295,11 +1313,10 @@ Cache::getTimingPacket() BlkType *blk = tags->findBlock(mshr->addr); pkt = getBusPacket(tgt_pkt, blk, mshr->needsExclusive()); - mshr->isCacheFill = (pkt != NULL); + mshr->isForward = (pkt == NULL); - if (pkt == NULL) { + if (mshr->isForward) { // not a cache block request, but a response is expected - assert(!mshr->isSimpleForward()); // make copy of current packet to forward, keep current // copy for response handling pkt = new Packet(tgt_pkt); @@ -1473,7 +1490,7 @@ Cache::MemSidePort::sendPacket() waitingOnRetry = !success; if (waitingOnRetry) { DPRINTF(CachePort, "now waiting on a retry\n"); - if (!mshr->isSimpleForward()) { + if (!mshr->isForwardNoResponse()) { delete pkt; } } else { diff --git a/src/mem/cache/mshr.cc b/src/mem/cache/mshr.cc index 6537f6343..04b2b8d77 100644 --- a/src/mem/cache/mshr.cc +++ b/src/mem/cache/mshr.cc @@ -156,7 +156,7 @@ MSHR::allocate(Addr _addr, int _size, PacketPtr target, readyTime = whenReady; order = _order; assert(target); - isCacheFill = false; + isForward = false; _isUncacheable = target->req->isUncacheable(); inService = false; downstreamPending = false; @@ -187,7 +187,7 @@ bool MSHR::markInService() { assert(!inService); - if (isSimpleForward()) { + if (isForwardNoResponse()) { // we just forwarded the request packet & don't expect a // response, so get rid of it assert(getNumTargets() == 1); @@ -403,7 +403,8 @@ MSHR::print(std::ostream &os, int verbosity, const std::string &prefix) const { ccprintf(os, "%s[%x:%x] %s %s %s state: %s %s %s %s\n", prefix, addr, addr+size-1, - isCacheFill ? "Fill" : "", + isForward ? "Forward" : "", + isForwardNoResponse() ? "ForwNoResp" : "", needsExclusive() ? "Excl" : "", _isUncacheable ? "Unc" : "", inService ? "InSvc" : "", diff --git a/src/mem/cache/mshr.hh b/src/mem/cache/mshr.hh index fdb0485cb..2ff1c2489 100644 --- a/src/mem/cache/mshr.hh +++ b/src/mem/cache/mshr.hh @@ -118,8 +118,8 @@ class MSHR : public Packet::SenderState, public Printable /** True if the request has been sent to the bus. */ bool inService; - /** True if we will be putting the returned block in the cache */ - bool isCacheFill; + /** True if the request is just a simple forward from an upper level */ + bool isForward; /** True if we need to get an exclusive copy of the block. */ bool needsExclusive() const { return targets->needsExclusive; } @@ -200,7 +200,7 @@ public: * Returns the current number of allocated targets. * @return The current number of allocated targets. */ - int getNumTargets() { return ntargets; } + int getNumTargets() const { return ntargets; } /** * Returns a pointer to the target list. @@ -212,13 +212,17 @@ public: * Returns true if there are targets left. * @return true if there are targets */ - bool hasTargets() { return !targets->empty(); } + bool hasTargets() const { return !targets->empty(); } /** * Returns a reference to the first target. * @return A pointer to the first target. */ - Target *getTarget() { assert(hasTargets()); return &targets->front(); } + Target *getTarget() const + { + assert(hasTargets()); + return &targets->front(); + } /** * Pop first target. @@ -229,7 +233,7 @@ public: targets->pop_front(); } - bool isSimpleForward() + bool isForwardNoResponse() const { if (getNumTargets() != 1) return false; -- 2.30.2