cpu-o3: Avoid early checker verification for store conditionals

The O3CPU allows stores to commit before they are completed and as
soon as they enter the store queue. This is the reason why stores are
verified by the the checker CPU, separately, once they complete
and after they are sent to the memory.

Store conditionals, on the other hand, have an additional writeback
stage in the pipeline as they return their result to a register,
similarly to loads. This is the reason why they do not commit
before they receive a response from the memory. This allows store
conditionals to be verified by the checker CPU as soon as they
commit in the same way as all other non-store insturctions.

At the same time, the presense of a checker CPU should not require
changes to way we handle instructions. This change removes explicit
calls to:
* incorrectly set the extra data of the request to 0 (a subsequent
  call to completeAcc already does this without making any ISA
  assumptions about the return value of the failed store conditional)
* complete failing store conditionals

Change-Id: If21d70b21caa55b35e9fdcc50f254c590465d3c3
Reviewed-by: Andreas Sandberg <andreas.sandberg@arm.com>
Reviewed-on: https://gem5-review.googlesource.com/4820
Maintainer: Andreas Sandberg <andreas.sandberg@arm.com>

2 files changed, 16 insertions, 9 deletions

diff --git a/src/cpu/o3/commit_impl.hh b/src/cpu/o3/commit_impl.hh
index bf5ee8a38..b3a97ad3a 100644
--- a/src/cpu/o3/commit_impl.hh
+++ b/src/cpu/o3/commit_impl.hh
@@ -1,6 +1,6 @@
 /*
  * Copyright 2014 Google, Inc.
- * Copyright (c) 2010-2014 ARM Limited
+ * Copyright (c) 2010-2014, 2017 ARM Limited
  * All rights reserved
  *
  * The license below extends only to copyright in the software and shall
@@ -1044,6 +1044,12 @@ DefaultCommit<Impl>::commitInsts()
                                              (!(pc[0].instAddr() & 0x3)));
                 }
 
+                // at this point store conditionals should either have
+                // been completed or predicated false
+                assert(!head_inst->isStoreConditional() ||
+                       head_inst->isCompleted() ||
+                       !head_inst->readPredicate());
+
                 // Updates misc. registers.
                 head_inst->updateMiscRegs();
 
diff --git a/src/cpu/o3/lsq_unit_impl.hh b/src/cpu/o3/lsq_unit_impl.hh
index 56f12cbb4..65abaa438 100644
--- a/src/cpu/o3/lsq_unit_impl.hh
+++ b/src/cpu/o3/lsq_unit_impl.hh
@@ -1,6 +1,6 @@
 
 /*
- * Copyright (c) 2010-2014 ARM Limited
+ * Copyright (c) 2010-2014, 2017 ARM Limited
  * Copyright (c) 2013 Advanced Micro Devices, Inc.
  * All rights reserved
  *
@@ -116,6 +116,9 @@ LSQUnit<Impl>::completeDataAccess(PacketPtr pkt)
     assert(!cpu->switchedOut());
     if (!inst->isSquashed()) {
         if (!state->noWB) {
+            // Only loads and store conditionals perform the writeback
+            // after receving the response from the memory
+            assert(inst->isLoad() || inst->isStoreConditional());
             if (!TheISA::HasUnalignedMemAcc || !state->isSplit ||
                 !state->isLoad) {
                 writeback(inst, pkt);
@@ -898,12 +901,6 @@ LSQUnit<Impl>::writebackStores()
                         inst->seqNum);
                 WritebackEvent *wb = new WritebackEvent(inst, data_pkt, this);
                 cpu->schedule(wb, curTick() + 1);
-                if (cpu->checker) {
-                    // Make sure to set the LLSC data for verification
-                    // if checker is loaded
-                    inst->reqToVerify->setExtraData(0);
-                    inst->completeAcc(data_pkt);
-                }
                 completeStore(storeWBIdx);
                 incrStIdx(storeWBIdx);
                 continue;
@@ -1211,7 +1208,11 @@ LSQUnit<Impl>::completeStore(int store_idx)
     // Tell the checker we've completed this instruction.  Some stores
     // may get reported twice to the checker, but the checker can
     // handle that case.
-    if (cpu->checker) {
+
+    // Store conditionals cannot be sent to the checker yet, they have
+    // to update the misc registers first which should take place
+    // when they commit
+    if (cpu->checker && !storeQueue[store_idx].inst->isStoreConditional()) {
         cpu->checker->verify(storeQueue[store_idx].inst);
     }
 }