derek/gem5
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

cpu: Simplify and revamp the InstResult class.

Browse Source 
The InstResult class is always used to store a register value, and also
only used to store a RegVal and not any more complex type like a
VecRegContainer. This is partially because the methods that *would*
store a complex result only have a pointer to work with, and don't have
a type to cast to to store the result in the InstResult.

This change reworks the InstResult class to hold the RegClass the
register goes with, and also either a standard RegVal, or a pointer to a
blob of memory holding the actual value if RegVal isn't appropriate. If
the InstResult has no RegClass, it is considered invalid.

To make working with InstResult easier, it also now has an "asString"
method which will just call into the RegClass's valString method with
the appropriate pointer.

By removing the ultimately unnecessary generality of the original class,
this change also simplifies InstResult significantly.

Change-Id: I71ace4da6c99b5dd82757e5365c493d795496fe5
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/50253
Maintainer: Gabe Black <gabe.black@gmail.com>
Tested-by: kokoro <noreply+kokoro@google.com>
Reviewed-by: Giacomo Travaglini <giacomo.travaglini@arm.com>
This commit is contained in:
Gabe Black
2021-09-11 02:02:20 -07:00
parent f698ca9c27
commit 81e07670b9
4 changed files with 119 additions and 157 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
src/cpu/checker/cpu.hh
View File

@@ -201,7 +201,7 @@ class CheckerCPU : public BaseCPU, public ExecContext
if (id.is(InvalidRegClass))
return;
thread->setReg(id, val);
result.emplace(val);
result.emplace(id.regClass(), val);
}
void
@@ -211,7 +211,7 @@ class CheckerCPU : public BaseCPU, public ExecContext
if (id.is(InvalidRegClass))
return;
thread->setReg(id, val);
//TODO setVecResult, setVecPredResult setVecElemResult?
result.emplace(id.regClass(), val);
}
bool readPredicate() const override { return thread->readPredicate(); }

75
src/cpu/checker/cpu_impl.hh
View File

@@ -466,24 +466,22 @@ Checker<DynInstPtr>::validateExecution(const DynInstPtr &inst)
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, InstResult((RegVal)0), idx);
copyResult(inst, InstResult(), 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++) {
checker_val = result.front();
result.pop();
inst_val = inst->popResult(InstResult((RegVal)0));
inst_val = inst->popResult();
if (checker_val != inst_val) {
result_mismatch = true;
idx = i;
scalar_mismatch = checker_val.is<RegVal>();
}
}
} // Checker CPU checks all the saved results in the dyninst passed by
@@ -493,12 +491,9 @@ Checker<DynInstPtr>::validateExecution(const DynInstPtr &inst)
// this is ok and not a bug. May be worthwhile to try and correct this.
if (result_mismatch) {
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.asNoAssert<RegVal>(),
checker_val.as<RegVal>());
}
warn("%lli: Instruction results (%i) do not match! Inst: %s, "
"checker: %s",
curTick(), idx, inst_val.asString(), checker_val.asString());
// It's useful to verify load values from memory, but in MP
// systems the value obtained at execute may be different than
@@ -580,56 +575,30 @@ Checker<DynInstPtr>::copyResult(
// so do the fix-up then start with the next dest reg;
if (start_idx >= 0) {
const RegId& idx = inst->destRegIdx(start_idx);
switch (idx.classValue()) {
case InvalidRegClass:
break;
case IntRegClass:
case FloatRegClass:
case VecElemClass:
case CCRegClass:
thread->setReg(idx, mismatch_val.as<RegVal>());
break;
case VecRegClass:
{
auto val = mismatch_val.as<TheISA::VecRegContainer>();
thread->setReg(idx, &val);
}
break;
case MiscRegClass:
thread->setMiscReg(idx.index(), mismatch_val.as<RegVal>());
break;
default:
panic("Unknown register class: %d", (int)idx.classValue());
}
if (idx.classValue() == InvalidRegClass)
; // Do nothing.
else if (idx.classValue() == MiscRegClass)
thread->setMiscReg(idx.index(), mismatch_val.asRegVal());
else if (mismatch_val.isBlob())
thread->setReg(idx, mismatch_val.asBlob());
else
thread->setReg(idx, mismatch_val.asRegVal());
}
start_idx++;
InstResult res;
for (int i = start_idx; i < inst->numDestRegs(); i++) {
const RegId& idx = inst->destRegIdx(i);
res = inst->popResult();
switch (idx.classValue()) {
case InvalidRegClass:
break;
case IntRegClass:
case FloatRegClass:
case VecElemClass:
case CCRegClass:
thread->setReg(idx, res.as<RegVal>());
break;
case VecRegClass:
{
auto val = res.as<TheISA::VecRegContainer>();
thread->setReg(idx, &val);
}
break;
case MiscRegClass:
// Try to get the proper misc register index for ARM here...
if (idx.classValue() == InvalidRegClass)
; // Do nothing.
else if (idx.classValue() == MiscRegClass)
thread->setMiscReg(idx.index(), 0);
break;
// else Register is out of range...
default:
panic("Unknown register class: %d", (int)idx.classValue());
}
else if (res.isBlob())
thread->setReg(idx, res.asBlob());
else
thread->setReg(idx, res.asRegVal());
}
}

189
src/cpu/inst_res.hh
View File

@@ -38,11 +38,15 @@
#ifndef __CPU_INST_RES_HH__
#define __CPU_INST_RES_HH__
#include <any>
#include <type_traits>
#include <cstdint>
#include <cstring>
#include <memory>
#include <string>
#include <variant>
#include "base/logging.hh"
#include "base/types.hh"
#include "cpu/reg_class.hh"
namespace gem5
{
@@ -50,65 +54,65 @@ namespace gem5
class InstResult
{
private:
std::any result;
std::function<bool(const std::any &a, const std::any &b)> equals;
using BlobPtr = std::unique_ptr<const uint8_t[]>;
std::variant<BlobPtr, RegVal> value;
const RegClass *_regClass = nullptr;
bool blob() const { return std::holds_alternative<BlobPtr>(value); }
bool valid() const { return _regClass != nullptr; }
// Raw accessors with no safety checks.
RegVal getRegVal() const { return std::get<RegVal>(value); }
const void *getBlob() const { return std::get<BlobPtr>(value).get(); }
// Store copies of blobs, not a pointer to the original.
void
set(const void *val)
{
uint8_t *temp = nullptr;
if (val) {
const size_t size = _regClass->regBytes();
temp = new uint8_t[size];
std::memcpy(temp, val, size);
}
value = BlobPtr(temp);
}
void set(RegVal val) { value = val; }
void
set(const InstResult &other)
{
other.blob() ? set(other.getBlob()) : set(other.getRegVal());
}
public:
/** Default constructor creates an invalid result. */
InstResult() :
// This InstResult is empty, and will only equal other InstResults
// which are also empty.
equals([](const std::any &a, const std::any &b) -> bool {
gem5_assert(!a.has_value());
return !b.has_value();
})
{}
InstResult(const InstResult &) = default;
template <typename T>
explicit InstResult(T val) : result(val),
// Set equals so it knows how to compare results of type T.
equals([](const std::any &a, const std::any &b) -> bool {
// If one has a value but the other doesn't, not equal.
if (a.has_value() != b.has_value())
return false;
// If they are both empty, equal.
if (!a.has_value())
return true;
// At least the local object should be of the right type.
gem5_assert(a.type() == typeid(T));
// If these aren't the same type, not equal.
if (a.type() != b.type())
return false;
// We now know these both hold a result of the right type.
return std::any_cast<const T&>(a) == std::any_cast<const T&>(b);
})
InstResult() {}
InstResult(const InstResult &other) : _regClass(other._regClass)
{
static_assert(!std::is_pointer_v<T>,
"InstResult shouldn't point to external data.");
// Floating point values should be converted to/from ints using
// floatToBits and bitsToFloat, and not stored in InstResult directly.
static_assert(!std::is_floating_point_v<T>,
"Floating point values should be converted to/from ints.");
set(other);
}
// Convert floating point values to integers.
template <typename T,
std::enable_if_t<std::is_floating_point_v<T>, int> = 0>
explicit InstResult(T val) : InstResult(floatToBits(val)) {}
InstResult(const RegClass &reg_class, RegVal val) :
_regClass(&reg_class)
{
set(val);
}
// Convert all integer types to RegVal.
template <typename T,
std::enable_if_t<std::is_integral_v<T> && !std::is_same_v<T, RegVal>,
int> = 0>
explicit InstResult(T val) : InstResult(static_cast<RegVal>(val)) {}
InstResult(const RegClass &reg_class, const void *val) :
_regClass(&reg_class)
{
set(val);
}
InstResult &
operator=(const InstResult& that)
operator=(const InstResult &that)
{
result = that.result;
equals = that.equals;
_regClass = that._regClass;
set(that);
return *this;
}
@@ -119,7 +123,23 @@ class InstResult
bool
operator==(const InstResult& that) const
{
return equals(result, that.result);
if (blob() != that.blob() || _regClass != that._regClass)
return false;
if (blob()) {
const void *my_blob = getBlob();
const void *their_blob = that.getBlob();
// Invalid results always differ.
if (!my_blob || !their_blob)
return false;
// Check the contents of the blobs, not their addresses.
return std::memcmp(getBlob(), that.getBlob(),
_regClass->regBytes()) == 0;
} else {
return getRegVal() == that.getRegVal();
}
}
bool
@@ -128,61 +148,34 @@ class InstResult
return !operator==(that);
}
/** Checks */
/** @{ */
const RegClass &regClass() const { return *_regClass; }
bool isValid() const { return valid(); }
bool isBlob() const { return blob(); }
template <typename T>
bool
is() const
RegVal
asRegVal() const
{
static_assert(!std::is_floating_point_v<T>,
"Floating point values should be converted to/from ints.");
return result.type() == typeid(T);
assert(!blob());
return getRegVal();
}
template <typename T>
std::enable_if_t<std::is_integral_v<T> && !std::is_same_v<T, RegVal>, bool>
is() const
const void *
asBlob() const
{
return is<RegVal>();
assert(blob());
return getBlob();
}
/** Is this a valid result?. */
bool isValid() const { return result.has_value(); }
/** @} */
/** Explicit cast-like operations. */
/** @{ */
template <typename T>
T
as() const
std::string
asString() const
{
assert(is<T>());
return std::any_cast<T>(result);
if (blob()) {
return _regClass->valString(getBlob());
} else {
RegVal reg = getRegVal();
return _regClass->valString(&reg);
}
}
template <typename T>
std::enable_if_t<std::is_integral_v<T> && !std::is_same_v<T, RegVal>,
RegVal>
as() const
{
return as<RegVal>();
}
/** 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. */
template <typename T>
T
asNoAssert() const
{
if (!is<T>())
return T{};
return as<T>();
}
/** @} */
};
} // namespace gem5

8
src/cpu/o3/dyn_inst.hh
View File

@@ -713,10 +713,10 @@ class DynInst : public ExecContext, public RefCounted
/** @{ */
template<typename T>
void
setResult(T &&t)
setResult(const RegClass &reg_class, T &&t)
{
if (instFlags[RecordResult]) {
instResult.emplace(std::forward<T>(t));
instResult.emplace(reg_class, std::forward<T>(t));
}
}
/** @} */
@@ -1144,7 +1144,7 @@ class DynInst : public ExecContext, public RefCounted
if (reg->is(InvalidRegClass))
return;
cpu->setReg(reg, val);
setResult(val);
setResult(reg->regClass(), val);
}
void
@@ -1154,7 +1154,7 @@ class DynInst : public ExecContext, public RefCounted
if (reg->is(InvalidRegClass))
return;
cpu->setReg(reg, val);
//TODO setResult
setResult(reg->regClass(), val);
}
};