changeset b32578b2af99 in /z/repo/gem5
details: http://repo.gem5.org/gem5?cmd=changeset;node=b32578b2af99
description:
        mem: Align all MSHR entries to block boundaries

        This patch aligns all MSHR queue entries to block boundaries to
        simplify checks for matches. Previously there were corner cases that
        could lead to existing entries not being identified as matches.

        There are, rather alarmingly, a few regressions that change with this
        patch.

diffstat:

 src/mem/cache/base.hh       |   9 +++++++--
 src/mem/cache/cache_impl.hh |  21 ++++++++++++++++-----
 src/mem/cache/mshr.cc       |  22 +++++++++++-----------
 src/mem/cache/mshr.hh       |  30 +++++++++++++++---------------
 src/mem/cache/mshr_queue.cc |  31 +++++++++++++------------------
 src/mem/cache/mshr_queue.hh |  34 +++++++++++++++++++---------------
 6 files changed, 81 insertions(+), 66 deletions(-)

diffs (truncated from 425 to 300 lines):

diff -r d524dc4f16ae -r b32578b2af99 src/mem/cache/base.hh
--- a/src/mem/cache/base.hh     Fri Mar 27 04:55:54 2015 -0400
+++ b/src/mem/cache/base.hh     Fri Mar 27 04:55:55 2015 -0400
@@ -217,6 +217,11 @@
     MSHR *allocateBufferInternal(MSHRQueue *mq, Addr addr, int size,
                                  PacketPtr pkt, Tick time, bool requestBus)
     {
+        // check that the address is block aligned since we rely on
+        // this in a number of places when checking for matches and
+        // overlap
+        assert(addr == blockAlign(addr));
+
         MSHR *mshr = mq->allocate(addr, size, pkt, time, order++);
 
         if (mq->isFull()) {
@@ -506,7 +511,7 @@
     {
         assert(pkt->isWrite() && !pkt->isRead());
         return allocateBufferInternal(&writeBuffer,
-                                      pkt->getAddr(), pkt->getSize(),
+                                      blockAlign(pkt->getAddr()), blkSize,
                                       pkt, time, requestBus);
     }
 
@@ -515,7 +520,7 @@
         assert(pkt->req->isUncacheable());
         assert(pkt->isRead());
         return allocateBufferInternal(&mshrQueue,
-                                      pkt->getAddr(), pkt->getSize(),
+                                      blockAlign(pkt->getAddr()), blkSize,
                                       pkt, time, requestBus);
     }
 
diff -r d524dc4f16ae -r b32578b2af99 src/mem/cache/cache_impl.hh
--- a/src/mem/cache/cache_impl.hh       Fri Mar 27 04:55:54 2015 -0400
+++ b/src/mem/cache/cache_impl.hh       Fri Mar 27 04:55:55 2015 -0400
@@ -836,6 +836,9 @@
     }
     PacketPtr pkt = new Packet(cpu_pkt->req, cmd, blkSize);
 
+    // the packet should be block aligned
+    assert(pkt->getAddr() == blockAlign(pkt->getAddr()));
+
     pkt->allocate();
     DPRINTF(Cache, "%s created %s addr %#llx size %d\n",
             __func__, pkt->cmdString(), pkt->getAddr(), pkt->getSize());
@@ -1209,6 +1212,10 @@
                 completion_time += clockEdge(responseLatency) +
                     pkt->payloadDelay;
                 if (pkt->isRead() && !is_error) {
+                    // sanity check
+                    assert(pkt->getAddr() == tgt_pkt->getAddr());
+                    assert(pkt->getSize() >= tgt_pkt->getSize());
+
                     tgt_pkt->setData(pkt->getConstPtr<uint8_t>());
                 }
             }
@@ -1543,7 +1550,10 @@
     // if we got new data, copy it in (checking for a read response
     // and a response that has data is the same in the end)
     if (pkt->isRead()) {
+        // sanity checks
         assert(pkt->hasData());
+        assert(pkt->getSize() == blkSize);
+
         std::memcpy(blk->data, pkt->getConstPtr<uint8_t>(), blkSize);
     }
     // We pay for fillLatency here.
@@ -1899,7 +1909,7 @@
          !miss_mshr)) {
         // need to search MSHR queue for conflicting earlier miss.
         MSHR *conflict_mshr =
-            mshrQueue.findPending(write_mshr->addr, write_mshr->size,
+            mshrQueue.findPending(write_mshr->blkAddr,
                                   write_mshr->isSecure);
 
         if (conflict_mshr && conflict_mshr->order < write_mshr->order) {
@@ -1914,7 +1924,7 @@
     } else if (miss_mshr) {
         // need to check for conflicting earlier writeback
         MSHR *conflict_mshr =
-            writeBuffer.findPending(miss_mshr->addr, miss_mshr->size,
+            writeBuffer.findPending(miss_mshr->blkAddr,
                                     miss_mshr->isSecure);
         if (conflict_mshr) {
             // not sure why we don't check order here... it was in the
@@ -1985,10 +1995,10 @@
 
     if (mshr->isForwardNoResponse()) {
         // no response expected, just forward packet as it is
-        assert(tags->findBlock(mshr->addr, mshr->isSecure) == NULL);
+        assert(tags->findBlock(mshr->blkAddr, mshr->isSecure) == NULL);
         pkt = tgt_pkt;
     } else {
-        BlkType *blk = tags->findBlock(mshr->addr, mshr->isSecure);
+        BlkType *blk = tags->findBlock(mshr->blkAddr, mshr->isSecure);
 
         if (tgt_pkt->cmd == MemCmd::HardPFReq) {
             // We need to check the caches above us to verify that
@@ -2025,7 +2035,8 @@
 
             if (snoop_pkt.isBlockCached() || blk != NULL) {
                 DPRINTF(Cache, "Block present, prefetch squashed by cache.  "
-                               "Deallocating mshr target %#x.\n", mshr->addr);
+                               "Deallocating mshr target %#x.\n",
+                        mshr->blkAddr);
 
                 // Deallocate the mshr target
                 if (mshr->queue->forceDeallocateTarget(mshr)) {
diff -r d524dc4f16ae -r b32578b2af99 src/mem/cache/mshr.cc
--- a/src/mem/cache/mshr.cc     Fri Mar 27 04:55:54 2015 -0400
+++ b/src/mem/cache/mshr.cc     Fri Mar 27 04:55:55 2015 -0400
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2012-2013 ARM Limited
+ * Copyright (c) 2012-2013, 2015 ARM Limited
  * All rights reserved.
  *
  * The license below extends only to copyright in the software and shall
@@ -64,8 +64,8 @@
 MSHR::MSHR() : readyTime(0), _isUncacheable(false), downstreamPending(false),
                pendingDirty(false),
                postInvalidate(false), postDowngrade(false),
-               queue(NULL), order(0), addr(0),
-               size(0), isSecure(false), inService(false),
+               queue(NULL), order(0), blkAddr(0),
+               blkSize(0), isSecure(false), inService(false),
                isForward(false), threadNum(InvalidThreadID), data(NULL)
 {
 }
@@ -202,13 +202,13 @@
 
 
 void
-MSHR::allocate(Addr _addr, int _size, PacketPtr target, Tick whenReady,
-               Counter _order)
+MSHR::allocate(Addr blk_addr, unsigned blk_size, PacketPtr target,
+               Tick when_ready, Counter _order)
 {
-    addr = _addr;
-    size = _size;
+    blkAddr = blk_addr;
+    blkSize = blk_size;
     isSecure = target->isSecure();
-    readyTime = whenReady;
+    readyTime = when_ready;
     order = _order;
     assert(target);
     isForward = false;
@@ -221,7 +221,7 @@
     // 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);
+    targets.add(target, when_ready, _order, source, true);
     assert(deferredTargets.isReset());
     data = NULL;
 }
@@ -446,7 +446,7 @@
     // For other requests, we iterate over the individual targets
     // since that's where the actual data lies.
     if (pkt->isPrint()) {
-        pkt->checkFunctional(this, addr, isSecure, size, NULL);
+        pkt->checkFunctional(this, blkAddr, isSecure, blkSize, NULL);
         return false;
     } else {
         return (targets.checkFunctional(pkt) ||
@@ -459,7 +459,7 @@
 MSHR::print(std::ostream &os, int verbosity, const std::string &prefix) const
 {
     ccprintf(os, "%s[%#llx:%#llx](%s) %s %s %s state: %s %s %s %s %s\n",
-             prefix, addr, addr+size-1,
+             prefix, blkAddr, blkAddr + blkSize - 1,
              isSecure ? "s" : "ns",
              isForward ? "Forward" : "",
              isForwardNoResponse() ? "ForwNoResp" : "",
diff -r d524dc4f16ae -r b32578b2af99 src/mem/cache/mshr.hh
--- a/src/mem/cache/mshr.hh     Fri Mar 27 04:55:54 2015 -0400
+++ b/src/mem/cache/mshr.hh     Fri Mar 27 04:55:55 2015 -0400
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2012-2013 ARM Limited
+ * Copyright (c) 2012-2013, 2015 ARM Limited
  * All rights reserved.
  *
  * The license below extends only to copyright in the software and shall
@@ -45,8 +45,8 @@
  * Miss Status and Handling Register (MSHR) declaration.
  */
 
-#ifndef __MSHR_HH__
-#define __MSHR_HH__
+#ifndef __MEM_CACHE_MSHR_HH__
+#define __MEM_CACHE_MSHR_HH__
 
 #include <list>
 
@@ -149,11 +149,11 @@
     /** Order number assigned by the miss queue. */
     Counter order;
 
-    /** Address of the request. */
-    Addr addr;
+    /** Block aligned address of the MSHR. */
+    Addr blkAddr;
 
-    /** Size of the request. */
-    int size;
+    /** Block size of the cache. */
+    unsigned blkSize;
 
     /** True if the request targets the secure memory space. */
     bool isSecure;
@@ -216,14 +216,14 @@
 
     /**
      * Allocate a miss to this MSHR.
-     * @param cmd The requesting command.
-     * @param addr The address of the miss.
-     * @param asid The address space id of the miss.
-     * @param size The number of bytes to request.
-     * @param pkt  The original miss.
+     * @param blk_addr The address of the block.
+     * @param blk_size The number of bytes to request.
+     * @param pkt The original miss.
+     * @param when_ready When should the MSHR be ready to act upon.
+     * @param _order The logical order of this MSHR
      */
-    void allocate(Addr addr, int size, PacketPtr pkt,
-                  Tick when, Counter _order);
+    void allocate(Addr blk_addr, unsigned blk_size, PacketPtr pkt,
+                  Tick when_ready, Counter _order);
 
     bool markInService(bool pending_dirty_resp);
 
@@ -304,4 +304,4 @@
     std::string print() const;
 };
 
-#endif //__MSHR_HH__
+#endif // __MEM_CACHE_MSHR_HH__
diff -r d524dc4f16ae -r b32578b2af99 src/mem/cache/mshr_queue.cc
--- a/src/mem/cache/mshr_queue.cc       Fri Mar 27 04:55:54 2015 -0400
+++ b/src/mem/cache/mshr_queue.cc       Fri Mar 27 04:55:55 2015 -0400
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2012-2013 ARM Limited
+ * Copyright (c) 2012-2013, 2015 ARM Limited
  * All rights reserved.
  *
  * The license below extends only to copyright in the software and shall
@@ -66,13 +66,13 @@
 }
 
 MSHR *
-MSHRQueue::findMatch(Addr addr, bool is_secure) const
+MSHRQueue::findMatch(Addr blk_addr, bool is_secure) const
 {
     MSHR::ConstIterator i = allocatedList.begin();
     MSHR::ConstIterator end = allocatedList.end();
     for (; i != end; ++i) {
         MSHR *mshr = *i;
-        if (mshr->addr == addr && mshr->isSecure == is_secure) {
+        if (mshr->blkAddr == blk_addr && mshr->isSecure == is_secure) {
             return mshr;
         }
     }
@@ -80,7 +80,8 @@
 }
 
 bool
-MSHRQueue::findMatches(Addr addr, bool is_secure, vector<MSHR*>& matches) const
+MSHRQueue::findMatches(Addr blk_addr, bool is_secure,
+                       vector<MSHR*>& matches) const
 {
     // Need an empty vector
     assert(matches.empty());
@@ -89,7 +90,7 @@
     MSHR::ConstIterator end = allocatedList.end();
     for (; i != end; ++i) {
         MSHR *mshr = *i;
-        if (mshr->addr == addr && mshr->isSecure == is_secure) {
+        if (mshr->blkAddr == blk_addr && mshr->isSecure == is_secure) {
             retval = true;
             matches.push_back(mshr);
         }
@@ -106,7 +107,7 @@
     MSHR::ConstIterator end = allocatedList.end();
     for (; i != end; ++i) {
         MSHR *mshr = *i;
-        if (mshr->addr == blk_addr && mshr->checkFunctional(pkt)) {
+        if (mshr->blkAddr == blk_addr && mshr->checkFunctional(pkt)) {
             pkt->popLabel();
             return true;
         }
@@ -117,20 +118,14 @@
 
 
 MSHR *
-MSHRQueue::findPending(Addr addr, int size, bool is_secure) const
_______________________________________________
gem5-dev mailing list
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev

Reply via email to