Hi Mitch,

Quick question regarding this patch.  Does this patch also handle replaying 
stores once the cache becomes unblocked?  The changes and comments appear to 
only handle loads, but it seems like stores could have the same problem.

Thanks,

Brad



-----Original Message-----
From: gem5-dev [mailto:[email protected]] On Behalf Of Mitch Hayenga 
via gem5-dev
Sent: Wednesday, September 03, 2014 4:38 AM
To: [email protected]
Subject: [gem5-dev] changeset in gem5: cpu: Fix cache blocked load behavior in 
o3 cpu

changeset 6be8945d226b in /z/repo/gem5
details: http://repo.gem5.org/gem5?cmd=changeset;node=6be8945d226b
description:
        cpu: Fix cache blocked load behavior in o3 cpu

        This patch fixes the load blocked/replay mechanism in the o3 cpu.  
Rather than
        flushing the entire pipeline, this patch replays loads once the cache 
becomes
        unblocked.

        Additionally, deferred memory instructions (loads which had conflicting 
stores),
        when replayed would not respect the number of functional units (only 
respected
        issue width).  This patch also corrects that.

        Improvements over 20% have been observed on a microbenchmark designed to
        exercise this behavior.

diffstat:

 src/cpu/o3/iew.hh               |   13 +-
 src/cpu/o3/iew_impl.hh          |   57 ++--------
 src/cpu/o3/inst_queue.hh        |   25 ++++-
 src/cpu/o3/inst_queue_impl.hh   |   68 ++++++++++---
 src/cpu/o3/lsq.hh               |   27 +-----
 src/cpu/o3/lsq_impl.hh          |   23 +---
 src/cpu/o3/lsq_unit.hh          |  198 ++++++++++++++++-----------------------
 src/cpu/o3/lsq_unit_impl.hh     |   40 ++-----
 src/cpu/o3/mem_dep_unit.hh      |    4 +-
 src/cpu/o3/mem_dep_unit_impl.hh |    4 +-
 10 files changed, 203 insertions(+), 256 deletions(-)

diffs (truncated from 846 to 300 lines):

diff -r 1ba825974ee6 -r 6be8945d226b src/cpu/o3/iew.hh
--- a/src/cpu/o3/iew.hh Wed Sep 03 07:42:38 2014 -0400
+++ b/src/cpu/o3/iew.hh Wed Sep 03 07:42:39 2014 -0400
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2010-2012 ARM Limited
+ * Copyright (c) 2010-2012, 2014 ARM Limited
  * All rights reserved
  *
  * The license below extends only to copyright in the software and shall @@ 
-181,6 +181,12 @@
     /** Re-executes all rescheduled memory instructions. */
     void replayMemInst(DynInstPtr &inst);
 
+    /** Moves memory instruction onto the list of cache blocked instructions */
+    void blockMemInst(DynInstPtr &inst);
+
+    /** Notifies that the cache has become unblocked */
+    void cacheUnblocked();
+
     /** Sends an instruction to commit through the time buffer. */
     void instToCommit(DynInstPtr &inst);
 
@@ -233,11 +239,6 @@
      */
     void squashDueToMemOrder(DynInstPtr &inst, ThreadID tid);
 
-    /** Sends commit proper information for a squash due to memory becoming
-     * blocked (younger issued instructions must be retried).
-     */
-    void squashDueToMemBlocked(DynInstPtr &inst, ThreadID tid);
-
     /** Sets Dispatch to blocked, and signals back to other stages to block. */
     void block(ThreadID tid);
 
diff -r 1ba825974ee6 -r 6be8945d226b src/cpu/o3/iew_impl.hh
--- a/src/cpu/o3/iew_impl.hh    Wed Sep 03 07:42:38 2014 -0400
+++ b/src/cpu/o3/iew_impl.hh    Wed Sep 03 07:42:39 2014 -0400
@@ -530,29 +530,6 @@
 
 template<class Impl>
 void
-DefaultIEW<Impl>::squashDueToMemBlocked(DynInstPtr &inst, ThreadID tid) -{
-    DPRINTF(IEW, "[tid:%i]: Memory blocked, squashing load and younger insts, "
-            "PC: %s [sn:%i].\n", tid, inst->pcState(), inst->seqNum);
-    if (!toCommit->squash[tid] ||
-            inst->seqNum < toCommit->squashedSeqNum[tid]) {
-        toCommit->squash[tid] = true;
-
-        toCommit->squashedSeqNum[tid] = inst->seqNum;
-        toCommit->pc[tid] = inst->pcState();
-        toCommit->mispredictInst[tid] = NULL;
-
-        // Must include the broadcasted SN in the squash.
-        toCommit->includeSquashInst[tid] = true;
-
-        ldstQueue.setLoadBlockedHandled(tid);
-
-        wroteToTimeBuffer = true;
-    }
-}
-
-template<class Impl>
-void
 DefaultIEW<Impl>::block(ThreadID tid)
 {
     DPRINTF(IEW, "[tid:%u]: Blocking.\n", tid); @@ -610,6 +587,20 @@
 
 template<class Impl>
 void
+DefaultIEW<Impl>::blockMemInst(DynInstPtr& inst) {
+    instQueue.blockMemInst(inst);
+}
+
+template<class Impl>
+void
+DefaultIEW<Impl>::cacheUnblocked()
+{
+    instQueue.cacheUnblocked();
+}
+
+template<class Impl>
+void
 DefaultIEW<Impl>::instToCommit(DynInstPtr &inst)  {
     // This function should not be called after writebackInsts in a @@ 
-1376,15 +1367,6 @@
                 squashDueToMemOrder(violator, tid);
 
                 ++memOrderViolationEvents;
-            } else if (ldstQueue.loadBlocked(tid) &&
-                       !ldstQueue.isLoadBlockedHandled(tid)) {
-                fetchRedirect[tid] = true;
-
-                DPRINTF(IEW, "Load operation couldn't execute because the "
-                        "memory system is blocked.  PC: %s [sn:%lli]\n",
-                        inst->pcState(), inst->seqNum);
-
-                squashDueToMemBlocked(inst, tid);
             }
         } else {
             // Reset any state associated with redirects that will not @@ 
-1403,17 +1385,6 @@
 
                 ++memOrderViolationEvents;
             }
-            if (ldstQueue.loadBlocked(tid) &&
-                !ldstQueue.isLoadBlockedHandled(tid)) {
-                DPRINTF(IEW, "Load operation couldn't execute because the "
-                        "memory system is blocked.  PC: %s [sn:%lli]\n",
-                        inst->pcState(), inst->seqNum);
-                DPRINTF(IEW, "Blocked load will not be handled because "
-                        "already squashing\n");
-
-                ldstQueue.setLoadBlockedHandled(tid);
-            }
-
         }
     }
 
diff -r 1ba825974ee6 -r 6be8945d226b src/cpu/o3/inst_queue.hh
--- a/src/cpu/o3/inst_queue.hh  Wed Sep 03 07:42:38 2014 -0400
+++ b/src/cpu/o3/inst_queue.hh  Wed Sep 03 07:42:39 2014 -0400
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2011-2012 ARM Limited
+ * Copyright (c) 2011-2012, 2014 ARM Limited
  * Copyright (c) 2013 Advanced Micro Devices, Inc.
  * All rights reserved.
  *
@@ -188,11 +188,16 @@
      */
     DynInstPtr getInstToExecute();
 
-    /** Returns a memory instruction that was referred due to a delayed DTB
-     *  translation if it is now ready to execute.
+    /** Gets a memory instruction that was referred due to a delayed DTB
+     *  translation if it is now ready to execute.  NULL if none available.
      */
     DynInstPtr getDeferredMemInstToExecute();
 
+    /** Gets a memory instruction that was blocked on the cache. NULL if none
+     *  available.
+     */
+    DynInstPtr getBlockedMemInstToExecute();
+
     /**
      * Records the instruction as the producer of a register without
      * adding it to the rest of the IQ.
@@ -242,6 +247,12 @@
      */
     void deferMemInst(DynInstPtr &deferred_inst);
 
+    /**  Defers a memory instruction when it is cache blocked. */
+    void blockMemInst(DynInstPtr &blocked_inst);
+
+    /**  Notify instruction queue that a previous blockage has resolved */
+    void cacheUnblocked();
+
     /** Indicates an ordering violation between a store and a load. */
     void violation(DynInstPtr &store, DynInstPtr &faulting_load);
 
@@ -308,6 +319,14 @@
      */
     std::list<DynInstPtr> deferredMemInsts;
 
+    /** List of instructions that have been cache blocked. */
+    std::list<DynInstPtr> blockedMemInsts;
+
+    /** List of instructions that were cache blocked, but a retry has been seen
+     * since, so they can now be retried. May fail again go on the blocked 
list.
+     */
+    std::list<DynInstPtr> retryMemInsts;
+
     /**
      * Struct for comparing entries to be added to the priority queue.
      * This gives reverse ordering to the instructions in terms of diff -r 
1ba825974ee6 -r 6be8945d226b src/cpu/o3/inst_queue_impl.hh
--- a/src/cpu/o3/inst_queue_impl.hh     Wed Sep 03 07:42:38 2014 -0400
+++ b/src/cpu/o3/inst_queue_impl.hh     Wed Sep 03 07:42:39 2014 -0400
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2011-2013 ARM Limited
+ * Copyright (c) 2011-2014 ARM Limited
  * Copyright (c) 2013 Advanced Micro Devices, Inc.
  * All rights reserved.
  *
@@ -413,6 +413,8 @@
     nonSpecInsts.clear();
     listOrder.clear();
     deferredMemInsts.clear();
+    blockedMemInsts.clear();
+    retryMemInsts.clear();
 }
 
 template <class Impl>
@@ -734,13 +736,14 @@
 
     IssueStruct *i2e_info = issueToExecuteQueue->access(0);
 
-    DynInstPtr deferred_mem_inst;
-    int total_deferred_mem_issued = 0;
-    while (total_deferred_mem_issued < totalWidth &&
-           (deferred_mem_inst = getDeferredMemInstToExecute()) != 0) {
-        issueToExecuteQueue->access(0)->size++;
-        instsToExecute.push_back(deferred_mem_inst);
-        total_deferred_mem_issued++;
+    DynInstPtr mem_inst;
+    while (mem_inst = getDeferredMemInstToExecute()) {
+        addReadyMemInst(mem_inst);
+    }
+
+    // See if any cache blocked instructions are able to be executed
+    while (mem_inst = getBlockedMemInstToExecute()) {
+        addReadyMemInst(mem_inst);
     }
 
     // Have iterator to head of the list @@ -751,12 +754,11 @@
     // Increment the iterator.
     // This will avoid trying to schedule a certain op class if there are no
     // FUs that handle it.
+    int total_issued = 0;
     ListOrderIt order_it = listOrder.begin();
     ListOrderIt order_end_it = listOrder.end();
-    int total_issued = 0;
 
-    while (total_issued < (totalWidth - total_deferred_mem_issued) &&
-           order_it != order_end_it) {
+    while (total_issued < totalWidth && order_it != order_end_it) {
         OpClass op_class = (*order_it).queueType;
 
         assert(!readyInsts[op_class].empty());
@@ -874,7 +876,7 @@
     // @todo If the way deferred memory instructions are handeled due to
     // translation changes then the deferredMemInsts condition should be 
removed
     // from the code below.
-    if (total_issued || total_deferred_mem_issued || deferredMemInsts.size()) {
+    if (total_issued || !retryMemInsts.empty() || 
+ !deferredMemInsts.empty()) {
         cpu->activityThisCycle();
     } else {
         DPRINTF(IQ, "Not able to schedule any instructions.\n"); @@ -1050,7 
+1052,7 @@  void  InstructionQueue<Impl>::replayMemInst(DynInstPtr 
&replay_inst)  {
-    memDepUnit[replay_inst->threadNumber].replay(replay_inst);
+    memDepUnit[replay_inst->threadNumber].replay();
 }
 
 template <class Impl>
@@ -1078,18 +1080,52 @@
 }
 
 template <class Impl>
+void
+InstructionQueue<Impl>::blockMemInst(DynInstPtr &blocked_inst) {
+    blocked_inst->translationStarted(false);
+    blocked_inst->translationCompleted(false);
+
+    blocked_inst->clearIssued();
+    blocked_inst->clearCanIssue();
+    blockedMemInsts.push_back(blocked_inst);
+}
+
+template <class Impl>
+void
+InstructionQueue<Impl>::cacheUnblocked()
+{
+    retryMemInsts.splice(retryMemInsts.end(), blockedMemInsts);
+    // Get the CPU ticking again
+    cpu->wakeCPU();
+}
+
+template <class Impl>
 typename Impl::DynInstPtr
 InstructionQueue<Impl>::getDeferredMemInstToExecute()
 {
     for (ListIt it = deferredMemInsts.begin(); it != deferredMemInsts.end();
          ++it) {
         if ((*it)->translationCompleted() || (*it)->isSquashed()) {
-            DynInstPtr ret = *it;
+            DynInstPtr mem_inst = *it;
             deferredMemInsts.erase(it);
-            return ret;
+            return mem_inst;
         }
     }
-    return NULL;
+    return nullptr;
+}
+
+template <class Impl>
+typename Impl::DynInstPtr
+InstructionQueue<Impl>::getBlockedMemInstToExecute()
+{
_______________________________________________
gem5-dev mailing list
[email protected]
http://m5sim.org/mailman/listinfo/gem5-dev
_______________________________________________
gem5-dev mailing list
[email protected]
http://m5sim.org/mailman/listinfo/gem5-dev

Reply via email to