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.

2 files changed, 46 insertions, 76 deletions

diff --git a/src/mem/packet.cc b/src/mem/packet.cc
index 09b612ad0..ecc2feb26 100644
--- a/src/mem/packet.cc
+++ b/src/mem/packet.cc
@@ -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 @@ Packet::checkFunctional(Printable *obj, Addr addr, bool is_secure, int size,
     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 @@ Packet::checkFunctional(Printable *obj, Addr addr, bool is_secure, int size,
 
             // 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");
-            }
-
-            // 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!");
-                }
+                panic("Missed a case for checkFunctional with "
+                      " %s 0x%x size %d, against 0x%x size %d\n",
+                      cmdString(), getAddr(), getSize(), addr, size);
             }
-            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 --git a/src/mem/packet.hh b/src/mem/packet.hh
index e80307ffc..cb8a5cbf4 100644
--- a/src/mem/packet.hh
+++ b/src/mem/packet.hh
@@ -304,11 +304,9 @@ class Packet : public Printable
     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 @@ class Packet : public Printable
      */
     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 @@ class Packet : public Printable
      */
     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 @@ class Packet : public Printable
         :  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)