changeset 85a001f2193b in /z/repo/gem5
details: http://repo.gem5.org/gem5?cmd=changeset;node=85a001f2193b
description:
        mem: Delay responses in the crossbar before forwarding

        This patch changes how the crossbar classes deal with
        responses. Instead of forwarding responses directly and burdening the
        neighbouring modules in paying for the latency (through the
        pkt->headerDelay), we now queue them before sending them.

        The coherency protocol is not affected as requests and any snoop
        requests/responses are still passed on in zero time. Thus, the
        responses end up paying for any header delay accumulated when passing
        through the crossbar. Any latency incurred on the request path will be
        paid for on the response side, if no other module has dealt with it.

        As a result of this patch, responses are returned at a later
        point. This affects the number of outstanding transactions, and quite
        a few regressions see an impact in blocking due to no MSHRs, increased
        cache-miss latencies, etc.

        Going forward we should be able to use the same concept also for snoop
        responses, and any request that is not an express snoop.

diffstat:

 src/mem/cache/cache_impl.hh |   5 ++++
 src/mem/coherent_xbar.cc    |  46 ++++++++++++++++++++++++--------------------
 src/mem/coherent_xbar.hh    |  28 +++++++++++++-------------
 src/mem/noncoherent_xbar.cc |  25 +++++++++++++++++------
 src/mem/noncoherent_xbar.hh |  14 +++++-------
 src/mem/snoop_filter.hh     |   5 ++-
 src/mem/xbar.hh             |   3 +-
 7 files changed, 73 insertions(+), 53 deletions(-)

diffs (truncated from 342 to 300 lines):

diff -r 279efb97ec99 -r 85a001f2193b src/mem/cache/cache_impl.hh
--- a/src/mem/cache/cache_impl.hh       Fri Jul 03 10:14:43 2015 -0400
+++ b/src/mem/cache/cache_impl.hh       Fri Jul 03 10:14:44 2015 -0400
@@ -1176,6 +1176,11 @@
 {
     assert(pkt->isResponse());
 
+    // all header delay should be paid for by the crossbar, unless
+    // this is a prefetch response from above
+    panic_if(pkt->headerDelay != 0 && pkt->cmd != MemCmd::HardPFResp,
+             "%s saw a non-zero packet delay\n", name());
+
     MSHR *mshr = dynamic_cast<MSHR*>(pkt->senderState);
     bool is_error = pkt->isError();
 
diff -r 279efb97ec99 -r 85a001f2193b src/mem/coherent_xbar.cc
--- a/src/mem/coherent_xbar.cc  Fri Jul 03 10:14:43 2015 -0400
+++ b/src/mem/coherent_xbar.cc  Fri Jul 03 10:14:44 2015 -0400
@@ -89,7 +89,7 @@
     // create the slave ports, once again starting at zero
     for (int i = 0; i < p->port_slave_connection_count; ++i) {
         std::string portName = csprintf("%s.slave[%d]", name(), i);
-        SlavePort* bp = new CoherentXBarSlavePort(portName, *this, i);
+        QueuedSlavePort* bp = new CoherentXBarSlavePort(portName, *this, i);
         slavePorts.push_back(bp);
         respLayers.push_back(new RespLayer(*bp, *this,
                                            csprintf(".respLayer%d", i)));
@@ -345,12 +345,11 @@
         snoopFilter->updateResponse(pkt, *slavePorts[slave_port_id]);
     }
 
-    // send the packet through the destination slave port
-    bool success M5_VAR_USED = slavePorts[slave_port_id]->sendTimingResp(pkt);
-
-    // currently it is illegal to block responses... can lead to
-    // deadlock
-    assert(success);
+    // send the packet through the destination slave port and pay for
+    // any outstanding header delay
+    Tick latency = pkt->headerDelay;
+    pkt->headerDelay = 0;
+    slavePorts[slave_port_id]->schedTimingResp(pkt, curTick() + latency);
 
     // remove the request from the routing table
     routeTo.erase(route_lookup);
@@ -517,18 +516,11 @@
                 pkt->getAddr());
 
         // as a normal response, it should go back to a master through
-        // one of our slave ports, at this point we are ignoring the
-        // fact that the response layer could be busy and do not touch
-        // its state
-        bool success M5_VAR_USED =
-            slavePorts[dest_port_id]->sendTimingResp(pkt);
-
-        // @todo Put the response in an internal FIFO and pass it on
-        // to the response layer from there
-
-        // currently it is illegal to block responses... can lead
-        // to deadlock
-        assert(success);
+        // one of our slave ports, we also pay for any outstanding
+        // header latency
+        Tick latency = pkt->headerDelay;
+        pkt->headerDelay = 0;
+        slavePorts[dest_port_id]->schedTimingResp(pkt, curTick() + latency);
 
         respLayers[dest_port_id]->succeededTiming(packetFinishTime);
     }
@@ -546,7 +538,7 @@
 
 void
 CoherentXBar::forwardTiming(PacketPtr pkt, PortID exclude_slave_port_id,
-                           const std::vector<SlavePort*>& dests)
+                           const std::vector<QueuedSlavePort*>& dests)
 {
     DPRINTF(CoherentXBar, "%s for %s address %x size %d\n", __func__,
             pkt->cmdString(), pkt->getAddr(), pkt->getSize());
@@ -700,7 +692,7 @@
 std::pair<MemCmd, Tick>
 CoherentXBar::forwardAtomic(PacketPtr pkt, PortID exclude_slave_port_id,
                            PortID source_master_port_id,
-                           const std::vector<SlavePort*>& dests)
+                           const std::vector<QueuedSlavePort*>& dests)
 {
     // the packet may be changed on snoops, record the original
     // command to enable us to restore it between snoops so that
@@ -787,6 +779,18 @@
     // there is no need to continue if the snooping has found what we
     // were looking for and the packet is already a response
     if (!pkt->isResponse()) {
+        // since our slave ports are queued ports we need to check them as well
+        for (const auto& p : slavePorts) {
+            // if we find a response that has the data, then the
+            // downstream caches/memories may be out of date, so simply stop
+            // here
+            if (p->checkFunctional(pkt)) {
+                if (pkt->needsResponse())
+                    pkt->makeResponse();
+                return;
+            }
+        }
+
         PortID dest_id = findPort(pkt->getAddr());
 
         masterPorts[dest_id]->sendFunctional(pkt);
diff -r 279efb97ec99 -r 85a001f2193b src/mem/coherent_xbar.hh
--- a/src/mem/coherent_xbar.hh  Fri Jul 03 10:14:43 2015 -0400
+++ b/src/mem/coherent_xbar.hh  Fri Jul 03 10:14:44 2015 -0400
@@ -84,7 +84,7 @@
      * be instantiated for each of the master ports connecting to the
      * crossbar.
      */
-    class CoherentXBarSlavePort : public SlavePort
+    class CoherentXBarSlavePort : public QueuedSlavePort
     {
 
       private:
@@ -92,11 +92,15 @@
         /** A reference to the crossbar to which this port belongs. */
         CoherentXBar &xbar;
 
+        /** A normal packet queue used to store responses. */
+        RespPacketQueue queue;
+
       public:
 
         CoherentXBarSlavePort(const std::string &_name,
                              CoherentXBar &_xbar, PortID _id)
-            : SlavePort(_name, &_xbar, _id), xbar(_xbar)
+            : QueuedSlavePort(_name, &_xbar, queue, _id), xbar(_xbar),
+              queue(_xbar, *this)
         { }
 
       protected:
@@ -126,12 +130,6 @@
         { xbar.recvFunctional(pkt, id); }
 
         /**
-         * When receiving a retry, pass it to the crossbar.
-         */
-        virtual void recvRespRetry()
-        { panic("Crossbar slave ports should never retry.\n"); }
-
-        /**
          * Return the union of all adress ranges seen by this crossbar.
          */
         virtual AddrRangeList getAddrRanges() const
@@ -215,14 +213,14 @@
       private:
 
         /** The port which we mirror internally. */
-        SlavePort& slavePort;
+        QueuedSlavePort& slavePort;
 
       public:
 
         /**
          * Create a snoop response port that mirrors a given slave port.
          */
-        SnoopRespPort(SlavePort& slave_port, CoherentXBar& _xbar) :
+        SnoopRespPort(QueuedSlavePort& slave_port, CoherentXBar& _xbar) :
             MasterPort(slave_port.name() + ".snoopRespPort", &_xbar),
             slavePort(slave_port) { }
 
@@ -253,7 +251,7 @@
 
     std::vector<SnoopRespPort*> snoopRespPorts;
 
-    std::vector<SlavePort*> snoopPorts;
+    std::vector<QueuedSlavePort*> snoopPorts;
 
     /**
      * Store the outstanding requests that we are expecting snoop
@@ -324,7 +322,7 @@
      * @param dests Vector of destination ports for the forwarded pkt
      */
     void forwardTiming(PacketPtr pkt, PortID exclude_slave_port_id,
-                       const std::vector<SlavePort*>& dests);
+                       const std::vector<QueuedSlavePort*>& dests);
 
     /** Function called by the port when the crossbar is recieving a Atomic
       transaction.*/
@@ -347,7 +345,8 @@
     std::pair<MemCmd, Tick> forwardAtomic(PacketPtr pkt,
                                           PortID exclude_slave_port_id)
     {
-        return forwardAtomic(pkt, exclude_slave_port_id, InvalidPortID, 
snoopPorts);
+        return forwardAtomic(pkt, exclude_slave_port_id, InvalidPortID,
+                             snoopPorts);
     }
 
     /**
@@ -365,7 +364,8 @@
     std::pair<MemCmd, Tick> forwardAtomic(PacketPtr pkt,
                                           PortID exclude_slave_port_id,
                                           PortID source_master_port_id,
-                                          const std::vector<SlavePort*>& 
dests);
+                                          const std::vector<QueuedSlavePort*>&
+                                          dests);
 
     /** Function called by the port when the crossbar is recieving a Functional
         transaction.*/
diff -r 279efb97ec99 -r 85a001f2193b src/mem/noncoherent_xbar.cc
--- a/src/mem/noncoherent_xbar.cc       Fri Jul 03 10:14:43 2015 -0400
+++ b/src/mem/noncoherent_xbar.cc       Fri Jul 03 10:14:44 2015 -0400
@@ -82,7 +82,7 @@
     // create the slave ports, once again starting at zero
     for (int i = 0; i < p->port_slave_connection_count; ++i) {
         std::string portName = csprintf("%s.slave[%d]", name(), i);
-        SlavePort* bp = new NoncoherentXBarSlavePort(portName, *this, i);
+        QueuedSlavePort* bp = new NoncoherentXBarSlavePort(portName, *this, i);
         slavePorts.push_back(bp);
         respLayers.push_back(new RespLayer(*bp, *this,
                                            csprintf(".respLayer%d", i)));
@@ -218,12 +218,11 @@
     // determine how long to be crossbar layer is busy
     Tick packetFinishTime = clockEdge(Cycles(1)) + pkt->payloadDelay;
 
-    // send the packet through the destination slave port
-    bool success M5_VAR_USED = slavePorts[slave_port_id]->sendTimingResp(pkt);
-
-    // currently it is illegal to block responses... can lead to
-    // deadlock
-    assert(success);
+    // send the packet through the destination slave port, and pay for
+    // any outstanding latency
+    Tick latency = pkt->headerDelay;
+    pkt->headerDelay = 0;
+    slavePorts[slave_port_id]->schedTimingResp(pkt, curTick() + latency);
 
     // remove the request from the routing table
     routeTo.erase(route_lookup);
@@ -295,6 +294,18 @@
                 pkt->cmdString());
     }
 
+    // since our slave ports are queued ports we need to check them as well
+    for (const auto& p : slavePorts) {
+        // if we find a response that has the data, then the
+        // downstream caches/memories may be out of date, so simply stop
+        // here
+        if (p->checkFunctional(pkt)) {
+            if (pkt->needsResponse())
+                pkt->makeResponse();
+            return;
+        }
+    }
+
     // determine the destination port
     PortID dest_id = findPort(pkt->getAddr());
 
diff -r 279efb97ec99 -r 85a001f2193b src/mem/noncoherent_xbar.hh
--- a/src/mem/noncoherent_xbar.hh       Fri Jul 03 10:14:43 2015 -0400
+++ b/src/mem/noncoherent_xbar.hh       Fri Jul 03 10:14:44 2015 -0400
@@ -84,18 +84,22 @@
      * will be instantiated for each of the master ports connecting to
      * the crossbar.
      */
-    class NoncoherentXBarSlavePort : public SlavePort
+    class NoncoherentXBarSlavePort : public QueuedSlavePort
     {
       private:
 
         /** A reference to the crossbar to which this port belongs. */
         NoncoherentXBar &xbar;
 
+        /** A normal packet queue used to store responses. */
+        RespPacketQueue queue;
+
       public:
 
         NoncoherentXBarSlavePort(const std::string &_name,
                                 NoncoherentXBar &_xbar, PortID _id)
-            : SlavePort(_name, &_xbar, _id), xbar(_xbar)
+            : QueuedSlavePort(_name, &_xbar, queue, _id), xbar(_xbar),
+              queue(_xbar, *this)
         { }
 
       protected:
@@ -119,12 +123,6 @@
         { xbar.recvFunctional(pkt, id); }
 
         /**
-         * When receiving a retry, pass it to the crossbar.
-         */
-        virtual void recvRespRetry()
-        { panic("Crossbar slave ports should never retry.\n"); }
-
-        /**
          * Return the union of all adress ranges seen by this crossbar.
          */
         virtual AddrRangeList getAddrRanges() const
diff -r 279efb97ec99 -r 85a001f2193b src/mem/snoop_filter.hh
--- a/src/mem/snoop_filter.hh   Fri Jul 03 10:14:43 2015 -0400
+++ b/src/mem/snoop_filter.hh   Fri Jul 03 10:14:44 2015 -0400
@@ -50,6 +50,7 @@
 #include "base/hashmap.hh"
 #include "mem/packet.hh"
 #include "mem/port.hh"
_______________________________________________
gem5-dev mailing list
[email protected]
http://m5sim.org/mailman/listinfo/gem5-dev

Reply via email to