checker: CheckerCPU handling of MiscRegs was incorrect
authorGeoffrey Blake <Geoffrey.Blake@arm.com>
Fri, 24 Jan 2014 21:29:30 +0000 (15:29 -0600)
committerGeoffrey Blake <Geoffrey.Blake@arm.com>
Fri, 24 Jan 2014 21:29:30 +0000 (15:29 -0600)
The CheckerCPU model in pre-v8 code was not checking the
updates to miscellaneous registers due to some methods
for setting misc regs were not instrumented.  The v8 patches
exposed this by calling the instrumented misc reg update
methods and then invoking the checker before the main CPU had
updated its misc regs, leading to false positives about
register mismatches. This patch fixes the non-instrumented
misc reg update methods and places calls to the checker in
the proper places in the O3 model.

src/cpu/checker/cpu.cc
src/cpu/checker/cpu.hh
src/cpu/checker/cpu_impl.hh
src/cpu/o3/commit_impl.hh

index 5cb1ccf18b96bbe892f731c6e94c801b8a4de1a2..de9945af1f56561f08ade830917a5bebfd507391 100644 (file)
@@ -78,7 +78,7 @@ CheckerCPU::CheckerCPU(Params *p)
     startNumLoad = 0;
     youngestSN = 0;
 
-    changedPC = willChangePC = changedNextPC = false;
+    changedPC = willChangePC = false;
 
     exitOnError = p->exitOnError;
     warnOnlyOnLoadError = p->warnOnlyOnLoadError;
index c49f264f9ffb5099063e96d46ab768f3aa0a8904..9f4c4d58c995c9edf12c69515d8c4d66e6d47cc6 100644 (file)
@@ -296,12 +296,14 @@ class CheckerCPU : public BaseCPU
 
     void setMiscRegNoEffect(int misc_reg, const MiscReg &val)
     {
+        DPRINTF(Checker, "Setting misc reg %d with no effect to check later\n", misc_reg);
         miscRegIdxs.push(misc_reg);
         return thread->setMiscRegNoEffect(misc_reg, val);
     }
 
     void setMiscReg(int misc_reg, const MiscReg &val)
     {
+        DPRINTF(Checker, "Setting misc reg %d with effect to check later\n", misc_reg);
         miscRegIdxs.push(misc_reg);
         return thread->setMiscReg(misc_reg, val);
     }
@@ -316,7 +318,7 @@ class CheckerCPU : public BaseCPU
             const StaticInst *si, int idx, const MiscReg &val)
     {
         int reg_idx = si->destRegIdx(idx) - TheISA::Misc_Reg_Base;
-        return thread->setMiscReg(reg_idx, val);
+        return this->setMiscReg(reg_idx, val);
     }
 
 #if THE_ISA == MIPS_ISA
@@ -392,7 +394,6 @@ class CheckerCPU : public BaseCPU
     bool changedPC;
     bool willChangePC;
     TheISA::PCState newPCState;
-    bool changedNextPC;
     bool exitOnError;
     bool updateOnError;
     bool warnOnlyOnLoadError;
index 23e9c103e2885c99b89c511bf3028ff1de4a1b01..b6ec4f77bd0b3e8c9edb52c06fff74af8d2404cc 100644 (file)
@@ -230,11 +230,6 @@ Checker<Impl>::verify(DynInstPtr &completed_inst)
             }
             changedPC = false;
         }
-        if (changedNextPC) {
-            DPRINTF(Checker, "Changed NextPC recently to %#x\n",
-                    thread->nextInstAddr());
-            changedNextPC = false;
-        }
 
         // Try to fetch the instruction
         uint64_t fetchOffset = 0;
index a6f2a63db35cbe9eb670b91f487288af81af7c02..91e8e76810470b1fb88ee660e45928fede25f1f1 100644 (file)
@@ -1043,6 +1043,12 @@ DefaultCommit<Impl>::commitInsts()
                 // Updates misc. registers.
                 head_inst->updateMiscRegs();
 
+                // Check instruction execution if it successfully commits and
+                // is not carrying a fault.
+                if (cpu->checker) {
+                    cpu->checker->verify(head_inst);
+                }
+
                 cpu->traceFunctions(pc[tid].instAddr());
 
                 TheISA::advancePC(pc[tid], head_inst->staticInst);
@@ -1168,12 +1174,6 @@ DefaultCommit<Impl>::commitHead(DynInstPtr &head_inst, unsigned inst_num)
         head_inst->setCompleted();
     }
 
-    // Use checker prior to updating anything due to traps or PC
-    // based events.
-    if (cpu->checker) {
-        cpu->checker->verify(head_inst);
-    }
-
     if (inst_fault != NoFault) {
         DPRINTF(Commit, "Inst [sn:%lli] PC %s has a fault\n",
                 head_inst->seqNum, head_inst->pcState());
@@ -1185,6 +1185,8 @@ DefaultCommit<Impl>::commitHead(DynInstPtr &head_inst, unsigned inst_num)
 
         head_inst->setCompleted();
 
+        // If instruction has faulted, let the checker execute it and
+        // check if it sees the same fault and control flow.
         if (cpu->checker) {
             // Need to check the instruction before its fault is processed
             cpu->checker->verify(head_inst);