From 893ccdff45f514084899641d78721d3b432521f7 Mon Sep 17 00:00:00 2001 From: "Daniel R. Carvalho" Date: Mon, 21 Sep 2020 18:26:30 +0200 Subject: [PATCH] mem-cache: Encapsulate CacheBlk's status Encapsulate this variable to facilitate polymorphism. - The status enum was renamed to CoherenceBits, since it lists the coherence bits supported by the CacheBlk. - status was made protected and renamed to coherence since it contains the coherence bits. - Functions to set, clear and get the coherence bits were created. - To set a status bit, the block must be validated first. This guarantees a constant flow and helps catching bugs. As a side effect, some of the modified files contained long lines, which had to be split. Change-Id: I558cc51ac685d30b6bf298c78f86a6e24ff06973 Signed-off-by: Daniel R. Carvalho Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/34960 Reviewed-by: Jason Lowe-Power Reviewed-by: Nikos Nikoleris Maintainer: Jason Lowe-Power Maintainer: Nikos Nikoleris Tested-by: kokoro --- src/mem/cache/base.cc | 107 +++++++++++++++-------------- src/mem/cache/cache.cc | 47 +++++++------ src/mem/cache/cache_blk.cc | 6 +- src/mem/cache/cache_blk.hh | 105 +++++++++++++++++----------- src/mem/cache/noncoherent_cache.cc | 4 +- 5 files changed, 151 insertions(+), 118 deletions(-) diff --git a/src/mem/cache/base.cc b/src/mem/cache/base.cc index 7b6b3c38d..a24ffc765 100644 --- a/src/mem/cache/base.cc +++ b/src/mem/cache/base.cc @@ -318,9 +318,10 @@ BaseCache::handleTimingReqMiss(PacketPtr pkt, MSHR *mshr, CacheBlk *blk, // internally, and have a sufficiently weak memory // model, this is probably unnecessary, but at some // point it must have seemed like we needed it... - assert((pkt->needsWritable() && !blk->isWritable()) || - pkt->req->isCacheMaintenance()); - blk->status &= ~BlkReadable; + assert((pkt->needsWritable() && + !blk->isSet(CacheBlk::WritableBit)) || + pkt->req->isCacheMaintenance()); + blk->clearCoherenceBits(CacheBlk::ReadableBit); } // Here we are using forward_time, modelling the latency of // a miss (outbound) just as forwardLatency, neglecting the @@ -476,7 +477,7 @@ BaseCache::recvTimingResp(PacketPtr pkt) if (blk && blk->isValid() && pkt->isClean() && !pkt->isInvalidate()) { // The block was marked not readable while there was a pending // cache maintenance operation, restore its flag. - blk->status |= BlkReadable; + blk->setCoherenceBits(CacheBlk::ReadableBit); // This was a cache clean operation (without invalidate) // and we have a copy of the block already. Since there @@ -485,7 +486,8 @@ BaseCache::recvTimingResp(PacketPtr pkt) mshr->promoteReadable(); } - if (blk && blk->isWritable() && !pkt->req->isCacheInvalidate()) { + if (blk && blk->isSet(CacheBlk::WritableBit) && + !pkt->req->isCacheInvalidate()) { // If at this point the referenced block is writable and the // response is not a cache invalidate, we promote targets that // were deferred as we couldn't guarrantee a writable copy @@ -498,7 +500,7 @@ BaseCache::recvTimingResp(PacketPtr pkt) // avoid later read getting stale data while write miss is // outstanding.. see comment in timingAccess() if (blk) { - blk->status &= ~BlkReadable; + blk->clearCoherenceBits(CacheBlk::ReadableBit); } mshrQueue.markPending(mshr); schedMemSideSendEvent(clockEdge() + pkt->payloadDelay); @@ -551,7 +553,7 @@ BaseCache::recvAtomic(PacketPtr pkt) PacketList writebacks; bool satisfied = access(pkt, blk, lat, writebacks); - if (pkt->isClean() && blk && blk->isDirty()) { + if (pkt->isClean() && blk && blk->isSet(CacheBlk::DirtyBit)) { // A cache clean opearation is looking for a dirty // block. If a dirty block is encountered a WriteClean // will update any copies to the path to the memory @@ -641,7 +643,7 @@ BaseCache::functionalAccess(PacketPtr pkt, bool from_cpu_side) // data we have is dirty if marked as such or if we have an // in-service MSHR that is pending a modified line bool have_dirty = - have_data && (blk->isDirty() || + have_data && (blk->isSet(CacheBlk::DirtyBit) || (mshr && mshr->inService && mshr->isPendingModified())); bool done = have_dirty || @@ -709,7 +711,7 @@ BaseCache::cmpAndSwap(CacheBlk *blk, PacketPtr pkt) if (overwrite_mem) { std::memcpy(blk_data, &overwrite_val, pkt->getSize()); - blk->status |= BlkDirty; + blk->setCoherenceBits(CacheBlk::DirtyBit); } } @@ -805,8 +807,8 @@ BaseCache::handleEvictions(std::vector &evict_blks, if (mshr) { // Must be an outstanding upgrade or clean request on a block // we're about to replace - assert((!blk->isWritable() && mshr->needsWritable()) || - mshr->isCleaning()); + assert((!blk->isSet(CacheBlk::WritableBit) && + mshr->needsWritable()) || mshr->isCleaning()); return false; } } @@ -912,7 +914,7 @@ BaseCache::satisfyRequest(PacketPtr pkt, CacheBlk *blk, bool, bool) // can satisfy a following ReadEx anyway since we can rely on the // Read requestor(s) to have buffered the ReadEx snoop and to // invalidate their blocks after receiving them. - // assert(!pkt->needsWritable() || blk->isWritable()); + // assert(!pkt->needsWritable() || blk->isSet(CacheBlk::WritableBit)); assert(pkt->getOffset(blkSize) + pkt->getSize() <= blkSize); // Check RMW operations first since both isRead() and @@ -929,7 +931,7 @@ BaseCache::satisfyRequest(PacketPtr pkt, CacheBlk *blk, bool, bool) (*(pkt->getAtomicOp()))(blk_data); // set block status to dirty - blk->status |= BlkDirty; + blk->setCoherenceBits(CacheBlk::DirtyBit); } else { cmpAndSwap(blk, pkt); } @@ -938,7 +940,7 @@ BaseCache::satisfyRequest(PacketPtr pkt, CacheBlk *blk, bool, bool) // note that the line may be also be considered writable in // downstream caches along the path to memory, but always // Exclusive, and never Modified - assert(blk->isWritable()); + assert(blk->isSet(CacheBlk::WritableBit)); // Write or WriteLine at the first cache with block in writable state if (blk->checkWrite(pkt)) { pkt->writeDataToBlock(blk->data, blkSize); @@ -947,7 +949,7 @@ BaseCache::satisfyRequest(PacketPtr pkt, CacheBlk *blk, bool, bool) // Modified state) even if we are a failed StoreCond so we // supply data to any snoops that have appended themselves to // this cache before knowing the store will fail. - blk->status |= BlkDirty; + blk->setCoherenceBits(CacheBlk::DirtyBit); DPRINTF(CacheVerbose, "%s for %s (write)\n", __func__, pkt->print()); } else if (pkt->isRead()) { if (pkt->isLLSC()) { @@ -961,15 +963,15 @@ BaseCache::satisfyRequest(PacketPtr pkt, CacheBlk *blk, bool, bool) // sanity check assert(!pkt->hasSharers()); - if (blk->isDirty()) { + if (blk->isSet(CacheBlk::DirtyBit)) { // we were in the Owned state, and a cache above us that // has the line in Shared state needs to be made aware // that the data it already has is in fact dirty pkt->setCacheResponding(); - blk->status &= ~BlkDirty; + blk->clearCoherenceBits(CacheBlk::DirtyBit); } } else if (pkt->isClean()) { - blk->status &= ~BlkDirty; + blk->clearCoherenceBits(CacheBlk::DirtyBit); } else { assert(pkt->isInvalidate()); invalidateBlock(blk); @@ -1137,7 +1139,7 @@ BaseCache::access(PacketPtr pkt, CacheBlk *&blk, Cycles &lat, return false; } - blk->status |= BlkReadable; + blk->setCoherenceBits(CacheBlk::ReadableBit); } else if (compressor) { // This is an overwrite to an existing block, therefore we need // to check for data expansion (i.e., block was compressed with @@ -1153,14 +1155,14 @@ BaseCache::access(PacketPtr pkt, CacheBlk *&blk, Cycles &lat, // only mark the block dirty if we got a writeback command, // and leave it as is for a clean writeback if (pkt->cmd == MemCmd::WritebackDirty) { - // TODO: the coherent cache can assert(!blk->isDirty()); - blk->status |= BlkDirty; + // TODO: the coherent cache can assert that the dirty bit is set + blk->setCoherenceBits(CacheBlk::DirtyBit); } // if the packet does not have sharers, it is passing // writable, and we got the writeback in Modified or Exclusive // state, if not we are in the Owned or Shared state if (!pkt->hasSharers()) { - blk->status |= BlkWritable; + blk->setCoherenceBits(CacheBlk::WritableBit); } // nothing else to do; writeback doesn't expect response assert(!pkt->needsResponse()); @@ -1213,7 +1215,7 @@ BaseCache::access(PacketPtr pkt, CacheBlk *&blk, Cycles &lat, return false; } - blk->status |= BlkReadable; + blk->setCoherenceBits(CacheBlk::ReadableBit); } } else if (compressor) { // This is an overwrite to an existing block, therefore we need @@ -1231,9 +1233,9 @@ BaseCache::access(PacketPtr pkt, CacheBlk *&blk, Cycles &lat, // write clean operation and the block is already in this // cache, we need to update the data and the block flags assert(blk); - // TODO: the coherent cache can assert(!blk->isDirty()); + // TODO: the coherent cache can assert that the dirty bit is set if (!pkt->writeThrough()) { - blk->status |= BlkDirty; + blk->setCoherenceBits(CacheBlk::DirtyBit); } // nothing else to do; writeback doesn't expect response assert(!pkt->needsResponse()); @@ -1250,8 +1252,9 @@ BaseCache::access(PacketPtr pkt, CacheBlk *&blk, Cycles &lat, // If this a write-through packet it will be sent to cache below return !pkt->writeThrough(); - } else if (blk && (pkt->needsWritable() ? blk->isWritable() : - blk->isReadable())) { + } else if (blk && (pkt->needsWritable() ? + blk->isSet(CacheBlk::WritableBit) : + blk->isSet(CacheBlk::ReadableBit))) { // OK to satisfy access incHitCount(pkt); @@ -1293,8 +1296,8 @@ BaseCache::access(PacketPtr pkt, CacheBlk *&blk, Cycles &lat, void BaseCache::maintainClusivity(bool from_cache, CacheBlk *blk) { - if (from_cache && blk && blk->isValid() && !blk->isDirty() && - clusivity == Enums::mostly_excl) { + if (from_cache && blk && blk->isValid() && + !blk->isSet(CacheBlk::DirtyBit) && clusivity == Enums::mostly_excl) { // if we have responded to a cache, and our block is still // valid, but not dirty, and this cache is mostly exclusive // with respect to the cache above, drop the block @@ -1345,7 +1348,7 @@ BaseCache::handleFill(PacketPtr pkt, CacheBlk *blk, PacketList &writebacks, assert(blk->isSecure() == is_secure); assert(regenerateBlkAddr(blk) == addr); - blk->status |= BlkReadable; + blk->setCoherenceBits(CacheBlk::ReadableBit); // sanity check for whole-line writes, which should always be // marked as writable as part of the fill, and then later marked @@ -1365,14 +1368,14 @@ BaseCache::handleFill(PacketPtr pkt, CacheBlk *blk, PacketList &writebacks, // we could get a writable line from memory (rather than a // cache) even in a read-only cache, note that we set this bit // even for a read-only cache, possibly revisit this decision - blk->status |= BlkWritable; + blk->setCoherenceBits(CacheBlk::WritableBit); // check if we got this via cache-to-cache transfer (i.e., from a // cache that had the block in Modified or Owned state) if (pkt->cacheResponding()) { // we got the block in Modified state, and invalidated the // owners copy - blk->status |= BlkDirty; + blk->setCoherenceBits(CacheBlk::DirtyBit); chatty_assert(!isReadOnly, "Should never see dirty snoop response " "in read-only cache %s\n", name()); @@ -1487,7 +1490,8 @@ BaseCache::writebackBlk(CacheBlk *blk) { chatty_assert(!isReadOnly || writebackClean, "Writeback from read-only cache"); - assert(blk && blk->isValid() && (blk->isDirty() || writebackClean)); + assert(blk && blk->isValid() && + (blk->isSet(CacheBlk::DirtyBit) || writebackClean)); stats.writebacks[Request::wbRequestorId]++; @@ -1500,23 +1504,24 @@ BaseCache::writebackBlk(CacheBlk *blk) req->taskId(blk->getTaskId()); PacketPtr pkt = - new Packet(req, blk->isDirty() ? + new Packet(req, blk->isSet(CacheBlk::DirtyBit) ? MemCmd::WritebackDirty : MemCmd::WritebackClean); DPRINTF(Cache, "Create Writeback %s writable: %d, dirty: %d\n", - pkt->print(), blk->isWritable(), blk->isDirty()); + pkt->print(), blk->isSet(CacheBlk::WritableBit), + blk->isSet(CacheBlk::DirtyBit)); - if (blk->isWritable()) { + if (blk->isSet(CacheBlk::WritableBit)) { // not asserting shared means we pass the block in modified // state, mark our own block non-writeable - blk->status &= ~BlkWritable; + blk->clearCoherenceBits(CacheBlk::WritableBit); } else { // we are in the Owned state, tell the receiver pkt->setHasSharers(); } // make sure the block is not marked dirty - blk->status &= ~BlkDirty; + blk->clearCoherenceBits(CacheBlk::DirtyBit); pkt->allocate(); pkt->setDataFromBlock(blk->data, blkSize); @@ -1549,19 +1554,19 @@ BaseCache::writecleanBlk(CacheBlk *blk, Request::Flags dest, PacketId id) } DPRINTF(Cache, "Create %s writable: %d, dirty: %d\n", pkt->print(), - blk->isWritable(), blk->isDirty()); + blk->isSet(CacheBlk::WritableBit), blk->isSet(CacheBlk::DirtyBit)); - if (blk->isWritable()) { + if (blk->isSet(CacheBlk::WritableBit)) { // not asserting shared means we pass the block in modified // state, mark our own block non-writeable - blk->status &= ~BlkWritable; + blk->clearCoherenceBits(CacheBlk::WritableBit); } else { // we are in the Owned state, tell the receiver pkt->setHasSharers(); } // make sure the block is not marked dirty - blk->status &= ~BlkDirty; + blk->clearCoherenceBits(CacheBlk::DirtyBit); pkt->allocate(); pkt->setDataFromBlock(blk->data, blkSize); @@ -1591,7 +1596,8 @@ BaseCache::memInvalidate() bool BaseCache::isDirty() const { - return tags->anyBlk([](CacheBlk &blk) { return blk.isDirty(); }); + return tags->anyBlk([](CacheBlk &blk) { + return blk.isSet(CacheBlk::DirtyBit); }); } bool @@ -1603,7 +1609,7 @@ BaseCache::coalesce() const void BaseCache::writebackVisitor(CacheBlk &blk) { - if (blk.isDirty()) { + if (blk.isSet(CacheBlk::DirtyBit)) { assert(blk.isValid()); RequestPtr request = std::make_shared( @@ -1619,19 +1625,19 @@ BaseCache::writebackVisitor(CacheBlk &blk) memSidePort.sendFunctional(&packet); - blk.status &= ~BlkDirty; + blk.clearCoherenceBits(CacheBlk::DirtyBit); } } void BaseCache::invalidateVisitor(CacheBlk &blk) { - if (blk.isDirty()) + if (blk.isSet(CacheBlk::DirtyBit)) warn_once("Invalidating dirty cache lines. " \ "Expect things to break.\n"); if (blk.isValid()) { - assert(!blk.isDirty()); + assert(!blk.isSet(CacheBlk::DirtyBit)); invalidateBlock(&blk); } } @@ -1707,7 +1713,7 @@ BaseCache::sendMSHRQueuePacket(MSHR* mshr) // as forwarded packets may already have existing state pkt->pushSenderState(mshr); - if (pkt->isClean() && blk && blk->isDirty()) { + if (pkt->isClean() && blk && blk->isSet(CacheBlk::DirtyBit)) { // A cache clean opearation is looking for a dirty block. Mark // the packet so that the destination xbar can determine that // there will be a follow-up write packet as well. @@ -1738,7 +1744,7 @@ BaseCache::sendMSHRQueuePacket(MSHR* mshr) pkt->cacheResponding(); markInService(mshr, pending_modified_resp); - if (pkt->isClean() && blk && blk->isDirty()) { + if (pkt->isClean() && blk && blk->isSet(CacheBlk::DirtyBit)) { // A cache clean opearation is looking for a dirty // block. If a dirty block is encountered a WriteClean // will update any copies to the path to the memory @@ -2264,7 +2270,8 @@ BaseCache::CacheStats::regStats() overallAvgMshrUncacheableLatency = overallMshrUncacheableLatency / overallMshrUncacheable; for (int i = 0; i < max_requestors; i++) { - overallAvgMshrUncacheableLatency.subname(i, system->getRequestorName(i)); + overallAvgMshrUncacheableLatency.subname(i, + system->getRequestorName(i)); } dataExpansions.flags(nozero | nonan); diff --git a/src/mem/cache/cache.cc b/src/mem/cache/cache.cc index 9e87c2ade..b9cf8308d 100644 --- a/src/mem/cache/cache.cc +++ b/src/mem/cache/cache.cc @@ -89,13 +89,13 @@ Cache::satisfyRequest(PacketPtr pkt, CacheBlk *blk, // if we have a dirty copy, make sure the recipient // keeps it marked dirty (in the modified state) - if (blk->isDirty()) { + if (blk->isSet(CacheBlk::DirtyBit)) { pkt->setCacheResponding(); - blk->status &= ~BlkDirty; + blk->clearCoherenceBits(CacheBlk::DirtyBit); } - } else if (blk->isWritable() && !pending_downgrade && - !pkt->hasSharers() && - pkt->cmd != MemCmd::ReadCleanReq) { + } else if (blk->isSet(CacheBlk::WritableBit) && + !pending_downgrade && !pkt->hasSharers() && + pkt->cmd != MemCmd::ReadCleanReq) { // we can give the requestor a writable copy on a read // request if: // - we have a writable copy at this level (& below) @@ -106,7 +106,7 @@ Cache::satisfyRequest(PacketPtr pkt, CacheBlk *blk, // snooping the packet) // - the read has explicitly asked for a clean // copy of the line - if (blk->isDirty()) { + if (blk->isSet(CacheBlk::DirtyBit)) { // special considerations if we're owner: if (!deferred_response) { // respond with the line in Modified state @@ -128,7 +128,7 @@ Cache::satisfyRequest(PacketPtr pkt, CacheBlk *blk, // the cache hierarchy through a cache, // and first snoop upwards in all other // branches - blk->status &= ~BlkDirty; + blk->clearCoherenceBits(CacheBlk::DirtyBit); } else { // if we're responding after our own miss, // there's a window where the recipient didn't @@ -496,7 +496,7 @@ Cache::createMissPacket(PacketPtr cpu_pkt, CacheBlk *blk, const bool useUpgrades = true; assert(cpu_pkt->cmd != MemCmd::WriteLineReq || is_whole_line_write); if (is_whole_line_write) { - assert(!blkValid || !blk->isWritable()); + assert(!blkValid || !blk->isSet(CacheBlk::WritableBit)); // forward as invalidate to all other caches, this gives us // the line in Exclusive state, and invalidates all other // copies @@ -505,7 +505,7 @@ Cache::createMissPacket(PacketPtr cpu_pkt, CacheBlk *blk, // only reason to be here is that blk is read only and we need // it to be writable assert(needsWritable); - assert(!blk->isWritable()); + assert(!blk->isSet(CacheBlk::WritableBit)); cmd = cpu_pkt->isLLSC() ? MemCmd::SCUpgradeReq : MemCmd::UpgradeReq; } else if (cpu_pkt->cmd == MemCmd::SCUpgradeFailReq || cpu_pkt->cmd == MemCmd::StoreCondFailReq) { @@ -722,7 +722,7 @@ Cache::serviceMSHRTargets(MSHR *mshr, const PacketPtr pkt, CacheBlk *blk) // between the PrefetchExReq and the expected WriteReq, we // proactively mark the block as Dirty. assert(blk); - blk->status |= BlkDirty; + blk->setCoherenceBits(CacheBlk::DirtyBit); panic_if(isReadOnly, "Prefetch exclusive requests from " "read-only cache %s\n", name()); @@ -745,7 +745,7 @@ Cache::serviceMSHRTargets(MSHR *mshr, const PacketPtr pkt, CacheBlk *blk) if (tgt_pkt->cmd == MemCmd::WriteLineReq) { assert(!is_error); assert(blk); - assert(blk->isWritable()); + assert(blk->isSet(CacheBlk::WritableBit)); } // Here we decide whether we will satisfy the target using @@ -888,7 +888,7 @@ Cache::serviceMSHRTargets(MSHR *mshr, const PacketPtr pkt, CacheBlk *blk) if (is_invalidate || mshr->hasPostInvalidate()) { invalidateBlock(blk); } else if (mshr->hasPostDowngrade()) { - blk->status &= ~BlkWritable; + blk->clearCoherenceBits(CacheBlk::WritableBit); } } } @@ -896,7 +896,7 @@ Cache::serviceMSHRTargets(MSHR *mshr, const PacketPtr pkt, CacheBlk *blk) PacketPtr Cache::evictBlock(CacheBlk *blk) { - PacketPtr pkt = (blk->isDirty() || writebackClean) ? + PacketPtr pkt = (blk->isSet(CacheBlk::DirtyBit) || writebackClean) ? writebackBlk(blk) : cleanEvictBlk(blk); invalidateBlock(blk); @@ -908,7 +908,7 @@ PacketPtr Cache::cleanEvictBlk(CacheBlk *blk) { assert(!writebackClean); - assert(blk && blk->isValid() && !blk->isDirty()); + assert(blk && blk->isValid() && !blk->isSet(CacheBlk::DirtyBit)); // Creating a zero sized write, a message to the snoop filter RequestPtr req = std::make_shared( @@ -1053,10 +1053,11 @@ Cache::handleSnoop(PacketPtr pkt, CacheBlk *blk, bool is_timing, bool respond = false; bool blk_valid = blk && blk->isValid(); if (pkt->isClean()) { - if (blk_valid && blk->isDirty()) { + if (blk_valid && blk->isSet(CacheBlk::DirtyBit)) { DPRINTF(CacheVerbose, "%s: packet (snoop) %s found block: %s\n", __func__, pkt->print(), blk->print()); - PacketPtr wb_pkt = writecleanBlk(blk, pkt->req->getDest(), pkt->id); + PacketPtr wb_pkt = + writecleanBlk(blk, pkt->req->getDest(), pkt->id); PacketList writebacks; writebacks.push_back(wb_pkt); @@ -1098,10 +1099,11 @@ Cache::handleSnoop(PacketPtr pkt, CacheBlk *blk, bool is_timing, // invalidation itself is taken care of below. We don't respond to // cache maintenance operations as this is done by the destination // xbar. - respond = blk->isDirty() && pkt->needsResponse(); + respond = blk->isSet(CacheBlk::DirtyBit) && pkt->needsResponse(); - chatty_assert(!(isReadOnly && blk->isDirty()), "Should never have " - "a dirty block in a read-only cache %s\n", name()); + chatty_assert(!(isReadOnly && blk->isSet(CacheBlk::DirtyBit)), + "Should never have a dirty block in a read-only cache %s\n", + name()); } // Invalidate any prefetch's from below that would strip write permissions @@ -1125,8 +1127,9 @@ Cache::handleSnoop(PacketPtr pkt, CacheBlk *blk, bool is_timing, // which means we go from Modified to Owned (and will respond // below), remain in Owned (and will respond below), from // Exclusive to Shared, or remain in Shared - if (!pkt->req->isUncacheable()) - blk->status &= ~BlkWritable; + if (!pkt->req->isUncacheable()) { + blk->clearCoherenceBits(CacheBlk::WritableBit); + } DPRINTF(Cache, "new state is %s\n", blk->print()); } @@ -1135,7 +1138,7 @@ Cache::handleSnoop(PacketPtr pkt, CacheBlk *blk, bool is_timing, // memory, and also prevent any memory from even seeing the // request pkt->setCacheResponding(); - if (!pkt->isClean() && blk->isWritable()) { + if (!pkt->isClean() && blk->isSet(CacheBlk::WritableBit)) { // inform the cache hierarchy that this cache had the line // in the Modified state so that we avoid unnecessary // invalidations (see Packet::setResponderHadWritable) diff --git a/src/mem/cache/cache_blk.cc b/src/mem/cache/cache_blk.cc index b747f3905..e4bcc6328 100644 --- a/src/mem/cache/cache_blk.cc +++ b/src/mem/cache/cache_blk.cc @@ -48,7 +48,7 @@ CacheBlk::insert(const Addr tag, const bool is_secure, const int src_requestor_ID, const uint32_t task_ID) { // Make sure that the block has been properly invalidated - assert(status == 0); + assert(!isValid()); insert(tag, is_secure); @@ -71,8 +71,8 @@ CacheBlkPrintWrapper::print(std::ostream &os, int verbosity, { ccprintf(os, "%sblk %c%c%c%c\n", prefix, blk->isValid() ? 'V' : '-', - blk->isWritable() ? 'E' : '-', - blk->isDirty() ? 'M' : '-', + blk->isSet(CacheBlk::WritableBit) ? 'E' : '-', + blk->isSet(CacheBlk::DirtyBit) ? 'M' : '-', blk->isSecure() ? 'S' : '-'); } diff --git a/src/mem/cache/cache_blk.hh b/src/mem/cache/cache_blk.hh index bf9bfe6b8..c16e59980 100644 --- a/src/mem/cache/cache_blk.hh +++ b/src/mem/cache/cache_blk.hh @@ -59,18 +59,6 @@ #include "mem/request.hh" #include "sim/core.hh" -/** - * Cache block status bit assignments - */ -enum CacheBlkStatusBits : unsigned { - /** write permission */ - BlkWritable = 0x02, - /** read permission (yes, block can be valid but not readable) */ - BlkReadable = 0x04, - /** dirty (modified) */ - BlkDirty = 0x08, -}; - /** * A Basic Cache block. * Contains information regarding its coherence, prefetching status, as @@ -79,6 +67,29 @@ enum CacheBlkStatusBits : unsigned { class CacheBlk : public TaggedEntry { public: + /** + * Cache block's enum listing the supported coherence bits. The valid + * bit is not defined here because it is part of a TaggedEntry. + */ + enum CoherenceBits : unsigned + { + /** write permission */ + WritableBit = 0x02, + /** + * Read permission. Note that a block can be valid but not readable + * if there is an outstanding write upgrade miss. + */ + ReadableBit = 0x04, + /** dirty (modified) */ + DirtyBit = 0x08, + + /** + * Helper enum value that includes all other bits. Whenever a new + * bits is added, this should be updated. + */ + AllBits = 0x0E, + }; + /** * Contains a copy of the data in this block for easy access. This is used * for efficient execution when the data could be actually stored in @@ -88,12 +99,6 @@ class CacheBlk : public TaggedEntry */ uint8_t *data; - /** block state: OR of CacheBlkStatusBit */ - typedef unsigned State; - - /** The current status of this block. @sa CacheBlockStatusBits */ - State status; - /** * Which curTick() will this block be accessible. Its value is only * meaningful if the block is valid. @@ -152,29 +157,17 @@ class CacheBlk : public TaggedEntry CacheBlk& operator=(const CacheBlk&) = delete; virtual ~CacheBlk() {}; - /** - * Checks the write permissions of this block. - * @return True if the block is writable. - */ - bool isWritable() const { return isValid() && (status & BlkWritable); } - - /** - * Checks the read permissions of this block. Note that a block - * can be valid but not readable if there is an outstanding write - * upgrade miss. - * @return True if the block is readable. - */ - bool isReadable() const { return isValid() && (status & BlkReadable); } - /** * Invalidate the block and clear all state. */ virtual void invalidate() { TaggedEntry::invalidate(); + clearPrefetched(); + clearCoherenceBits(AllBits); + setTaskId(ContextSwitchTaskId::Unknown); - status = 0; whenReady = MaxTick; setRefCount(0); setSrcRequestorId(Request::invldRequestorId); @@ -182,12 +175,33 @@ class CacheBlk : public TaggedEntry } /** - * Check to see if a block has been written. - * @return True if the block is dirty. + * Sets the corresponding coherence bits. + * + * @param bits The coherence bits to be set. */ - bool isDirty() const + void + setCoherenceBits(unsigned bits) { - return (status & BlkDirty) != 0; + assert(isValid()); + coherence |= bits; + } + + /** + * Clear the corresponding coherence bits. + * + * @param bits The coherence bits to be cleared. + */ + void clearCoherenceBits(unsigned bits) { coherence &= ~bits; } + + /** + * Checks the given coherence bits are set. + * + * @return True if the block is readable. + */ + bool + isSet(unsigned bits) const + { + return isValid() && (coherence & bits); } /** @@ -327,7 +341,7 @@ class CacheBlk : public TaggedEntry * * Note that only one cache ever has a block in Modified or * Owned state, i.e., only one cache owns the block, or - * equivalently has the BlkDirty bit set. However, multiple + * equivalently has the DirtyBit bit set. However, multiple * caches on the same path to memory can have a block in the * Exclusive state (despite the name). Exclusive means this * cache has the only copy at this level of the hierarchy, @@ -336,7 +350,8 @@ class CacheBlk : public TaggedEntry * this branch of the hierarchy, and no caches at or above * this level on any other branch have copies either. **/ - unsigned state = isWritable() << 2 | isDirty() << 1 | isValid(); + unsigned state = + isSet(WritableBit) << 2 | isSet(DirtyBit) << 1 | isValid(); char s = '?'; switch (state) { case 0b111: s = 'M'; break; @@ -347,8 +362,8 @@ class CacheBlk : public TaggedEntry default: s = 'T'; break; // @TODO add other types } return csprintf("state: %x (%c) writable: %d readable: %d " - "dirty: %d | %s", status, s, isWritable(), isReadable(), - isDirty(), TaggedEntry::print()); + "dirty: %d | %s", coherence, s, isSet(WritableBit), + isSet(ReadableBit), isSet(DirtyBit), TaggedEntry::print()); } /** @@ -399,6 +414,14 @@ class CacheBlk : public TaggedEntry } protected: + /** The current coherence status of this block. @sa CoherenceBits */ + unsigned coherence; + + // The following setters have been marked as protected because their + // respective variables should only be modified at 2 moments: + // invalidation and insertion. Because of that, they shall only be + // called by the functions that perform those actions. + /** Set the task id value. */ void setTaskId(const uint32_t task_id) { _taskId = task_id; } diff --git a/src/mem/cache/noncoherent_cache.cc b/src/mem/cache/noncoherent_cache.cc index 0cea4945e..c09e9e107 100644 --- a/src/mem/cache/noncoherent_cache.cc +++ b/src/mem/cache/noncoherent_cache.cc @@ -83,7 +83,7 @@ NoncoherentCache::access(PacketPtr pkt, CacheBlk *&blk, Cycles &lat, // referenced block was not present or it was invalid. If that // is the case, make sure that the new block is marked as // writable - blk->status |= BlkWritable; + blk->setCoherenceBits(CacheBlk::WritableBit); } return success; @@ -340,7 +340,7 @@ NoncoherentCache::evictBlock(CacheBlk *blk) // If we clean writebacks are not enabled, we do not take any // further action for evictions of clean blocks (i.e., CleanEvicts // are unnecessary). - PacketPtr pkt = (blk->isDirty() || writebackClean) ? + PacketPtr pkt = (blk->isSet(CacheBlk::DirtyBit) || writebackClean) ? writebackBlk(blk) : nullptr; invalidateBlock(blk); -- 2.30.2