Daniel Carvalho has uploaded this change for review. (
https://gem5-review.googlesource.com/c/public/gem5/+/17531
Change subject: mem-cache: Remove intermediate blk address in queue_entry
......................................................................
mem-cache: Remove intermediate blk address in queue_entry
A queue entry was created due to a writeback (or similar) happening,
or a miss, and these have packets. These packets have a copy of the
address, and therefore although having a local copy would speed
things up, it adds assumption of the packet model to the queue entry.
This blocks, for example, the introduction of compressed packets,
which may refer to multiple addresses instead of just one. With this
change we forward the address retrieval to the packet.
Change-Id: Ic3b6c4693461932f0951840518df5053b9b3ab4e
Signed-off-by: Daniel R. Carvalho <[email protected]>
---
M src/mem/cache/mshr.cc
M src/mem/cache/mshr.hh
M src/mem/cache/queue_entry.hh
M src/mem/cache/write_queue_entry.cc
M src/mem/cache/write_queue_entry.hh
5 files changed, 45 insertions(+), 16 deletions(-)
diff --git a/src/mem/cache/mshr.cc b/src/mem/cache/mshr.cc
index 479a1b4..a4cbeed 100644
--- a/src/mem/cache/mshr.cc
+++ b/src/mem/cache/mshr.cc
@@ -253,7 +253,6 @@
MSHR::allocate(Addr blk_addr, unsigned blk_size, PacketPtr target,
Tick when_ready, Counter _order, bool alloc_on_fill)
{
- blkAddr = blk_addr;
blkSize = blk_size;
_isSecure = target->isSecure();
readyTime = when_ready;
@@ -265,8 +264,8 @@
inService = false;
downstreamPending = false;
- targets.init(blkAddr, blkSize);
- deferredTargets.init(blkAddr, blkSize);
+ targets.init(blk_addr, blkSize);
+ deferredTargets.init(blk_addr, blkSize);
// Don't know of a case where we would allocate a new MSHR for a
// snoop (mem-side request), so set source according to request here
@@ -490,7 +489,7 @@
MSHR::extractServiceableTargets(PacketPtr pkt)
{
TargetList ready_targets;
- ready_targets.init(blkAddr, blkSize);
+ ready_targets.init(getAddr(), blkSize);
// If the downstream MSHR got an invalidation request then we only
// service the first of the FromCPU targets and any other
// non-FromCPU target. This way the remaining FromCPU targets
@@ -635,7 +634,8 @@
// For other requests, we iterate over the individual targets
// since that's where the actual data lies.
if (pkt->isPrint()) {
- pkt->trySatisfyFunctional(this, blkAddr, _isSecure, blkSize,
nullptr);
+ pkt->trySatisfyFunctional(this, getAddr(), _isSecure, blkSize,
+ nullptr);
return false;
} else {
return (targets.trySatisfyFunctional(pkt) ||
@@ -653,7 +653,7 @@
MSHR::print(std::ostream &os, int verbosity, const std::string &prefix)
const
{
ccprintf(os, "%s[%#llx:%#llx](%s) %s %s %s state: %s %s %s %s %s %s\n",
- prefix, blkAddr, blkAddr + blkSize - 1,
+ prefix, getAddr(), getAddr() + blkSize - 1,
isSecure() ? "s" : "ns",
isForward ? "Forward" : "",
allocOnFill() ? "AllocOnFill" : "",
diff --git a/src/mem/cache/mshr.hh b/src/mem/cache/mshr.hh
index b94dfb9..9d65d88 100644
--- a/src/mem/cache/mshr.hh
+++ b/src/mem/cache/mshr.hh
@@ -483,6 +483,16 @@
}
/**
+ * The address of any of the MSHR targets is the entry's address.
+ * @return The address to which this entry refers.
+ */
+ Addr getAddr() const override
+ {
+ assert(hasTargets());
+ return targets.front().pkt->getBlockAddr(blkSize);
+ }
+
+ /**
* Pop first target.
*/
void popTarget()
diff --git a/src/mem/cache/queue_entry.hh b/src/mem/cache/queue_entry.hh
index 17426e7..3d4b92d 100644
--- a/src/mem/cache/queue_entry.hh
+++ b/src/mem/cache/queue_entry.hh
@@ -68,10 +68,6 @@
friend class Queue;
protected:
-
- /** Block aligned address. */
- Addr blkAddr;
-
/** True if the entry targets the secure memory space. */
bool _isSecure;
@@ -93,11 +89,21 @@
unsigned blkSize;
QueueEntry()
- : blkAddr(0), _isSecure(false), readyTime(0),
_isUncacheable(false),
+ : _isSecure(false), readyTime(0), _isUncacheable(false),
inService(false), order(0), blkSize(0)
{}
- Addr getAddr() const { return blkAddr; }
+ /**
+ * The address of the block to which this entry refers is not stored in
+ * the entry itself, but in the packet that originated it. We do not
keep
+ * a copy because if the packet is compressed, this may incur in
extending
+ * compression handling to the queue entry. This should ultimately be
+ * removed, and the address(es) must be only known and handled by the
+ * packet.
+ *
+ * @return The address to which this entry refers.
+ */
+ virtual Addr getAddr() const = 0;
bool isSecure() const { return _isSecure; }
@@ -111,7 +117,7 @@
* @return True if entry matches given information.
*/
bool match(const Addr addr, const bool is_secure) const {
- return (blkAddr == addr) && (_isSecure == is_secure);
+ return (getAddr() == addr) && (_isSecure == is_secure);
}
/**
diff --git a/src/mem/cache/write_queue_entry.cc
b/src/mem/cache/write_queue_entry.cc
index eab9386..cfc16e9 100644
--- a/src/mem/cache/write_queue_entry.cc
+++ b/src/mem/cache/write_queue_entry.cc
@@ -91,7 +91,6 @@
WriteQueueEntry::allocate(Addr blk_addr, unsigned blk_size, PacketPtr
target,
Tick when_ready, Counter _order)
{
- blkAddr = blk_addr;
blkSize = blk_size;
_isSecure = target->isSecure();
readyTime = when_ready;
@@ -128,7 +127,8 @@
// entity. For other requests, we iterate over the individual
// targets since that's where the actual data lies.
if (pkt->isPrint()) {
- pkt->trySatisfyFunctional(this, blkAddr, _isSecure, blkSize,
nullptr);
+ pkt->trySatisfyFunctional(this, getAddr(), _isSecure, blkSize,
+ nullptr);
return false;
} else {
return targets.trySatisfyFunctional(pkt);
@@ -146,7 +146,7 @@
const std::string &prefix) const
{
ccprintf(os, "%s[%#llx:%#llx](%s) %s %s %s state: %s %s %s %s %s\n",
- prefix, blkAddr, blkAddr + blkSize - 1,
+ prefix, getAddr(), getAddr() + blkSize - 1,
isSecure() ? "s" : "ns",
_isUncacheable ? "Unc" : "",
inService ? "InSvc" : "");
diff --git a/src/mem/cache/write_queue_entry.hh
b/src/mem/cache/write_queue_entry.hh
index f4ccb1a..800ba4f 100644
--- a/src/mem/cache/write_queue_entry.hh
+++ b/src/mem/cache/write_queue_entry.hh
@@ -126,6 +126,19 @@
/** List of all requests that match the address */
TargetList targets;
+ /**
+ * Get one of the packets' address and use it as a representative of
the
+ * entry's address. Should only be used for print calls since it
assumes
+ * each packet has a single address.
+ *
+ * @return One of the addresses this entry refers to.
+ */
+ Addr getAddr() const override
+ {
+ assert(hasTargets());
+ return targets.front().pkt->getBlockAddr(blkSize);
+ }
+
public:
/** A simple constructor. */
--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/17531
To unsubscribe, or for help writing mail filters, visit
https://gem5-review.googlesource.com/settings
Gerrit-Project: public/gem5
Gerrit-Branch: master
Gerrit-Change-Id: Ic3b6c4693461932f0951840518df5053b9b3ab4e
Gerrit-Change-Number: 17531
Gerrit-PatchSet: 1
Gerrit-Owner: Daniel Carvalho <[email protected]>
Gerrit-MessageType: newchange
_______________________________________________
gem5-dev mailing list
[email protected]
http://m5sim.org/mailman/listinfo/gem5-dev
