changeset b1d90d88420e in /z/repo/gem5
details: http://repo.gem5.org/gem5?cmd=changeset;node=b1d90d88420e
description:
        mem: Add byte mask to Packet::checkFunctional

        This patch changes the valid-bytes start/end to a proper byte
        mask. With the changes in timing introduced in previous patches there
        are more packets waiting in queues, and there are regressions using
        the checker CPU failing due to non-contigous read data being found in
        the various cache queues.

        This patch also adds some more comments explaining what is going on,
        and adds the fourth and missing case to Packet::checkFunctional.

diffstat:

 src/mem/packet.cc |  109 ++++++++++++++++++++---------------------------------
 src/mem/packet.hh |   13 +----
 2 files changed, 46 insertions(+), 76 deletions(-)

diffs (195 lines):

diff -r 886d2458e0d6 -r b1d90d88420e src/mem/packet.cc
--- a/src/mem/packet.cc Mon Mar 02 04:00:49 2015 -0500
+++ b/src/mem/packet.cc Mon Mar 02 04:00:52 2015 -0500
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2011-2014 ARM Limited
+ * Copyright (c) 2011-2015 ARM Limited
  * All rights reserved
  *
  * The license below extends only to copyright in the software and shall
@@ -207,6 +207,8 @@
     if (isRead()) {
         if (func_start >= val_start && func_end <= val_end) {
             memcpy(getPtr<uint8_t>(), _data + offset, getSize());
+            if (bytesValid.empty())
+                bytesValid.resize(getSize(), true);
             // complete overlap, and as the current packet is a read
             // we are done
             return true;
@@ -218,91 +220,64 @@
 
             // calculate offsets and copy sizes for the two byte arrays
             if (val_start < func_start && val_end <= func_end) {
+                // the one we are checking against starts before and
+                // ends before or the same
                 val_offset = func_start - val_start;
                 func_offset = 0;
                 overlap_size = val_end - func_start;
             } else if (val_start >= func_start && val_end > func_end) {
+                // the one we are checking against starts after or the
+                // same, and ends after
                 val_offset = 0;
                 func_offset = val_start - func_start;
                 overlap_size = func_end - val_start;
             } else if (val_start >= func_start && val_end <= func_end) {
+                // the one we are checking against is completely
+                // subsumed in the current packet, possibly starting
+                // and ending at the same address
                 val_offset = 0;
                 func_offset = val_start - func_start;
                 overlap_size = size;
+            } else if (val_start < func_start && val_end > func_end) {
+                // the current packet is completely subsumed in the
+                // one we are checking against
+                val_offset = func_start - val_start;
+                func_offset = 0;
+                overlap_size = func_end - func_start;
             } else {
-                panic("BUG: Missed a case for a partial functional request");
+                panic("Missed a case for checkFunctional with "
+                      " %s 0x%x size %d, against 0x%x size %d\n",
+                      cmdString(), getAddr(), getSize(), addr, size);
             }
 
-            // Figure out how much of the partial overlap should be copied
-            // into the packet and not overwrite previously found bytes.
-            if (bytesValidStart == 0 && bytesValidEnd == 0) {
-                // No bytes have been copied yet, just set indices
-                // to found range
-                bytesValidStart = func_offset;
-                bytesValidEnd = func_offset + overlap_size;
-            } else {
-                // Some bytes have already been copied. Use bytesValid
-                // indices and offset values to figure out how much data
-                // to copy and where to copy it to.
-
-                // Indice overlap conditions to check
-                int a = func_offset - bytesValidStart;
-                int b = (func_offset + overlap_size) - bytesValidEnd;
-                int c = func_offset - bytesValidEnd;
-                int d = (func_offset + overlap_size) - bytesValidStart;
-
-                if (a >= 0 && b <= 0) {
-                    // bytes already in pkt data array are superset of
-                    // found bytes, will not copy any bytes
-                    overlap_size = 0;
-                } else if (a < 0 && d >= 0 && b <= 0) {
-                    // found bytes will move bytesValidStart towards 0
-                    overlap_size = bytesValidStart - func_offset;
-                    bytesValidStart = func_offset;
-                } else if (b > 0 && c <= 0 && a >= 0) {
-                    // found bytes will move bytesValidEnd
-                    // towards end of pkt data array
-                    overlap_size =
-                        (func_offset + overlap_size) - bytesValidEnd;
-                    val_offset += bytesValidEnd - func_offset;
-                    func_offset = bytesValidEnd;
-                    bytesValidEnd += overlap_size;
-                } else if (a < 0 && b > 0) {
-                    // Found bytes are superset of copied range. Will move
-                    // bytesValidStart towards 0 and bytesValidEnd towards
-                    // end of pkt data array.  Need to break copy into two
-                    // pieces so as to not overwrite previously found data.
-
-                    // copy the first half
-                    uint8_t *dest = getPtr<uint8_t>() + func_offset;
-                    uint8_t *src = _data + val_offset;
-                    memcpy(dest, src, (bytesValidStart - func_offset));
-
-                    // re-calc the offsets and indices to do the copy
-                    // required for the second half
-                    val_offset += (bytesValidEnd - func_offset);
-                    bytesValidStart = func_offset;
-                    overlap_size =
-                        (func_offset + overlap_size) - bytesValidEnd;
-                    func_offset = bytesValidEnd;
-                    bytesValidEnd += overlap_size;
-                } else if ((c > 0 && b > 0)
-                           || (a < 0 && d < 0)) {
-                    // region to be copied is discontiguous! Not supported.
-                    panic("BUG: Discontiguous bytes found"
-                          "for functional copying!");
-                }
-            }
-            assert(bytesValidEnd <= getSize());
-
             // copy partial data into the packet's data array
             uint8_t *dest = getPtr<uint8_t>() + func_offset;
             uint8_t *src = _data + val_offset;
             memcpy(dest, src, overlap_size);
 
-            // check if we're done filling the functional access
-            bool done = (bytesValidStart == 0) && (bytesValidEnd == getSize());
-            return done;
+            // initialise the tracking of valid bytes if we have not
+            // used it already
+            if (bytesValid.empty())
+                bytesValid.resize(getSize(), false);
+
+            // track if we are done filling the functional access
+            bool all_bytes_valid = true;
+
+            int i = 0;
+
+            // check up to func_offset
+            for (; all_bytes_valid && i < func_offset; ++i)
+                all_bytes_valid &= bytesValid[i];
+
+            // update the valid bytes
+            for (i = func_offset; i < func_offset + overlap_size; ++i)
+                bytesValid[i] = true;
+
+            // check the bit after the update we just made
+            for (; all_bytes_valid && i < getSize(); ++i)
+                all_bytes_valid &= bytesValid[i];
+
+            return all_bytes_valid;
         }
     } else if (isWrite()) {
         if (offset >= 0) {
diff -r 886d2458e0d6 -r b1d90d88420e src/mem/packet.hh
--- a/src/mem/packet.hh Mon Mar 02 04:00:49 2015 -0500
+++ b/src/mem/packet.hh Mon Mar 02 04:00:52 2015 -0500
@@ -304,11 +304,9 @@
     MemCmd origCmd;
 
     /**
-     * These values specify the range of bytes found that satisfy a
-     * functional read.
+     * Track the bytes found that satisfy a functional read.
      */
-    uint16_t bytesValidStart;
-    uint16_t bytesValidEnd;
+    std::vector<bool> bytesValid;
 
   public:
 
@@ -573,8 +571,7 @@
      */
     Packet(const RequestPtr _req, MemCmd _cmd)
         :  cmd(_cmd), req(_req), data(nullptr), addr(0), _isSecure(false),
-           size(0), bytesValidStart(0), bytesValidEnd(0),
-           headerDelay(0), payloadDelay(0),
+           size(0), headerDelay(0), payloadDelay(0),
            senderState(NULL)
     {
         if (req->hasPaddr()) {
@@ -595,7 +592,6 @@
      */
     Packet(const RequestPtr _req, MemCmd _cmd, int _blkSize)
         :  cmd(_cmd), req(_req), data(nullptr), addr(0), _isSecure(false),
-           bytesValidStart(0), bytesValidEnd(0),
            headerDelay(0), payloadDelay(0),
            senderState(NULL)
     {
@@ -619,8 +615,7 @@
         :  cmd(pkt->cmd), req(pkt->req),
            data(nullptr),
            addr(pkt->addr), _isSecure(pkt->_isSecure), size(pkt->size),
-           bytesValidStart(pkt->bytesValidStart),
-           bytesValidEnd(pkt->bytesValidEnd),
+           bytesValid(pkt->bytesValid),
            headerDelay(pkt->headerDelay),
            payloadDelay(pkt->payloadDelay),
            senderState(pkt->senderState)
_______________________________________________
gem5-dev mailing list
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev

Reply via email to