Gabe Black has uploaded this change for review. ( 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
---
M src/sim/serialize.hh
1 file changed, 113 insertions(+), 124 deletions(-)



diff --git a/src/sim/serialize.hh b/src/sim/serialize.hh
index 1a1193b..3d1ce23 100644
--- a/src/sim/serialize.hh
+++ b/src/sim/serialize.hh
@@ -320,139 +320,126 @@

 /**
  * @ingroup api_serialize
+ * @{
  */
+
+/*
+ * 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 +453,7 @@
 paramOut(CheckpointOut &os, const std::string &name, const T &param)
 {
     os << name << "=";
-    showParam(os, param);
+    ShowParam<T>::show(os, param);
     os << "\n";
 }

@@ -489,7 +476,7 @@
 {
     const std::string &section(Serializable::currentSection());
     std::string str;
-    if (!cp.find(section, name, str) || !parseParam(str, param)) {
+ if (!cp.find(section, name, str) || !ParseParam<T>::parse(str, param)) {
         if (warn)
             warn("optional parameter %s:%s not present\n", section, name);
         return false;
@@ -523,11 +510,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";
 }
@@ -587,7 +575,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: 1
Gerrit-Owner: Gabe Black <[email protected]>
Gerrit-MessageType: newchange
_______________________________________________
gem5-dev mailing list -- [email protected]
To unsubscribe send an email to [email protected]
%(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s

Reply via email to