From: Rekai Gonzalez-Alberquilla Date: Wed, 5 Apr 2017 18:20:30 +0000 (-0500) Subject: cpu: Result refactoring X-Git-Tag: v19.0.0.0~2712 X-Git-Url: https://git.libre-soc.org/?a=commitdiff_plain;h=2da7656a9a2fbf30cac0caffa4a2d168f736b4a1;p=gem5.git 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 Reviewed-on: https://gem5-review.googlesource.com/2703 Reviewed-by: Anthony Gutierrez Reviewed-by: Jason Lowe-Power Maintainer: Andreas Sandberg --- 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 #include #include -#include #include +#include #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 instResult; + std::queue 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 - 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 - void readResult(T& t) - { - instResult.back().get(t); - } - - /** Pushes a result onto the instResult queue */ - template - void setResult(T t) + /** Pushes a result onto the instResult queue. */ + template + void setScalarResult(T&& t) { if (instFlags[RecordResult]) { - Result instRes; - instRes.set(t); - instResult.push(instRes); + instResult.push(InstResult(std::forward(t), + InstResult::ResultType::Scalar)); } } /** Records an integer register being set to a value. */ void setIntRegOperand(const StaticInst *si, int idx, IntReg val) { - setResult(val); + setScalarResult(val); } /** Records a CC register being set to a value. */ void setCCRegOperand(const StaticInst *si, int idx, CCReg val) { - setResult(val); + setScalarResult(val); } /** Records an fp register being set to a value. */ void setFloatRegOperand(const StaticInst *si, int idx, FloatReg val) { - setResult(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(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; + std::queue 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 - void setResult(T t) + template + void setScalarResult(T&& t) { - Result instRes; - instRes.set(t); - result.push(instRes); + result.push(InstResult(std::forward(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(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(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(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(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 void Checker::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(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::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::validateState() template void -Checker::copyResult(DynInstPtr &inst, uint64_t mismatch_val, +Checker::copyResult(DynInstPtr &inst, const InstResult& mismatch_val, int start_idx) { // We've already popped one dest off the queue, @@ -598,37 +603,45 @@ Checker::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(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 + +#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 + explicit InstResult(T i, const ResultType& t) : type(t) { + static_assert(std::is_integral::value ^ + std::is_floating_point::value, + "Parameter type is neither integral nor fp, or it is both"); + if (std::is_integral::value) { + result.integer = i; + } else if (std::is_floating_point::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__