changeset 0d4ee33078bb in /z/repo/gem5
details: http://repo.gem5.org/gem5?cmd=changeset;node=0d4ee33078bb
description:
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.
diffstat:
src/mem/cache/mshr.cc | 94 ++++++++++++++++++--------------------------
src/mem/cache/mshr.hh | 81 +++++++++++++++++++-------------------
src/mem/cache/mshr_queue.cc | 18 ++------
src/mem/cache/mshr_queue.hh | 23 ++++------
4 files changed, 95 insertions(+), 121 deletions(-)
diffs (truncated from 510 to 300 lines):
diff -r 7c7ed0cae353 -r 0d4ee33078bb src/mem/cache/mshr.cc
--- a/src/mem/cache/mshr.cc Thu May 30 12:54:09 2013 -0400
+++ b/src/mem/cache/mshr.cc Thu May 30 12:54:11 2013 -0400
@@ -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 @@
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 @@
downstreamPending = false;
// recursively clear flag on any MSHRs we will be forwarding
// responses to
- targets->clearDownstreamPending();
+ targets.clearDownstreamPending();
}
bool
@@ -249,14 +247,14 @@
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 @@
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 @@
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 @@
// 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 @@
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 @@
// 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 @@
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 @@
{
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 @@
// 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 @@
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 @@
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 @@
print(str);
return str.str();
}
-
-MSHR::~MSHR()
-{
- delete[] targets;
- delete[] deferredTargets;
-}
diff -r 7c7ed0cae353 -r 0d4ee33078bb src/mem/cache/mshr.hh
--- a/src/mem/cache/mshr.hh Thu May 30 12:54:09 2013 -0400
+++ b/src/mem/cache/mshr.hh Thu May 30 12:54:11 2013 -0400
@@ -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 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 @@
/** 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 @@
/** 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.
_______________________________________________
gem5-dev mailing list
[email protected]
http://m5sim.org/mailman/listinfo/gem5-dev
