Daniel Carvalho has uploaded this change for review. (
https://gem5-review.googlesource.com/c/public/gem5/+/34960 )
Change subject: mem-cache: Encapsulate CacheBlk's status
......................................................................
mem-cache: Encapsulate CacheBlk's status
Encapsulate this variable to facilitate polymorphism.
- status was made protected, along with its respective enum.
- The typedef was removed due to lack of necessity. External
use was for debugging reasons; thus it has been replaced
by calls to the printing function.
- setters and getters were created for each status bit
- To set a status bit, the block must be validated first.
This guarantees a constant flow and helps catching bugs.
- The compression bit is specific to compressed blocks. Its
invalidation is then responsibility of CompressionBlk.
Change-Id: I558cc51ac685d30b6bf298c78f86a6e24ff06973
Signed-off-by: Daniel R. Carvalho <oda...@yahoo.com.br>
---
M src/mem/cache/base.cc
M src/mem/cache/cache.cc
M src/mem/cache/cache_blk.cc
M src/mem/cache/cache_blk.hh
M src/mem/cache/noncoherent_cache.cc
M src/mem/cache/tags/super_blk.cc
M src/mem/cache/tags/super_blk.hh
7 files changed, 138 insertions(+), 109 deletions(-)
diff --git a/src/mem/cache/base.cc b/src/mem/cache/base.cc
index 3d3c9a9..3fc2d6a 100644
--- a/src/mem/cache/base.cc
+++ b/src/mem/cache/base.cc
@@ -320,7 +320,7 @@
// point it must have seemed like we needed it...
assert((pkt->needsWritable() && !blk->isWritable()) ||
pkt->req->isCacheMaintenance());
- blk->status &= ~BlkReadable;
+ blk->clearReadable();
}
// Here we are using forward_time, modelling the latency of
// a miss (outbound) just as forwardLatency, neglecting the
@@ -367,7 +367,7 @@
ppHit->notify(pkt);
if (prefetcher && blk && blk->wasPrefetched()) {
- blk->status &= ~BlkHWPrefetched;
+ blk->clearPrefetched();
}
handleTimingReqHit(pkt, blk, request_time);
@@ -476,7 +476,7 @@
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->setReadable();
// This was a cache clean operation (without invalidate)
// and we have a copy of the block already. Since there
@@ -498,7 +498,7 @@
// avoid later read getting stale data while write miss is
// outstanding.. see comment in timingAccess()
if (blk) {
- blk->status &= ~BlkReadable;
+ blk->clearReadable();
}
mshrQueue.markPending(mshr);
schedMemSideSendEvent(clockEdge() + pkt->payloadDelay);
@@ -709,7 +709,7 @@
if (overwrite_mem) {
std::memcpy(blk_data, &overwrite_val, pkt->getSize());
- blk->status |= BlkDirty;
+ blk->setDirty();
}
}
@@ -929,7 +929,7 @@
(*(pkt->getAtomicOp()))(blk_data);
// set block status to dirty
- blk->status |= BlkDirty;
+ blk->setDirty();
} else {
cmpAndSwap(blk, pkt);
}
@@ -947,7 +947,7 @@
// 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->setDirty();
DPRINTF(CacheVerbose, "%s for %s (write)\n", __func__,
pkt->print());
} else if (pkt->isRead()) {
if (pkt->isLLSC()) {
@@ -966,10 +966,10 @@
// 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->clearDirty();
}
} else if (pkt->isClean()) {
- blk->status &= ~BlkDirty;
+ blk->clearDirty();
} else {
assert(pkt->isInvalidate());
invalidateBlock(blk);
@@ -1137,7 +1137,7 @@
return false;
}
- blk->status |= BlkReadable;
+ blk->setReadable();
} 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
@@ -1154,13 +1154,13 @@
// 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;
+ blk->setDirty();
}
// 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->setWritable();
}
// nothing else to do; writeback doesn't expect response
assert(!pkt->needsResponse());
@@ -1213,7 +1213,7 @@
return false;
}
- blk->status |= BlkReadable;
+ blk->setReadable();
}
} else if (compressor) {
// This is an overwrite to an existing block, therefore we need
@@ -1233,7 +1233,7 @@
assert(blk);
// TODO: the coherent cache can assert(!blk->isDirty());
if (!pkt->writeThrough()) {
- blk->status |= BlkDirty;
+ blk->setDirty();
}
// nothing else to do; writeback doesn't expect response
assert(!pkt->needsResponse());
@@ -1310,7 +1310,7 @@
Addr addr = pkt->getAddr();
bool is_secure = pkt->isSecure();
#if TRACING_ON
- CacheBlk::State old_state = blk ? blk->status : 0;
+ std::string old_state = blk ? blk->print() : "";
#endif
// When handling a fill, we should have no writes to this line.
@@ -1345,7 +1345,7 @@
assert(blk->isSecure() == is_secure);
assert(regenerateBlkAddr(blk) == addr);
- blk->status |= BlkReadable;
+ blk->setReadable();
// sanity check for whole-line writes, which should always be
// marked as writable as part of the fill, and then later marked
@@ -1365,14 +1365,14 @@
// 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->setWritable();
// 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->setDirty();
chatty_assert(!isReadOnly, "Should never see dirty snoop
response "
"in read-only cache %s\n", name());
@@ -1380,7 +1380,7 @@
}
}
- DPRINTF(Cache, "Block addr %#llx (%s) moving from state %x to %s\n",
+ DPRINTF(Cache, "Block addr %#llx (%s) moving from state %s to %s\n",
addr, is_secure ? "s" : "ns", old_state, blk->print());
// if we got new data, copy it in (checking for a read response
@@ -1509,14 +1509,14 @@
if (blk->isWritable()) {
// not asserting shared means we pass the block in modified
// state, mark our own block non-writeable
- blk->status &= ~BlkWritable;
+ blk->clearWritable();
} else {
// we are in the Owned state, tell the receiver
pkt->setHasSharers();
}
// make sure the block is not marked dirty
- blk->status &= ~BlkDirty;
+ blk->clearDirty();
pkt->allocate();
pkt->setDataFromBlock(blk->data, blkSize);
@@ -1554,14 +1554,14 @@
if (blk->isWritable()) {
// not asserting shared means we pass the block in modified
// state, mark our own block non-writeable
- blk->status &= ~BlkWritable;
+ blk->clearWritable();
} else {
// we are in the Owned state, tell the receiver
pkt->setHasSharers();
}
// make sure the block is not marked dirty
- blk->status &= ~BlkDirty;
+ blk->clearDirty();
pkt->allocate();
pkt->setDataFromBlock(blk->data, blkSize);
@@ -1619,7 +1619,7 @@
memSidePort.sendFunctional(&packet);
- blk.status &= ~BlkDirty;
+ blk.clearDirty();
}
}
diff --git a/src/mem/cache/cache.cc b/src/mem/cache/cache.cc
index ad18c28..2ee5ec0 100644
--- a/src/mem/cache/cache.cc
+++ b/src/mem/cache/cache.cc
@@ -91,7 +91,7 @@
// keeps it marked dirty (in the modified state)
if (blk->isDirty()) {
pkt->setCacheResponding();
- blk->status &= ~BlkDirty;
+ blk->clearDirty();
}
} else if (blk->isWritable() && !pending_downgrade &&
!pkt->hasSharers() &&
@@ -128,7 +128,7 @@
// the cache hierarchy through a cache,
// and first snoop upwards in all other
// branches
- blk->status &= ~BlkDirty;
+ blk->clearDirty();
} else {
// if we're responding after our own miss,
// there's a window where the recipient didn't
@@ -590,7 +590,7 @@
bus_pkt->print());
#if TRACING_ON
- CacheBlk::State old_state = blk ? blk->status : 0;
+ std::string old_state = blk ? blk->print() : "";
#endif
Cycles latency = ticksToCycles(memSidePort.sendAtomic(bus_pkt));
@@ -598,7 +598,7 @@
bool is_invalidate = bus_pkt->isInvalidate();
// We are now dealing with the response handling
- DPRINTF(Cache, "%s: Receive response: %s in state %i\n", __func__,
+ DPRINTF(Cache, "%s: Receive response: %s in state %s\n", __func__,
bus_pkt->print(), old_state);
// If packet was a forward, the response (if any) is already
@@ -722,7 +722,7 @@
// between the PrefetchExReq and the expected
WriteReq, we
// proactively mark the block as Dirty.
assert(blk);
- blk->status |= BlkDirty;
+ blk->setDirty();
panic_if(isReadOnly, "Prefetch exclusive requests
from "
"read-only cache %s\n", name());
@@ -849,8 +849,9 @@
case MSHR::Target::FromPrefetcher:
assert(tgt_pkt->cmd == MemCmd::HardPFReq);
- if (blk)
- blk->status |= BlkHWPrefetched;
+ if (blk) {
+ blk->setPrefetched();
+ }
delete tgt_pkt;
break;
@@ -888,7 +889,7 @@
if (is_invalidate || mshr->hasPostInvalidate()) {
invalidateBlock(blk);
} else if (mshr->hasPostDowngrade()) {
- blk->status &= ~BlkWritable;
+ blk->clearWritable();
}
}
}
@@ -1125,8 +1126,9 @@
// 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->clearWritable();
+ }
DPRINTF(Cache, "new state is %s\n", blk->print());
}
diff --git a/src/mem/cache/cache_blk.cc b/src/mem/cache/cache_blk.cc
index 96c54a2..f826ea6 100644
--- a/src/mem/cache/cache_blk.cc
+++ b/src/mem/cache/cache_blk.cc
@@ -54,7 +54,7 @@
const int src_requestor_ID, const uint32_t task_ID)
{
// Make sure that the block has been properly invalidated
- assert(status == 0);
+ assert(!isValid());
// Set block tag
setTag(tag);
diff --git a/src/mem/cache/cache_blk.hh b/src/mem/cache/cache_blk.hh
index 0099f7a..d1dd5f3 100644
--- a/src/mem/cache/cache_blk.hh
+++ b/src/mem/cache/cache_blk.hh
@@ -60,26 +60,6 @@
#include "sim/core.hh"
/**
- * Cache block status bit assignments
- */
-enum CacheBlkStatusBits : unsigned {
- /** valid, readable */
- BlkValid = 0x01,
- /** write permission */
- BlkWritable = 0x02,
- /** read permission (yes, block can be valid but not readable) */
- BlkReadable = 0x04,
- /** dirty (modified) */
- BlkDirty = 0x08,
- /** block was a hardware prefetch yet unaccessed*/
- BlkHWPrefetched = 0x20,
- /** block holds data from the secure memory space */
- BlkSecure = 0x40,
- /** block holds compressed data */
- BlkCompressed = 0x80
-};
-
-/**
* A Basic Cache block.
* Contains the tag, status, and a pointer to data.
*/
@@ -105,6 +85,27 @@
Tick _tickInserted;
protected:
+ /** Cache block status bit assignments. */
+ enum CacheBlkStatusBits : unsigned {
+ /** valid, readable */
+ BlkValid = 0x01,
+ /** write permission */
+ BlkWritable = 0x02,
+ /** read permission (yes, block can be valid but not readable) */
+ BlkReadable = 0x04,
+ /** dirty (modified) */
+ BlkDirty = 0x08,
+ /** block was a hardware prefetch yet unaccessed*/
+ BlkHWPrefetched = 0x20,
+ /** block holds data from the secure memory space */
+ BlkSecure = 0x40,
+ /** block holds compressed data */
+ BlkCompressed = 0x80
+ };
+
+ /** The current status of this block. @sa CacheBlockStatusBits */
+ unsigned status;
+
/** Set the task id value. */
inline void setTaskId(const uint32_t task_id) { _taskId = task_id; }
@@ -117,6 +118,23 @@
/** Set the current tick as this block's insertion tick. */
inline void setTickInserted() { _tickInserted = curTick(); }
+ /** Mark the block as valid. */
+ virtual void
+ setValid()
+ {
+ assert(!isValid());
+ status |= BlkValid;
+ }
+
+ /** Mark the block as invalid. */
+ void clearValid() { status &= ~BlkValid; }
+
+ /** Marks the block as coming from a secure memory region. */
+ virtual void setSecure() { status |= BlkSecure; }
+
+ /** Marks the block as coming from a regular memory region. */
+ void clearSecure() { status &= ~BlkSecure; }
+
public:
/**
* Contains a copy of the data in this block for easy access. This is
used
@@ -127,12 +145,6 @@
*/
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.
@@ -191,69 +203,94 @@
CacheBlk& operator=(const CacheBlk&) = delete;
virtual ~CacheBlk() {};
+ /** Marks the block as writable. */
+ void
+ setWritable()
+ {
+ assert(isValid());
+ status |= BlkWritable;
+ }
+ /** Marks the block as not writable. */
+ void clearWritable() { status &= ~BlkWritable; }
/**
* Checks the write permissions of this block.
* @return True if the block is writable.
*/
- bool isWritable() const
- {
- const State needed_bits = BlkWritable | BlkValid;
- return (status & needed_bits) == needed_bits;
- }
+ bool isWritable() const { return isValid() && (status & BlkWritable); }
+ /** Marks the block as readable. */
+ void
+ setReadable()
+ {
+ assert(isValid());
+ status |= BlkReadable;
+ }
+ /** Marks the block as not readable. */
+ void clearReadable() { status &= ~BlkReadable; }
/**
* 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
- {
- const State needed_bits = BlkReadable | BlkValid;
- return (status & needed_bits) == needed_bits;
- }
+ bool isReadable() const { return isValid() && (status & BlkReadable); }
/**
* Checks that a block is valid.
* @return True if the block is valid.
*/
- bool isValid() const
- {
- return (status & BlkValid) != 0;
- }
+ bool isValid() const { return (status & BlkValid) != 0; }
/**
* Invalidate the block and clear all state.
*/
virtual void invalidate()
{
+ clearValid();
+ clearSecure();
+ clearWritable();
+ clearReadable();
+ clearDirty();
+ clearPrefetched();
+
setTag(MaxAddr);
setTaskId(ContextSwitchTaskId::Unknown);
- status = 0;
whenReady = MaxTick;
setRefCount(0);
setSrcRequestorId(Request::invldRequestorId);
lockList.clear();
}
+ /** Marks the block as dirty. */
+ void
+ setDirty()
+ {
+ assert(isValid());
+ status |= BlkDirty;
+ }
+ /** Marks the block as not dirty. */
+ void clearDirty() { status &= ~BlkDirty; }
/**
* Check to see if a block has been written.
* @return True if the block is dirty.
*/
- bool isDirty() const
- {
- return (status & BlkDirty) != 0;
- }
+ bool isDirty() const { return (status & BlkDirty) != 0; }
+ /** Marks the block as prefetched. */
+ void
+ setPrefetched()
+ {
+ assert(isValid());
+ status |= BlkHWPrefetched;
+ }
+ /** Marks the block as not prefetched. */
+ void clearPrefetched() { status &= ~BlkHWPrefetched; }
/**
* Check if this block was the result of a hardware prefetch, yet to
* be touched.
* @return True if the block was a hardware prefetch, unaccesed.
*/
- bool wasPrefetched() const
- {
- return (status & BlkHWPrefetched) != 0;
- }
+ bool wasPrefetched() const { return (status & BlkHWPrefetched) != 0; }
/**
* Get tag associated to this block.
@@ -273,27 +310,7 @@
* Check if this block holds data from the secure memory space.
* @return True if the block holds data from the secure memory space.
*/
- bool isSecure() const
- {
- return (status & BlkSecure) != 0;
- }
-
- /**
- * Set valid bit.
- */
- virtual void setValid()
- {
- assert(!isValid());
- status |= BlkValid;
- }
-
- /**
- * Set secure bit.
- */
- virtual void setSecure()
- {
- status |= BlkSecure;
- }
+ bool isSecure() const { return (status & BlkSecure) != 0; }
/**
* Get tick at which block's data will be available for access.
@@ -535,7 +552,7 @@
override
{
// Make sure that the block has been properly invalidated
- assert(status == 0);
+ assert(!isValid());
// Set block address
_addr = addr;
diff --git a/src/mem/cache/noncoherent_cache.cc
b/src/mem/cache/noncoherent_cache.cc
index 5ca1da0..1b1bc13 100644
--- a/src/mem/cache/noncoherent_cache.cc
+++ b/src/mem/cache/noncoherent_cache.cc
@@ -83,7 +83,7 @@
// 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->setWritable();
}
return success;
@@ -287,8 +287,9 @@
// attached to this cache
assert(tgt_pkt->cmd == MemCmd::HardPFReq);
- if (blk)
- blk->status |= BlkHWPrefetched;
+ if (blk) {
+ blk->setPrefetched();
+ }
// We have filled the block and the prefetcher does not
// require responses.
diff --git a/src/mem/cache/tags/super_blk.cc
b/src/mem/cache/tags/super_blk.cc
index 982d9b0..3cc4368 100644
--- a/src/mem/cache/tags/super_blk.cc
+++ b/src/mem/cache/tags/super_blk.cc
@@ -83,6 +83,13 @@
_decompressionLatency = lat;
}
+void
+CompressionBlk::invalidate()
+{
+ setUncompressed();
+ SectorSubBlk::invalidate();
+}
+
std::string
CompressionBlk::print() const
{
diff --git a/src/mem/cache/tags/super_blk.hh
b/src/mem/cache/tags/super_blk.hh
index a1c3ff4..6441cbf 100644
--- a/src/mem/cache/tags/super_blk.hh
+++ b/src/mem/cache/tags/super_blk.hh
@@ -110,6 +110,8 @@
*/
void setDecompressionLatency(const Cycles lat);
+ void invalidate() override;
+
/**
* Pretty-print sector offset and other CacheBlk information.
*
--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/34960
To unsubscribe, or for help writing mail filters, visit
https://gem5-review.googlesource.com/settings
Gerrit-Project: public/gem5
Gerrit-Branch: develop
Gerrit-Change-Id: I558cc51ac685d30b6bf298c78f86a6e24ff06973
Gerrit-Change-Number: 34960
Gerrit-PatchSet: 1
Gerrit-Owner: Daniel Carvalho <oda...@yahoo.com.br>
Gerrit-MessageType: newchange
_______________________________________________
gem5-dev mailing list -- gem5-dev@gem5.org
To unsubscribe send an email to gem5-dev-le...@gem5.org
%(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s
