Gabe Black has submitted this change. (
https://gem5-review.googlesource.com/c/public/gem5/+/36280 )
Change subject: sim: Refactor how serialization types are handled in the
backend.
......................................................................
sim: Refactor how serialization types are handled in the backend.
The parseParam and showParam functions partially worked using template
specialization, and partially worked using function overloading. The
template specialization could be resolved later once other functions
were added, but the regular function overloads could not. That meant
that it was practically impossible to add new definitions of those two
functions local to the types they worked with.
Also, because C++ does not allow partial specialization of template
functions, it would not be possible to truly use specialization to wire
in BitUnion types.
To fix these problems, these functions have been turned into structs
which wrap static functions. These can be partially specialized as
desired, making them compatible with BitUnions. Also, it's not possible
to overload structures like it is with functions, so only specialization
is considered, not overloading.
While making these changes, these functions (now structs) were also
reworked so that they share implementation more, and are generally
more streamlined.
Given the fact that the previous parseParam and showParam functions
could not actually be expanded beyond serialize.hh, and were not
actually called directly by any code outside of that file, they should
have never been considered part of the API.
Now that these structs actually *can* be specialized outside of this
file, they should be considered part of the interface.
Change-Id: Ic8e677b97fda8378ee1da1f3cf6001e02783fde3
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/36280
Reviewed-by: Jason Lowe-Power <[email protected]>
Reviewed-by: Richard Cooper <[email protected]>
Reviewed-by: Andreas Sandberg <[email protected]>
Maintainer: Andreas Sandberg <[email protected]>
Tested-by: kokoro <[email protected]>
---
M src/sim/serialize.hh
1 file changed, 118 insertions(+), 124 deletions(-)
Approvals:
Jason Lowe-Power: Looks good to me, approved
Andreas Sandberg: Looks good to me, approved; Looks good to me, approved
Richard Cooper: Looks good to me, but someone else must approve
kokoro: Regressions pass
diff --git a/src/sim/serialize.hh b/src/sim/serialize.hh
index b5fb47c..3adcc5b 100644
--- a/src/sim/serialize.hh
+++ b/src/sim/serialize.hh
@@ -320,139 +320,131 @@
/**
* @ingroup api_serialize
+ * @{
*/
+
+// To add support for a new type of field that can be serialized, define
+// template specializations of the two classes below, ParseParam and
ShowParam,
+// as described above each. The way ParseParam is specialized for
std::string
+// or ShowParam is specialied for bool can be used as examples.
+
+/*
+ * A structure which should be specialized to contain a static method with
the
+ * signature:
+ *
+ * bool parse(const std::string &s, T &value)
+ *
+ * which fills in value using the contents of s, and returns if that was
+ * successful.
+ */
+template <class T, class Enable=void>
+struct ParseParam;
+
+// Specialization for anything to_number can accept.
template <class T>
-bool
-parseParam(const std::string &s, T &value)
+struct ParseParam<T, decltype(to_number("", std::declval<T&>()), void())>
{
- // The base implementations use to_number for parsing and '<<' for
- // displaying, suitable for integer types.
- return to_number(s, value);
-}
+ static bool
+ parse(const std::string &s, T &value)
+ {
+ return to_number(s, value);
+ }
+};
-/**
- * @ingroup api_serialize
- */
+template <>
+struct ParseParam<bool>
+{
+ static bool
+ parse(const std::string &s, bool &value)
+ {
+ return to_bool(s, value);
+ }
+};
+
+template <>
+struct ParseParam<std::string>
+{
+ static bool
+ parse(const std::string &s, std::string &value)
+ {
+ // String requires no processing to speak of
+ value = s;
+ return true;
+ }
+};
+
+// Specialization for BitUnion types.
template <class T>
-void
-showParam(CheckpointOut &os, const T &value)
+struct ParseParam<BitUnionType<T>>
{
- os << value;
-}
+ static bool
+ parse(const std::string &s, BitUnionType<T> &value)
+ {
+ // Zero initialize storage to avoid leaking an uninitialized value
+ BitUnionBaseType<T> storage = BitUnionBaseType<T>();
+ auto res = to_number(s, storage);
+ value = storage;
+ return res;
+ }
+};
-/**
- * @ingroup api_serialize
+/*
+ * A structure which should be specialized to contain a static method with
the
+ * signature:
+ *
+ * void show(std::ostream &os, const T &value)
+ *
+ * which outputs value to the stream os.
+ *
+ * This default implementation falls back to the << operator which should
work
+ * for many types.
*/
+template <class T, class Enabled=void>
+struct ShowParam
+{
+ static void show(std::ostream &os, const T &value) { os << value; }
+};
+
+// Handle characters specially so that we print their value, not the
character
+// they encode.
template <class T>
-bool
-parseParam(const std::string &s, BitUnionType<T> &value)
+struct ShowParam<T, std::enable_if_t<std::is_same<char, T>::value ||
+ std::is_same<unsigned char, T>::value
||
+ std::is_same<signed char, T>::value>>
{
- // Zero initialize storage to avoid leaking an uninitialized value
- BitUnionBaseType<T> storage = BitUnionBaseType<T>();
- auto res = to_number(s, storage);
- value = storage;
- return res;
-}
+ static void
+ show(std::ostream &os, const T &value)
+ {
+ if (std::is_signed<T>::value)
+ os << (int)value;
+ else
+ os << (unsigned int)value;
+ }
+};
-/**
- * @ingroup api_serialize
- */
+template <>
+struct ShowParam<bool>
+{
+ static void
+ show(std::ostream &os, const bool &value)
+ {
+ // Display bools as strings
+ os << (value ? "true" : "false");
+ }
+};
+
template <class T>
-void
-showParam(CheckpointOut &os, const BitUnionType<T> &value)
+struct ShowParam<BitUnionType<T>>
{
- auto storage = static_cast<BitUnionBaseType<T>>(value);
+ static void
+ show(std::ostream &os, const BitUnionType<T> &value)
+ {
+ ShowParam<BitUnionBaseType<T>>::show(
+ os, static_cast<const BitUnionBaseType<T> &>(value));
+ }
+};
- // For a BitUnion8, the storage type is an unsigned char.
- // Since we want to serialize a number we need to cast to
- // unsigned int
- os << ((sizeof(storage) == 1) ?
- static_cast<unsigned int>(storage) : storage);
-}
-
-/**
- * @ingroup api_serialize
- */
-template <>
-inline void
-showParam(CheckpointOut &os, const char &value)
-{
- // Treat 8-bit ints (chars) as ints on output, not as chars
- os << (int)value;
-}
-
-/**
- * @ingroup api_serialize
- */
-template <>
-inline void
-showParam(CheckpointOut &os, const signed char &value)
-{
- os << (int)value;
-}
-
-/**
- * @ingroup api_serialize
- */
-template <>
-inline void
-showParam(CheckpointOut &os, const unsigned char &value)
-{
- os << (unsigned int)value;
-}
-
-/**
- * @ingroup api_serialize
- */
-template <>
-inline bool
-parseParam(const std::string &s, float &value)
-{
- return to_number(s, value);
-}
-
-/**
- * @ingroup api_serialize
- */
-template <>
-inline bool
-parseParam(const std::string &s, double &value)
-{
- return to_number(s, value);
-}
-
-/**
- * @ingroup api_serialize
- */
-template <>
-inline bool
-parseParam(const std::string &s, bool &value)
-{
- return to_bool(s, value);
-}
-
-/**
- * @ingroup api_serialize
- */
-template <>
-inline void
-showParam(CheckpointOut &os, const bool &value)
-{
- // Display bools as strings
- os << (value ? "true" : "false");
-}
-
-/**
- * @ingroup api_serialize
- */
-template <>
-inline bool
-parseParam(const std::string &s, std::string &value)
-{
- // String requires no processing to speak of
- value = s;
- return true;
-}
+/** @} */
/**
* This function is used for writing parameters to a checkpoint.
@@ -466,7 +458,7 @@
paramOut(CheckpointOut &os, const std::string &name, const T ¶m)
{
os << name << "=";
- showParam(os, param);
+ ShowParam<T>::show(os, param);
os << "\n";
}
@@ -476,7 +468,7 @@
{
const std::string §ion(Serializable::currentSection());
std::string str;
- return cp.find(section, name, str) && parseParam(str, param);
+ return cp.find(section, name, str) && ParseParam<T>::parse(str, param);
}
/**
@@ -529,11 +521,12 @@
{
os << name << "=";
auto it = start;
+ using Elem = std::remove_cv_t<std::remove_reference_t<decltype(*it)>>;
if (it != end)
- showParam(os, *it++);
+ ShowParam<Elem>::show(os, *it++);
while (it != end) {
os << " ";
- showParam(os, *it++);
+ ShowParam<Elem>::show(os, *it++);
}
os << "\n";
}
@@ -593,7 +586,8 @@
for (const auto &token: tokens) {
T value;
- fatal_if(!parseParam(token, value), "Could not parse \"%s\".",
str);
+ fatal_if(!ParseParam<T>::parse(token, value),
+ "Could not parse \"%s\".", str);
*inserter = value;
}
}
--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/36280
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: Ic8e677b97fda8378ee1da1f3cf6001e02783fde3
Gerrit-Change-Number: 36280
Gerrit-PatchSet: 5
Gerrit-Owner: Gabe Black <[email protected]>
Gerrit-Reviewer: Andreas Sandberg <[email protected]>
Gerrit-Reviewer: Bobby R. Bruce <[email protected]>
Gerrit-Reviewer: Gabe Black <[email protected]>
Gerrit-Reviewer: Giacomo Travaglini <[email protected]>
Gerrit-Reviewer: Jason Lowe-Power <[email protected]>
Gerrit-Reviewer: Richard Cooper <[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
