changeset bd376adfb7d4 in /z/repo/gem5
details: http://repo.gem5.org/gem5?cmd=changeset;node=bd376adfb7d4
description:
mem: Make the XBar responsible for tracking response routing
This patch removes the need for a source and destination field in the
packet by shifting the onus of the tracking to the crossbar, much like
a real implementation. This change in behaviour also means we no
longer need a SenderState to remember the source/dest when ever we
have multiple crossbars in the system. Thus, the stack that was
created by the SenderState is not needed, and each crossbar locally
tracks the response routing.
The fields in the packet are still left behind as the RubyPort (which
also acts as a crossbar) does routing based on them. In the succeeding
patches the uses of the src and dest field will be removed. Combined,
these patches improve the simulation performance by roughly 2%.
diffstat:
src/mem/coherent_xbar.cc | 140 ++++++++++++++++++++++++-------------------
src/mem/coherent_xbar.hh | 9 +-
src/mem/noncoherent_xbar.cc | 23 +++++-
src/mem/xbar.hh | 9 ++
4 files changed, 108 insertions(+), 73 deletions(-)
diffs (truncated from 360 to 300 lines):
diff -r b7ff344c3061 -r bd376adfb7d4 src/mem/coherent_xbar.cc
--- a/src/mem/coherent_xbar.cc Thu Jan 22 05:00:57 2015 -0500
+++ b/src/mem/coherent_xbar.cc Thu Jan 22 05:01:14 2015 -0500
@@ -142,6 +142,10 @@
// remember if the packet is an express snoop
bool is_express_snoop = pkt->isExpressSnoop();
+ bool is_inhibited = pkt->memInhibitAsserted();
+ // for normal requests, going downstream, the express snoop flag
+ // and the inhibited flag should always be the same
+ assert(is_express_snoop == is_inhibited);
// determine the destination based on the address
PortID master_port_id = findPort(pkt->getAddr());
@@ -163,9 +167,6 @@
unsigned int pkt_size = pkt->hasData() ? pkt->getSize() : 0;
unsigned int pkt_cmd = pkt->cmdToIndex();
- // set the source port for routing of the response
- pkt->setSrc(slave_port_id);
-
calcPacketTiming(pkt);
Tick packetFinishTime = pkt->lastWordDelay + curTick();
@@ -187,21 +188,10 @@
}
}
- // remember if we add an outstanding req so we can undo it if
- // necessary, if the packet needs a response, we should add it
- // as outstanding and express snoops never fail so there is
- // not need to worry about them
- bool add_outstanding = !is_express_snoop && pkt->needsResponse();
-
- // keep track that we have an outstanding request packet
- // matching this request, this is used by the coherency
- // mechanism in determining what to do with snoop responses
- // (in recvTimingSnoop)
- if (add_outstanding) {
- // we should never have an exsiting request outstanding
- assert(outstandingReq.find(pkt->req) == outstandingReq.end());
- outstandingReq.insert(pkt->req);
- }
+ // remember if the packet will generate a snoop response
+ const bool expect_snoop_resp = !is_inhibited && pkt->memInhibitAsserted();
+ const bool expect_response = pkt->needsResponse() &&
+ !pkt->memInhibitAsserted();
// Note: Cannot create a copy of the full packet, here.
MemCmd orig_cmd(pkt->cmd);
@@ -224,41 +214,58 @@
pkt->cmd = tmp_cmd;
}
- // if this is an express snoop, we are done at this point
- if (is_express_snoop) {
- assert(success);
- snoops++;
+ // check if we were successful in sending the packet onwards
+ if (!success) {
+ // express snoops and inhibited packets should never be forced
+ // to retry
+ assert(!is_express_snoop);
+ assert(!pkt->memInhibitAsserted());
+
+ // undo the calculation so we can check for 0 again
+ pkt->firstWordDelay = pkt->lastWordDelay = 0;
+
+ DPRINTF(CoherentXBar, "recvTimingReq: src %s %s 0x%x RETRY\n",
+ src_port->name(), pkt->cmdString(), pkt->getAddr());
+
+ // update the layer state and schedule an idle event
+ reqLayers[master_port_id]->failedTiming(src_port,
+ clockEdge(headerCycles));
} else {
- // for normal requests, check if successful
- if (!success) {
- // inhibited packets should never be forced to retry
- assert(!pkt->memInhibitAsserted());
+ // express snoops currently bypass the crossbar state entirely
+ if (!is_express_snoop) {
+ // if this particular request will generate a snoop
+ // response
+ if (expect_snoop_resp) {
+ // we should never have an exsiting request outstanding
+ assert(outstandingSnoop.find(pkt->req) ==
+ outstandingSnoop.end());
+ outstandingSnoop.insert(pkt->req);
- // if it was added as outstanding and the send failed, then
- // erase it again
- if (add_outstanding)
- outstandingReq.erase(pkt->req);
+ // basic sanity check on the outstanding snoops
+ panic_if(outstandingSnoop.size() > 512,
+ "Outstanding snoop requests exceeded 512\n");
+ }
- // undo the calculation so we can check for 0 again
- pkt->firstWordDelay = pkt->lastWordDelay = 0;
+ // remember where to route the normal response to
+ if (expect_response || expect_snoop_resp) {
+ assert(routeTo.find(pkt->req) == routeTo.end());
+ routeTo[pkt->req] = slave_port_id;
- DPRINTF(CoherentXBar, "recvTimingReq: src %s %s 0x%x RETRY\n",
- src_port->name(), pkt->cmdString(), pkt->getAddr());
+ panic_if(routeTo.size() > 512,
+ "Routing table exceeds 512 packets\n");
+ }
// update the layer state and schedule an idle event
- reqLayers[master_port_id]->failedTiming(src_port,
- clockEdge(headerCycles));
- } else {
- // update the layer state and schedule an idle event
reqLayers[master_port_id]->succeededTiming(packetFinishTime);
}
- }
- // stats updates only consider packets that were successfully sent
- if (success) {
+ // stats updates only consider packets that were successfully sent
pktCount[slave_port_id][master_port_id]++;
pktSize[slave_port_id][master_port_id] += pkt_size;
transDist[pkt_cmd]++;
+
+ if (is_express_snoop)
+ snoops++;
}
return success;
@@ -270,8 +277,10 @@
// determine the source port based on the id
MasterPort *src_port = masterPorts[master_port_id];
- // determine the destination based on what is stored in the packet
- PortID slave_port_id = pkt->getDest();
+ // determine the destination
+ const auto route_lookup = routeTo.find(pkt->req);
+ assert(route_lookup != routeTo.end());
+ const PortID slave_port_id = route_lookup->second;
assert(slave_port_id != InvalidPortID);
assert(slave_port_id < respLayers.size());
@@ -294,18 +303,11 @@
calcPacketTiming(pkt);
Tick packetFinishTime = pkt->lastWordDelay + curTick();
- // the packet is a normal response to a request that we should
- // have seen passing through the crossbar
- assert(outstandingReq.find(pkt->req) != outstandingReq.end());
-
if (snoopFilter && !pkt->req->isUncacheable() && !system->bypassCaches()) {
// let the snoop filter inspect the response and update its state
snoopFilter->updateResponse(pkt, *slavePorts[slave_port_id]);
}
- // remove it as outstanding
- outstandingReq.erase(pkt->req);
-
// send the packet through the destination slave port
bool success M5_VAR_USED = slavePorts[slave_port_id]->sendTimingResp(pkt);
@@ -313,6 +315,9 @@
// deadlock
assert(success);
+ // remove the request from the routing table
+ routeTo.erase(route_lookup);
+
respLayers[slave_port_id]->succeededTiming(packetFinishTime);
// stats updates
@@ -337,8 +342,8 @@
// we should only see express snoops from caches
assert(pkt->isExpressSnoop());
- // set the source port for routing of the response
- pkt->setSrc(master_port_id);
+ // remeber if the packet is inhibited so we can see if it changes
+ const bool is_inhibited = pkt->memInhibitAsserted();
if (snoopFilter) {
// let the Snoop Filter work its magic and guide probing
@@ -355,6 +360,12 @@
forwardTiming(pkt, InvalidPortID);
}
+ // if we can expect a response, remember how to route it
+ if (!is_inhibited && pkt->memInhibitAsserted()) {
+ assert(routeTo.find(pkt->req) == routeTo.end());
+ routeTo[pkt->req] = master_port_id;
+ }
+
// a snoop request came from a connected slave device (one of
// our master ports), and if it is not coming from the slave
// device responsible for the address range something is
@@ -369,16 +380,18 @@
// determine the source port based on the id
SlavePort* src_port = slavePorts[slave_port_id];
- // get the destination from the packet
- PortID dest_port_id = pkt->getDest();
+ // get the destination
+ const auto route_lookup = routeTo.find(pkt->req);
+ assert(route_lookup != routeTo.end());
+ const PortID dest_port_id = route_lookup->second;
assert(dest_port_id != InvalidPortID);
// determine if the response is from a snoop request we
// created as the result of a normal request (in which case it
- // should be in the outstandingReq), or if we merely forwarded
+ // should be in the outstandingSnoop), or if we merely forwarded
// someone else's snoop request
- bool forwardAsSnoop = outstandingReq.find(pkt->req) ==
- outstandingReq.end();
+ const bool forwardAsSnoop = outstandingSnoop.find(pkt->req) ==
+ outstandingSnoop.end();
// test if the crossbar should be considered occupied for the
// current port, note that the check is bypassed if the response
@@ -440,13 +453,11 @@
// i.e. from a coherent master connected to the crossbar, and
// since we created the snoop request as part of recvTiming,
// this should now be a normal response again
- outstandingReq.erase(pkt->req);
+ outstandingSnoop.erase(pkt->req);
- // this is a snoop response from a coherent master, with a
- // destination field set on its way through the crossbar as
- // request, hence it should never go back to where the snoop
- // response came from, but instead to where the original
- // request came from
+ // this is a snoop response from a coherent master, hence it
+ // should never go back to where the snoop response came from,
+ // but instead to where the original request came from
assert(slave_port_id != dest_port_id);
if (snoopFilter) {
@@ -476,6 +487,9 @@
respLayers[dest_port_id]->succeededTiming(packetFinishTime);
}
+ // remove the request from the routing table
+ routeTo.erase(route_lookup);
+
// stats updates
transDist[pkt_cmd]++;
snoops++;
diff -r b7ff344c3061 -r bd376adfb7d4 src/mem/coherent_xbar.hh
--- a/src/mem/coherent_xbar.hh Thu Jan 22 05:00:57 2015 -0500
+++ b/src/mem/coherent_xbar.hh Thu Jan 22 05:01:14 2015 -0500
@@ -51,7 +51,6 @@
#ifndef __MEM_COHERENT_XBAR_HH__
#define __MEM_COHERENT_XBAR_HH__
-#include "base/hashmap.hh"
#include "mem/snoop_filter.hh"
#include "mem/xbar.hh"
#include "params/CoherentXBar.hh"
@@ -259,11 +258,11 @@
std::vector<SlavePort*> snoopPorts;
/**
- * Store the outstanding requests so we can determine which ones
- * we generated and which ones were merely forwarded. This is used
- * in the coherent crossbar when coherency responses come back.
+ * Store the outstanding requests that we are expecting snoop
+ * responses from so we can determine which snoop responses we
+ * generated and which ones were merely forwarded.
*/
- m5::hash_set<RequestPtr> outstandingReq;
+ m5::hash_set<RequestPtr> outstandingSnoop;
/**
* Keep a pointer to the system to be allow to querying memory system
diff -r b7ff344c3061 -r bd376adfb7d4 src/mem/noncoherent_xbar.cc
--- a/src/mem/noncoherent_xbar.cc Thu Jan 22 05:00:57 2015 -0500
+++ b/src/mem/noncoherent_xbar.cc Thu Jan 22 05:01:14 2015 -0500
@@ -127,12 +127,14 @@
unsigned int pkt_size = pkt->hasData() ? pkt->getSize() : 0;
unsigned int pkt_cmd = pkt->cmdToIndex();
- // set the source port for routing of the response
- pkt->setSrc(slave_port_id);
-
calcPacketTiming(pkt);
Tick packetFinishTime = pkt->lastWordDelay + curTick();
+ // before forwarding the packet (and possibly altering it),
+ // remember if we are expecting a response
+ const bool expect_response = pkt->needsResponse() &&
+ !pkt->memInhibitAsserted();
+
// since it is a normal request, attempt to send the packet
bool success = masterPorts[master_port_id]->sendTimingReq(pkt);
@@ -153,6 +155,12 @@
return false;
_______________________________________________
gem5-dev mailing list
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev
