Gabe Black has submitted this change. (
https://gem5-review.googlesource.com/c/public/gem5/+/49127 )
Change subject: cpu: Use std::variant to simplify InstResult.
......................................................................
cpu: Use std::variant to simplify InstResult.
std::variant is a similar to (and also modestly superior to)
MultiResult. Use it instead to simplify InstResult.
Change-Id: I22338f5e89814c6d13538129757158126013a414
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/49127
Reviewed-by: Giacomo Travaglini <[email protected]>
Maintainer: Giacomo Travaglini <[email protected]>
Tested-by: kokoro <[email protected]>
---
M src/cpu/checker/cpu.hh
M src/cpu/checker/cpu_impl.hh
M src/cpu/inst_res.hh
M src/cpu/o3/dyn_inst.hh
4 files changed, 47 insertions(+), 107 deletions(-)
Approvals:
Giacomo Travaglini: Looks good to me, approved; Looks good to me, approved
kokoro: Regressions pass
diff --git a/src/cpu/checker/cpu.hh b/src/cpu/checker/cpu.hh
index 4ed21f5..e32d12d 100644
--- a/src/cpu/checker/cpu.hh
+++ b/src/cpu/checker/cpu.hh
@@ -248,24 +248,21 @@
void
setScalarResult(T&& t)
{
- result.push(InstResult(std::forward<T>(t),
- InstResult::ResultType::Scalar));
+ result.push(InstResult(std::forward<T>(t)));
}
template<typename T>
void
setVecResult(T&& t)
{
- result.push(InstResult(std::forward<T>(t),
- InstResult::ResultType::VecReg));
+ result.push(InstResult(std::forward<T>(t)));
}
template<typename T>
void
setVecPredResult(T&& t)
{
- result.push(InstResult(std::forward<T>(t),
- InstResult::ResultType::VecPredReg));
+ result.push(InstResult(std::forward<T>(t)));
}
void
diff --git a/src/cpu/checker/cpu_impl.hh b/src/cpu/checker/cpu_impl.hh
index 26c22a5..638a737 100644
--- a/src/cpu/checker/cpu_impl.hh
+++ b/src/cpu/checker/cpu_impl.hh
@@ -477,15 +477,14 @@
// 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(0ul, InstResult::ResultType::Scalar),
idx);
+ copyResult(inst, InstResult((RegVal)0), 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(0ul, InstResult::ResultType::Scalar));
+ inst_val = inst->popResult(InstResult((RegVal)0));
if (checker_val != inst_val) {
result_mismatch = true;
idx = i;
diff --git a/src/cpu/inst_res.hh b/src/cpu/inst_res.hh
index 1a774c9..19ce640 100644
--- a/src/cpu/inst_res.hh
+++ b/src/cpu/inst_res.hh
@@ -39,8 +39,9 @@
#define __CPU_INST_RES_HH__
#include <type_traits>
+#include <variant>
-#include "arch/generic/vec_reg.hh"
+#include "arch/vecregs.hh"
#include "base/types.hh"
namespace gem5
@@ -48,81 +49,26 @@
class InstResult
{
- public:
- union MultiResult
- {
- RegVal integer;
- TheISA::VecRegContainer vector;
- TheISA::VecPredRegContainer pred;
- MultiResult() {}
- };
-
- enum class ResultType
- {
- Scalar,
- VecReg,
- VecPredReg,
- NumResultTypes,
- Invalid
- };
-
private:
- MultiResult result;
- ResultType type;
+ std::variant<RegVal, TheISA::VecRegContainer,
+ TheISA::VecPredRegContainer> result;
public:
/** Default constructor creates an invalid result. */
- InstResult() : type(ResultType::Invalid) { }
+ InstResult() = default;
InstResult(const InstResult &) = default;
- /** Scalar result from scalar. */
- template<typename T>
- explicit InstResult(T i, const ResultType& t) : type(t)
- {
- static_assert(std::is_integral_v<T> ^ std::is_floating_point_v<T>,
- "Parameter type is neither integral nor fp, or it is
both");
- if constexpr (std::is_integral_v<T>) {
- result.integer = i;
- } else if constexpr (std::is_floating_point_v<T>) {
- result.integer = floatToBits(i);
- }
- }
- /** Vector result. */
- explicit InstResult(const TheISA::VecRegContainer& v, const
ResultType& t)
- : type(t)
- {
- result.vector = v;
- }
- /** Predicate result. */
- explicit InstResult(const TheISA::VecPredRegContainer& v,
- const ResultType& t)
- : type(t)
- {
- result.pred = v;
- }
- InstResult&
+ template <typename T>
+ explicit InstResult(T val) : result(val) {}
+
+ template <typename T, typename enable=
+ std::enable_if_t<std::is_floating_point_v<T>>>
+ explicit InstResult(T val) : result(floatToBits(val)) {}
+
+ InstResult &
operator=(const InstResult& that)
{
- type = that.type;
- switch (type) {
- // Given that misc regs are not written to, there may be
invalids in
- //the result stack.
- case ResultType::Invalid:
- break;
- case ResultType::Scalar:
- result.integer = that.result.integer;
- break;
- case ResultType::VecReg:
- result.vector = that.result.vector;
- break;
- case ResultType::VecPredReg:
- result.pred = that.result.pred;
- break;
-
- default:
- panic("Assigning result from unknown result type");
- break;
- }
+ result = that.result;
return *this;
}
@@ -133,20 +79,7 @@
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::VecReg:
- return result.vector == that.result.vector;
- case ResultType::VecPredReg:
- return result.pred == that.result.pred;
- case ResultType::Invalid:
- return false;
- default:
- panic("Unknown type of result: %d\n", (int)type);
- }
+ return result == that.result;
}
bool
@@ -158,45 +91,59 @@
/** Checks */
/** @{ */
/** Is this a scalar result?. */
- bool isScalar() const { return type == ResultType::Scalar; }
+ bool
+ isScalar() const
+ {
+ return std::holds_alternative<RegVal>(result);
+ }
/** Is this a vector result?. */
- bool isVector() const { return type == ResultType::VecReg; }
+ bool
+ isVector() const
+ {
+ return std::holds_alternative<TheISA::VecRegContainer>(result);
+ }
/** Is this a predicate result?. */
- bool isPred() const { return type == ResultType::VecPredReg; }
+ bool
+ isPred() const
+ {
+ return std::holds_alternative<TheISA::VecPredRegContainer>(result);
+ }
+
/** Is this a valid result?. */
- bool isValid() const { return type != ResultType::Invalid; }
+ bool isValid() const { return result.index() != 0; }
/** @} */
/** Explicit cast-like operations. */
/** @{ */
- const uint64_t&
+ RegVal
asInteger() const
{
assert(isScalar());
- return result.integer;
+ return std::get<RegVal>(result);
}
/** 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&
+ RegVal
asIntegerNoAssert() const
{
- return result.integer;
+ const RegVal *ptr = std::get_if<RegVal>(&result);
+ return ptr ? *ptr : 0;
}
const TheISA::VecRegContainer&
asVector() const
{
panic_if(!isVector(), "Converting scalar (or invalid) to
vector!!");
- return result.vector;
+ return std::get<TheISA::VecRegContainer>(result);
}
const TheISA::VecPredRegContainer&
asPred() const
{
panic_if(!isPred(), "Converting scalar (or invalid) to
predicate!!");
- return result.pred;
+ return std::get<TheISA::VecPredRegContainer>(result);
}
/** @} */
diff --git a/src/cpu/o3/dyn_inst.hh b/src/cpu/o3/dyn_inst.hh
index a23be38..24c34b0 100644
--- a/src/cpu/o3/dyn_inst.hh
+++ b/src/cpu/o3/dyn_inst.hh
@@ -777,8 +777,7 @@
setScalarResult(T &&t)
{
if (instFlags[RecordResult]) {
- instResult.push(InstResult(std::forward<T>(t),
- InstResult::ResultType::Scalar));
+ instResult.push(InstResult(std::forward<T>(t)));
}
}
@@ -788,8 +787,7 @@
setVecResult(T &&t)
{
if (instFlags[RecordResult]) {
- instResult.push(InstResult(std::forward<T>(t),
- InstResult::ResultType::VecReg));
+ instResult.push(InstResult(std::forward<T>(t)));
}
}
@@ -799,8 +797,7 @@
setVecPredResult(T &&t)
{
if (instFlags[RecordResult]) {
- instResult.push(InstResult(std::forward<T>(t),
- InstResult::ResultType::VecPredReg));
+ instResult.push(InstResult(std::forward<T>(t)));
}
}
/** @} */
--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/49127
To unsubscribe, or for help writing mail filters, visit
https://gem5-review.googlesource.com/settings
Gerrit-Project: public/gem5
Gerrit-Branch: develop
Gerrit-Change-Id: I22338f5e89814c6d13538129757158126013a414
Gerrit-Change-Number: 49127
Gerrit-PatchSet: 4
Gerrit-Owner: Gabe Black <[email protected]>
Gerrit-Reviewer: Bobby R. Bruce <[email protected]>
Gerrit-Reviewer: Daniel Carvalho <[email protected]>
Gerrit-Reviewer: Gabe Black <[email protected]>
Gerrit-Reviewer: Giacomo Travaglini <[email protected]>
Gerrit-Reviewer: Jason Lowe-Power <[email protected]>
Gerrit-Reviewer: kokoro <[email protected]>
Gerrit-MessageType: merged
_______________________________________________
gem5-dev mailing list -- [email protected]
To unsubscribe send an email to [email protected]
%(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s
