cpu: Result refactoring

The Result union used to collect the result of an instruction is now a
class of its own, with its constructor, and explicit casting methods for
cleanliness.

This is also a stepping stone to have vector registers, and instructions
that produce a vector register as output.

Change-Id: I6f40c11cb5e835d8b11f7804a4e967aff18025b9
Reviewed-by: Andreas Sandberg <andreas.sandberg@arm.com>
Reviewed-on: https://gem5-review.googlesource.com/2703
Reviewed-by: Anthony Gutierrez <anthony.gutierrez@amd.com>
Reviewed-by: Jason Lowe-Power <jason@lowepower.com>
Maintainer: Andreas Sandberg <andreas.sandberg@arm.com>

4 files changed, 205 insertions, 78 deletions

diff --git a/src/cpu/base_dyn_inst.hh b/src/cpu/base_dyn_inst.hh
index 9c6952310..a8e619cd9 100644
--- a/src/cpu/base_dyn_inst.hh
+++ b/src/cpu/base_dyn_inst.hh
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2011,2013 ARM Limited
+ * Copyright (c) 2011,2013,2016 ARM Limited
  * Copyright (c) 2013 Advanced Micro Devices, Inc.
  * All rights reserved.
  *
@@ -49,18 +49,19 @@
 #include <array>
 #include <bitset>
 #include <list>
-#include <string>
 #include <queue>
+#include <string>
 
 #include "arch/generic/tlb.hh"
 #include "arch/utility.hh"
 #include "base/trace.hh"
 #include "config/the_isa.hh"
 #include "cpu/checker/cpu.hh"
-#include "cpu/o3/comm.hh"
 #include "cpu/exec_context.hh"
 #include "cpu/exetrace.hh"
+#include "cpu/inst_res.hh"
 #include "cpu/inst_seq.hh"
+#include "cpu/o3/comm.hh"
 #include "cpu/op_class.hh"
 #include "cpu/static_inst.hh"
 #include "cpu/translation.hh"
@@ -94,15 +95,6 @@ class BaseDynInst : public ExecContext, public RefCounted
         MaxInstDestRegs = TheISA::MaxInstDestRegs       /// Max dest regs
     };
 
-    union Result {
-        uint64_t integer;
-        double dbl;
-        void set(uint64_t i) { integer = i; }
-        void set(double d) { dbl = d; }
-        void get(uint64_t& i) { i = integer; }
-        void get(double& d) { d = dbl; }
-    };
-
   protected:
     enum Status {
         IqEntry,                 /// Instruction is in the IQ
@@ -174,7 +166,7 @@ class BaseDynInst : public ExecContext, public RefCounted
     /** The result of the instruction; assumes an instruction can have many
      *  destination registers.
      */
-    std::queue<Result> instResult;
+    std::queue<InstResult> instResult;
 
     /** PC state for this instruction. */
     TheISA::PCState pc;
@@ -606,56 +598,55 @@ class BaseDynInst : public ExecContext, public RefCounted
     /** Returns the logical register index of the i'th source register. */
     const RegId& srcRegIdx(int i) const { return staticInst->srcRegIdx(i); }
 
-    /** Pops a result off the instResult queue */
-    template <class T>
-    void popResult(T& t)
+    /** Return the size of the instResult queue. */
+    uint8_t resultSize() { return instResult.size(); }
+
+    /** Pops a result off the instResult queue.
+     * If the result stack is empty, return the default value.
+     * */
+    InstResult popResult(InstResult dflt = InstResult())
     {
         if (!instResult.empty()) {
-            instResult.front().get(t);
+            InstResult t = instResult.front();
             instResult.pop();
+            return t;
         }
+        return dflt;
     }
 
-    /** Read the most recent result stored by this instruction */
-    template <class T>
-    void readResult(T& t)
-    {
-        instResult.back().get(t);
-    }
-
-    /** Pushes a result onto the instResult queue */
-    template <class T>
-    void setResult(T t)
+    /** Pushes a result onto the instResult queue. */
+    template<typename T>
+    void setScalarResult(T&& t)
     {
         if (instFlags[RecordResult]) {
-            Result instRes;
-            instRes.set(t);
-            instResult.push(instRes);
+            instResult.push(InstResult(std::forward<T>(t),
+                        InstResult::ResultType::Scalar));
         }
     }
 
     /** Records an integer register being set to a value. */
     void setIntRegOperand(const StaticInst *si, int idx, IntReg val)
     {
-        setResult<uint64_t>(val);
+        setScalarResult(val);
     }
 
     /** Records a CC register being set to a value. */
     void setCCRegOperand(const StaticInst *si, int idx, CCReg val)
     {
-        setResult<uint64_t>(val);
+        setScalarResult(val);
     }
 
     /** Records an fp register being set to a value. */
     void setFloatRegOperand(const StaticInst *si, int idx, FloatReg val)
     {
-        setResult<double>(val);
+        setScalarResult(val);
     }
 
     /** Records an fp register being set to an integer value. */
-    void setFloatRegOperandBits(const StaticInst *si, int idx, FloatRegBits val)
+    void
+    setFloatRegOperandBits(const StaticInst *si, int idx, FloatRegBits val)
     {
-        setResult<uint64_t>(val);
+        setScalarResult(val);
     }
 
     /** Records that one of the source registers is ready. */
diff --git a/src/cpu/checker/cpu.hh b/src/cpu/checker/cpu.hh
index 304caaa85..6571d034a 100644
--- a/src/cpu/checker/cpu.hh
+++ b/src/cpu/checker/cpu.hh
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2011 ARM Limited
+ * Copyright (c) 2011, 2016 ARM Limited
  * Copyright (c) 2013 Advanced Micro Devices, Inc.
  * All rights reserved
  *
@@ -53,6 +53,7 @@
 #include "cpu/base.hh"
 #include "cpu/base_dyn_inst.hh"
 #include "cpu/exec_context.hh"
+#include "cpu/inst_res.hh"
 #include "cpu/pc_event.hh"
 #include "cpu/simple_thread.hh"
 #include "cpu/static_inst.hh"
@@ -143,18 +144,9 @@ class CheckerCPU : public BaseCPU, public ExecContext
 
     Addr dbg_vtophys(Addr addr);
 
-    union Result {
-        uint64_t integer;
-        double dbl;
-        void set(uint64_t i) { integer = i; }
-        void set(double d) { dbl = d; }
-        void get(uint64_t& i) { i = integer; }
-        void get(double& d) { d = dbl; }
-    };
-
     // ISAs like ARM can have multiple destination registers to check,
     // keep them all in a std::queue
-    std::queue<Result> result;
+    std::queue<InstResult> result;
 
     // Pointer to the one memory request.
     RequestPtr memReq;
@@ -240,12 +232,11 @@ class CheckerCPU : public BaseCPU, public ExecContext
         return thread->readCCReg(reg.index());
     }
 
-    template <class T>
-    void setResult(T t)
+    template<typename T>
+    void setScalarResult(T&& t)
     {
-        Result instRes;
-        instRes.set(t);
-        result.push(instRes);
+        result.push(InstResult(std::forward<T>(t),
+                        InstResult::ResultType::Scalar));
     }
 
     void setIntRegOperand(const StaticInst *si, int idx,
@@ -254,7 +245,7 @@ class CheckerCPU : public BaseCPU, public ExecContext
         const RegId& reg = si->destRegIdx(idx);
         assert(reg.isIntReg());
         thread->setIntReg(reg.index(), val);
-        setResult<uint64_t>(val);
+        setScalarResult(val);
     }
 
     void setFloatRegOperand(const StaticInst *si, int idx,
@@ -263,7 +254,7 @@ class CheckerCPU : public BaseCPU, public ExecContext
         const RegId& reg = si->destRegIdx(idx);
         assert(reg.isFloatReg());
         thread->setFloatReg(reg.index(), val);
-        setResult<double>(val);
+        setScalarResult(val);
     }
 
     void setFloatRegOperandBits(const StaticInst *si, int idx,
@@ -272,7 +263,7 @@ class CheckerCPU : public BaseCPU, public ExecContext
         const RegId& reg = si->destRegIdx(idx);
         assert(reg.isFloatReg());
         thread->setFloatRegBits(reg.index(), val);
-        setResult<uint64_t>(val);
+        setScalarResult(val);
     }
 
     void setCCRegOperand(const StaticInst *si, int idx, CCReg val) override
@@ -280,7 +271,7 @@ class CheckerCPU : public BaseCPU, public ExecContext
         const RegId& reg = si->destRegIdx(idx);
         assert(reg.isCCReg());
         thread->setCCReg(reg.index(), val);
-        setResult<uint64_t>(val);
+        setScalarResult((uint64_t)val);
     }
 
     bool readPredicate() override { return thread->readPredicate(); }
@@ -422,7 +413,7 @@ class CheckerCPU : public BaseCPU, public ExecContext
     ThreadContext *tcBase() override { return tc; }
     SimpleThread *threadBase() { return thread; }
 
-    Result unverifiedResult;
+    InstResult unverifiedResult;
     Request *unverifiedReq;
     uint8_t *unverifiedMemData;
 
@@ -464,7 +455,8 @@ class Checker : public CheckerCPU
     void validateExecution(DynInstPtr &inst);
     void validateState();
 
-    void copyResult(DynInstPtr &inst, uint64_t mismatch_val, int start_idx);
+    void copyResult(DynInstPtr &inst, const InstResult& mismatch_val,
+                    int start_idx);
     void handlePendingInt();
 
   private:
diff --git a/src/cpu/checker/cpu_impl.hh b/src/cpu/checker/cpu_impl.hh
index 0c90590c7..ed86aec84 100644
--- a/src/cpu/checker/cpu_impl.hh
+++ b/src/cpu/checker/cpu_impl.hh
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2011 ARM Limited
+ * Copyright (c) 2011, 2016 ARM Limited
  * Copyright (c) 2013 Advanced Micro Devices, Inc.
  * All rights reserved
  *
@@ -481,27 +481,29 @@ template <class Impl>
 void
 Checker<Impl>::validateExecution(DynInstPtr &inst)
 {
-    uint64_t checker_val;
-    uint64_t inst_val;
+    InstResult checker_val;
+    InstResult inst_val;
     int idx = -1;
     bool result_mismatch = false;
+    bool scalar_mismatch = false;
 
     if (inst->isUnverifiable()) {
         // Unverifiable instructions assume they were executed
         // properly by the CPU. Grab the result from the
         // instruction and write it to the register.
-        copyResult(inst, 0, idx);
+        copyResult(inst, InstResult(0ul, InstResult::ResultType::Scalar), idx);
     } else if (inst->numDestRegs() > 0 && !result.empty()) {
         DPRINTF(Checker, "Dest regs %d, number of checker dest regs %d\n",
                          inst->numDestRegs(), result.size());
         for (int i = 0; i < inst->numDestRegs() && !result.empty(); i++) {
-            result.front().get(checker_val);
+            checker_val = result.front();
             result.pop();
-            inst_val = 0;
-            inst->template popResult<uint64_t>(inst_val);
+            inst_val = inst->popResult(
+                    InstResult(0ul, InstResult::ResultType::Scalar));
             if (checker_val != inst_val) {
                 result_mismatch = true;
                 idx = i;
+                scalar_mismatch = true;
                 break;
             }
         }
@@ -512,9 +514,12 @@ Checker<Impl>::validateExecution(DynInstPtr &inst)
       // this is ok and not a bug.  May be worthwhile to try and correct this.
 
     if (result_mismatch) {
-        warn("%lli: Instruction results do not match! (Values may not "
-             "actually be integers) Inst: %#x, checker: %#x",
-             curTick(), inst_val, checker_val);
+        if (scalar_mismatch) {
+            warn("%lli: Instruction results (%i) do not match! (Values may"
+                 " not actually be integers) Inst: %#x, checker: %#x",
+                 curTick(), idx, inst_val.asIntegerNoAssert(),
+                 checker_val.asInteger());
+        }
 
         // It's useful to verify load values from memory, but in MP
         // systems the value obtained at execute may be different than
@@ -589,7 +594,7 @@ Checker<Impl>::validateState()
 
 template <class Impl>
 void
-Checker<Impl>::copyResult(DynInstPtr &inst, uint64_t mismatch_val,
+Checker<Impl>::copyResult(DynInstPtr &inst, const InstResult& mismatch_val,
                           int start_idx)
 {
     // We've already popped one dest off the queue,
@@ -598,37 +603,45 @@ Checker<Impl>::copyResult(DynInstPtr &inst, uint64_t mismatch_val,
         const RegId& idx = inst->destRegIdx(start_idx);
         switch (idx.classValue()) {
           case IntRegClass:
-            thread->setIntReg(idx.index(), mismatch_val);
+            panic_if(!mismatch_val.isScalar(), "Unexpected type of result");
+            thread->setIntReg(idx.index(), mismatch_val.asInteger());
             break;
           case FloatRegClass:
-            thread->setFloatRegBits(idx.index(), mismatch_val);
+            panic_if(!mismatch_val.isScalar(), "Unexpected type of result");
+            thread->setFloatRegBits(idx.index(), mismatch_val.asInteger());
             break;
           case CCRegClass:
-            thread->setCCReg(idx.index(), mismatch_val);
+            panic_if(!mismatch_val.isScalar(), "Unexpected type of result");
+            thread->setCCReg(idx.index(), mismatch_val.asInteger());
             break;
           case MiscRegClass:
-            thread->setMiscReg(idx.index(), mismatch_val);
+            panic_if(!mismatch_val.isScalar(), "Unexpected type of result");
+            thread->setMiscReg(idx.index(), mismatch_val.asInteger());
             break;
         }
     }
     start_idx++;
-    uint64_t res = 0;
+    InstResult res;
     for (int i = start_idx; i < inst->numDestRegs(); i++) {
         const RegId& idx = inst->destRegIdx(i);
-        inst->template popResult<uint64_t>(res);
+        res = inst->popResult();
         switch (idx.classValue()) {
           case IntRegClass:
-            thread->setIntReg(idx.index(), res);
+            panic_if(!res.isScalar(), "Unexpected type of result");
+            thread->setIntReg(idx.index(), res.asInteger());
             break;
           case FloatRegClass:
-            thread->setFloatRegBits(idx.index(), res);
+            panic_if(!res.isScalar(), "Unexpected type of result");
+            thread->setFloatRegBits(idx.index(), res.asInteger());
             break;
           case CCRegClass:
-            thread->setCCReg(idx.index(), res);
+            panic_if(!res.isScalar(), "Unexpected type of result");
+            thread->setCCReg(idx.index(), res.asInteger());
             break;
           case MiscRegClass:
+            panic_if(res.isValid(), "MiscReg expecting invalid result");
             // Try to get the proper misc register index for ARM here...
-            thread->setMiscReg(idx.index(), res);
+            thread->setMiscReg(idx.index(), 0);
             break;
             // else Register is out of range...
         }
diff --git a/src/cpu/inst_res.hh b/src/cpu/inst_res.hh
new file mode 100644
index 000000000..f6f14fe19
--- /dev/null
+++ b/src/cpu/inst_res.hh
@@ -0,0 +1,131 @@
+/*
+ * Copyright (c) 2016 ARM Limited
+ * All rights reserved
+ *
+ * The license below extends only to copyright in the software and shall
+ * not be construed as granting a license to any other intellectual
+ * property including but not limited to intellectual property relating
+ * to a hardware implementation of the functionality of the software
+ * licensed hereunder.  You may use the software subject to the license
+ * terms below provided that you ensure that this notice is replicated
+ * unmodified and in its entirety in all distributions of the software,
+ * modified or unmodified, in source code or in binary form.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions are
+ * met: redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer;
+ * redistributions in binary form must reproduce the above copyright
+ * notice, this list of conditions and the following disclaimer in the
+ * documentation and/or other materials provided with the distribution;
+ * neither the name of the copyright holders nor the names of its
+ * contributors may be used to endorse or promote products derived from
+ * this software without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+ * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ *
+ * Authors: Nathanael Premillieu
+ */
+
+#ifndef __CPU_INST_RES_HH__
+#define __CPU_INST_RES_HH__
+
+#include <type_traits>
+
+#include "arch/generic/types.hh"
+
+class InstResult {
+  public:
+    union MultiResult {
+        uint64_t integer;
+        double dbl;
+        MultiResult() {}
+    };
+
+    enum class ResultType {
+        Scalar,
+        NumResultTypes,
+        Invalid
+    };
+
+  private:
+    MultiResult result;
+    ResultType type;
+
+  public:
+    /** Default constructor creates an invalid result. */
+    InstResult() : type(ResultType::Invalid) { }
+    /** Scalar result from scalar. */
+    template<typename T>
+    explicit InstResult(T i, const ResultType& t) : type(t) {
+        static_assert(std::is_integral<T>::value ^
+                        std::is_floating_point<T>::value,
+                "Parameter type is neither integral nor fp, or it is both");
+        if (std::is_integral<T>::value) {
+            result.integer = i;
+        } else if (std::is_floating_point<T>::value) {
+            result.dbl = i;
+        }
+    }
+
+    /**
+     * Result comparison
+     * Two invalid results always differ.
+     */
+    bool operator==(const InstResult& that) const {
+        if (this->type != that.type)
+            return false;
+        switch (type) {
+        case ResultType::Scalar:
+            return result.integer == that.result.integer;
+        case ResultType::Invalid:
+            return false;
+        default:
+            panic("Unknown type of result: %d\n", (int)type);
+        }
+    }
+
+    bool operator!=(const InstResult& that) const {
+        return !operator==(that);
+    }
+
+    /** Checks */
+    /** @{ */
+    /** Is this a scalar result?. */
+    bool isScalar() const { return type == ResultType::Scalar; }
+    /** Is this a valid result?. */
+    bool isValid() const { return type != ResultType::Invalid; }
+    /** @} */
+
+    /** Explicit cast-like operations. */
+    /** @{ */
+    const uint64_t&
+    asInteger() const
+    {
+        assert(isScalar());
+        return result.integer;
+    }
+
+    /** Cast to integer without checking type.
+     * This is required to have the o3 cpu checker happy, as it
+     * compares results as integers without being fully aware of
+     * their nature. */
+    const uint64_t&
+    asIntegerNoAssert() const
+    {
+        return result.integer;
+    }
+    /** @} */
+};
+
+#endif // __CPU_INST_RES_HH__