summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorAndreas Hansson <andreas.hansson@arm.com>2013-05-30 12:54:11 -0400
committerAndreas Hansson <andreas.hansson@arm.com>2013-05-30 12:54:11 -0400
commit7da851d1a834fbe6dd02f87884586129786b14a6 (patch)
treeba426b7526b9480c38ebead26fc320f7fcbec347
parent42191522cc16f0f1c98455e52681a928cba4052a (diff)
downloadgem5-7da851d1a834fbe6dd02f87884586129786b14a6.tar.xz
mem: Spring cleaning of MSHR and MSHRQueue
This patch does some minor tidying up of the MSHR and MSHRQueue. The clean up started as part of some ad-hoc tracing and debugging, but seems worthwhile enough to go in as a separate patch. The highlights of the changes are reduced scoping (private) members where possible, avoiding redundant new/delete, and constructor initialisation to please static code analyzers.
-rw-r--r--src/mem/cache/mshr.cc94
-rw-r--r--src/mem/cache/mshr.hh81
-rw-r--r--src/mem/cache/mshr_queue.cc18
-rw-r--r--src/mem/cache/mshr_queue.hh23
4 files changed, 95 insertions, 121 deletions
diff --git a/src/mem/cache/mshr.cc b/src/mem/cache/mshr.cc
index ac16568e8..f96c5c1a7 100644
--- a/src/mem/cache/mshr.cc
+++ b/src/mem/cache/mshr.cc
@@ -61,13 +61,12 @@
using namespace std;
-MSHR::MSHR()
+MSHR::MSHR() : readyTime(0), _isUncacheable(false), downstreamPending(false),
+ pendingDirty(false), postInvalidate(false),
+ postDowngrade(false), queue(NULL), order(0), addr(0), size(0),
+ inService(false), isForward(false), threadNum(InvalidThreadID),
+ data(NULL)
{
- inService = false;
- ntargets = 0;
- threadNum = InvalidThreadID;
- targets = new TargetList();
- deferredTargets = new TargetList();
}
@@ -215,14 +214,13 @@ MSHR::allocate(Addr _addr, int _size, PacketPtr target,
inService = false;
downstreamPending = false;
threadNum = 0;
- ntargets = 1;
- assert(targets->isReset());
+ assert(targets.isReset());
// 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
Target::Source source = (target->cmd == MemCmd::HardPFReq) ?
Target::FromPrefetcher : Target::FromCPU;
- targets->add(target, whenReady, _order, source, true);
- assert(deferredTargets->isReset());
+ targets.add(target, whenReady, _order, source, true);
+ assert(deferredTargets.isReset());
data = NULL;
}
@@ -234,7 +232,7 @@ MSHR::clearDownstreamPending()
downstreamPending = false;
// recursively clear flag on any MSHRs we will be forwarding
// responses to
- targets->clearDownstreamPending();
+ targets.clearDownstreamPending();
}
bool
@@ -249,14 +247,14 @@ MSHR::markInService(PacketPtr pkt)
return true;
}
inService = true;
- pendingDirty = (targets->needsExclusive ||
+ pendingDirty = (targets.needsExclusive ||
(!pkt->sharedAsserted() && pkt->memInhibitAsserted()));
postInvalidate = postDowngrade = false;
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();
+ targets.clearDownstreamPending();
}
return false;
}
@@ -265,10 +263,9 @@ MSHR::markInService(PacketPtr pkt)
void
MSHR::deallocate()
{
- assert(targets->empty());
- targets->resetFlags();
- assert(deferredTargets->isReset());
- assert(ntargets == 0);
+ assert(targets.empty());
+ targets.resetFlags();
+ assert(deferredTargets.isReset());
inService = false;
}
@@ -294,22 +291,20 @@ MSHR::allocateTarget(PacketPtr pkt, Tick whenReady, Counter _order)
assert(pkt->cmd != MemCmd::HardPFReq);
if (inService &&
- (!deferredTargets->empty() || hasPostInvalidate() ||
+ (!deferredTargets.empty() || hasPostInvalidate() ||
(pkt->needsExclusive() &&
(!isPendingDirty() || hasPostDowngrade() || isForward)))) {
// need to put on deferred list
if (hasPostInvalidate())
replaceUpgrade(pkt);
- deferredTargets->add(pkt, whenReady, _order, Target::FromCPU, true);
+ deferredTargets.add(pkt, whenReady, _order, Target::FromCPU, true);
} else {
// No request outstanding, or still OK to append to
// outstanding request: append to regular target list. Only
// mark pending if current request hasn't been issued yet
// (isn't in service).
- targets->add(pkt, whenReady, _order, Target::FromCPU, !inService);
+ targets.add(pkt, whenReady, _order, Target::FromCPU, !inService);
}
-
- ++ntargets;
}
bool
@@ -332,8 +327,8 @@ MSHR::handleSnoop(PacketPtr pkt, Counter _order)
// local bus first, some other invalidating transaction
// reached the global bus before the upgrade did.
if (pkt->needsExclusive()) {
- targets->replaceUpgrades();
- deferredTargets->replaceUpgrades();
+ targets.replaceUpgrades();
+ deferredTargets.replaceUpgrades();
}
return false;
@@ -344,7 +339,7 @@ MSHR::handleSnoop(PacketPtr pkt, Counter _order)
if (pkt->needsExclusive()) {
// snooped request still precedes the re-request we'll have to
// issue for deferred targets, if any...
- deferredTargets->replaceUpgrades();
+ deferredTargets.replaceUpgrades();
}
if (hasPostInvalidate()) {
@@ -365,9 +360,8 @@ MSHR::handleSnoop(PacketPtr pkt, Counter _order)
// Actual target device (typ. a memory) will delete the
// packet on reception, so we need to save a copy here.
PacketPtr cp_pkt = new Packet(pkt, true);
- targets->add(cp_pkt, curTick(), _order, Target::FromSnoop,
- downstreamPending && targets->needsExclusive);
- ++ntargets;
+ targets.add(cp_pkt, curTick(), _order, Target::FromSnoop,
+ downstreamPending && targets.needsExclusive);
if (isPendingDirty()) {
pkt->assertMemInhibit();
@@ -394,23 +388,19 @@ MSHR::handleSnoop(PacketPtr pkt, Counter _order)
bool
MSHR::promoteDeferredTargets()
{
- assert(targets->empty());
- if (deferredTargets->empty()) {
+ assert(targets.empty());
+ if (deferredTargets.empty()) {
return false;
}
// swap targets & deferredTargets lists
- TargetList *tmp = targets;
- targets = deferredTargets;
- deferredTargets = tmp;
-
- assert(targets->size() == ntargets);
+ std::swap(targets, deferredTargets);
// clear deferredTargets flags
- deferredTargets->resetFlags();
+ deferredTargets.resetFlags();
- order = targets->front().order;
- readyTime = std::max(curTick(), targets->front().readyTime);
+ order = targets.front().order;
+ readyTime = std::max(curTick(), targets.front().readyTime);
return true;
}
@@ -421,7 +411,7 @@ MSHR::handleFill(Packet *pkt, CacheBlk *blk)
{
if (!pkt->sharedAsserted()
&& !(hasPostInvalidate() || hasPostDowngrade())
- && deferredTargets->needsExclusive) {
+ && deferredTargets.needsExclusive) {
// We got an exclusive response, but we have deferred targets
// which are waiting to request an exclusive copy (not because
// of a pending invalidate). This can happen if the original
@@ -430,15 +420,15 @@ MSHR::handleFill(Packet *pkt, CacheBlk *blk)
// MOESI/MESI protocol. Since we got the exclusive copy
// there's no need to defer the targets, so move them up to
// the regular target list.
- assert(!targets->needsExclusive);
- targets->needsExclusive = true;
+ assert(!targets.needsExclusive);
+ targets.needsExclusive = true;
// if any of the deferred targets were upper-level cache
// requests marked downstreamPending, need to clear that
assert(!downstreamPending); // not pending here anymore
- deferredTargets->clearDownstreamPending();
+ deferredTargets.clearDownstreamPending();
// this clears out deferredTargets too
- targets->splice(targets->end(), *deferredTargets);
- deferredTargets->resetFlags();
+ targets.splice(targets.end(), deferredTargets);
+ deferredTargets.resetFlags();
}
}
@@ -453,8 +443,8 @@ MSHR::checkFunctional(PacketPtr pkt)
pkt->checkFunctional(this, addr, size, NULL);
return false;
} else {
- return (targets->checkFunctional(pkt) ||
- deferredTargets->checkFunctional(pkt));
+ return (targets.checkFunctional(pkt) ||
+ deferredTargets.checkFunctional(pkt));
}
}
@@ -474,10 +464,10 @@ MSHR::print(std::ostream &os, int verbosity, const std::string &prefix) const
hasPostDowngrade() ? "PostDowngr" : "");
ccprintf(os, "%s Targets:\n", prefix);
- targets->print(os, verbosity, prefix + " ");
- if (!deferredTargets->empty()) {
+ targets.print(os, verbosity, prefix + " ");
+ if (!deferredTargets.empty()) {
ccprintf(os, "%s Deferred Targets:\n", prefix);
- deferredTargets->print(os, verbosity, prefix + " ");
+ deferredTargets.print(os, verbosity, prefix + " ");
}
}
@@ -488,9 +478,3 @@ MSHR::print() const
print(str);
return str.str();
}
-
-MSHR::~MSHR()
-{
- delete[] targets;
- delete[] deferredTargets;
-}
diff --git a/src/mem/cache/mshr.hh b/src/mem/cache/mshr.hh
index f99a293fd..c9c30b3e6 100644
--- a/src/mem/cache/mshr.hh
+++ b/src/mem/cache/mshr.hh
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2012 ARM Limited
+ * Copyright (c) 2012-2013 ARM Limited
* All rights reserved.
*
* The license below extends only to copyright in the software and shall
@@ -64,6 +64,31 @@ class MSHRQueue;
class MSHR : public Packet::SenderState, public Printable
{
+ /**
+ * Consider the MSHRQueue a friend to avoid making everything public
+ */
+ friend class MSHRQueue;
+
+ private:
+
+ /** Cycle when ready to issue */
+ Tick readyTime;
+
+ /** True if the request is uncacheable */
+ bool _isUncacheable;
+
+ /** Flag set by downstream caches */
+ bool downstreamPending;
+
+ /** Will we have a dirty copy after this request? */
+ bool pendingDirty;
+
+ /** Did we snoop an invalidate while waiting for data? */
+ bool postInvalidate;
+
+ /** Did we snoop a read while waiting for data? */
+ bool postDowngrade;
+
public:
class Target {
@@ -121,9 +146,6 @@ class MSHR : public Packet::SenderState, public Printable
/** Pointer to queue containing this MSHR. */
MSHRQueue *queue;
- /** Cycle when ready to issue */
- Tick readyTime;
-
/** Order number assigned by the miss queue. */
Counter order;
@@ -139,42 +161,30 @@ class MSHR : public Packet::SenderState, public Printable
/** 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; }
-
- /** True if the request is uncacheable */
- bool _isUncacheable;
-
- bool downstreamPending;
-
/** The pending* and post* flags are only valid if inService is
* true. Using the accessor functions lets us detect if these
* flags are accessed improperly.
*/
- /** Will we have a dirty copy after this request? */
- bool pendingDirty;
+ /** True if we need to get an exclusive copy of the block. */
+ bool needsExclusive() const { return targets.needsExclusive; }
+
bool isPendingDirty() const {
assert(inService); return pendingDirty;
}
- /** Did we snoop an invalidate while waiting for data? */
- bool postInvalidate;
bool hasPostInvalidate() const {
assert(inService); return postInvalidate;
}
- /** Did we snoop a read while waiting for data? */
- bool postDowngrade;
bool hasPostDowngrade() const {
assert(inService); return postDowngrade;
}
/** Thread number of the miss. */
ThreadID threadNum;
- /** The number of currently allocated targets. */
- unsigned short ntargets;
+ private:
/** Data buffer (if needed). Currently used only for pending
* upgrade handling. */
@@ -192,15 +202,14 @@ class MSHR : public Packet::SenderState, public Printable
*/
Iterator allocIter;
-private:
/** List of all requests that match the address */
- TargetList *targets;
+ TargetList targets;
- TargetList *deferredTargets;
+ TargetList deferredTargets;
-public:
+ public:
- bool isUncacheable() { return _isUncacheable; }
+ bool isUncacheable() const { return _isUncacheable; }
/**
* Allocate a miss to this MSHR.
@@ -231,35 +240,28 @@ public:
/** A simple constructor. */
MSHR();
- /** A simple destructor. */
- ~MSHR();
/**
* Returns the current number of allocated targets.
* @return The current number of allocated targets.
*/
- int getNumTargets() const { return ntargets; }
-
- /**
- * Returns a pointer to the target list.
- * @return a pointer to the target list.
- */
- TargetList *getTargetList() { return targets; }
+ int getNumTargets() const
+ { return targets.size() + deferredTargets.size(); }
/**
* Returns true if there are targets left.
* @return true if there are targets
*/
- bool hasTargets() const { 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() const
+ Target *getTarget()
{
assert(hasTargets());
- return &targets->front();
+ return &targets.front();
}
/**
@@ -267,15 +269,14 @@ public:
*/
void popTarget()
{
- --ntargets;
- targets->pop_front();
+ targets.pop_front();
}
bool isForwardNoResponse() const
{
if (getNumTargets() != 1)
return false;
- Target *tgt = getTarget();
+ const Target *tgt = &targets.front();
return tgt->source == Target::FromCPU && !tgt->pkt->needsResponse();
}
diff --git a/src/mem/cache/mshr_queue.cc b/src/mem/cache/mshr_queue.cc
index af13d12d3..d8cc5f40a 100644
--- a/src/mem/cache/mshr_queue.cc
+++ b/src/mem/cache/mshr_queue.cc
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2012 ARM Limited
+ * Copyright (c) 2012-2013 ARM Limited
* All rights reserved.
*
* The license below extends only to copyright in the software and shall
@@ -51,24 +51,16 @@ using namespace std;
MSHRQueue::MSHRQueue(const std::string &_label,
int num_entries, int reserve, int _index)
- : label(_label),
- numEntries(num_entries + reserve - 1), numReserve(reserve),
- drainManager(NULL), index(_index)
+ : label(_label), numEntries(num_entries + reserve - 1),
+ numReserve(reserve), registers(numEntries),
+ drainManager(NULL), allocated(0), inServiceEntries(0), index(_index)
{
- allocated = 0;
- inServiceEntries = 0;
- registers = new MSHR[numEntries];
for (int i = 0; i < numEntries; ++i) {
registers[i].queue = this;
freeList.push_back(&registers[i]);
}
}
-MSHRQueue::~MSHRQueue()
-{
- delete [] registers;
-}
-
MSHR *
MSHRQueue::findMatch(Addr addr) const
{
@@ -253,7 +245,7 @@ MSHRQueue::squash(int threadNum)
assert(0/*target->req->threadId()*/ == threadNum);
}
assert(!mshr->hasTargets());
- assert(mshr->ntargets==0);
+ assert(mshr->getNumTargets()==0);
if (!mshr->inService) {
i = deallocateOne(mshr);
} else {
diff --git a/src/mem/cache/mshr_queue.hh b/src/mem/cache/mshr_queue.hh
index 44e1c5bd3..726aa6b8e 100644
--- a/src/mem/cache/mshr_queue.hh
+++ b/src/mem/cache/mshr_queue.hh
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2012 ARM Limited
+ * Copyright (c) 2012-2013 ARM Limited
* All rights reserved.
*
* The license below extends only to copyright in the software and shall
@@ -63,15 +63,6 @@ class MSHRQueue : public Drainable
/** Local label (for functional print requests) */
const std::string label;
- /** MSHR storage. */
- MSHR *registers;
- /** Holds pointers to all allocated entries. */
- MSHR::List allocatedList;
- /** Holds pointers to entries that haven't been sent to the bus. */
- MSHR::List readyList;
- /** Holds non allocated entries. */
- MSHR::List freeList;
-
// Parameters
/**
* The total number of entries in this queue. This number is set as the
@@ -86,6 +77,15 @@ class MSHRQueue : public Drainable
*/
const int numReserve;
+ /** MSHR storage. */
+ std::vector<MSHR> registers;
+ /** Holds pointers to all allocated entries. */
+ MSHR::List allocatedList;
+ /** Holds pointers to entries that haven't been sent to the bus. */
+ MSHR::List readyList;
+ /** Holds non allocated entries. */
+ MSHR::List freeList;
+
/** Drain manager to inform of a completed drain */
DrainManager *drainManager;
@@ -110,9 +110,6 @@ class MSHRQueue : public Drainable
MSHRQueue(const std::string &_label, int num_entries, int reserve,
int index);
- /** Destructor */
- ~MSHRQueue();
-
/**
* Find the first MSHR that matches the provided address.
* @param addr The address to find.