Gabe Black has uploaded this change for review. ( https://gem5-review.googlesource.com/c/public/gem5/+/41600 )

Change subject: sim: Simplify some code in the guest ABI mechanism.
......................................................................

sim: Simplify some code in the guest ABI mechanism.

Instead of using recursively applied templates to accumulate a series of
wrapper lambdas which dispatch to a call, use pure parameter pack
expansion. This has two benefits. One, it makes the code simpler(ish) and
easier to understand. The parameter pack machinery is still intrinsically
fairly tricky, but there's less of it and it's a fairly straightforward
application of that mechanism.

Also, a nice side benefit is that the template for simcall dispatch will
expand to a small fixed number of functions which do all their work
locally, instead of having a new function for each layer of the onion,
one per parameter, and no calls through lambdas. That should hopefully
make debugging easier, and produce less bookkeeping overhead as far as
really long names, lots of functions, etc.

This code, specifically the code in dispatch.hh, can be simplified even
further in the future once we start using c++17 which is if constexpr,
and std::apply which explodes a tuple and uses its components as
arguments to a function, something I'm doing manually here.

Change-Id: If7c9234cc1014101211474c2ec20362702cf78c2
---
M src/sim/guest_abi.hh
M src/sim/guest_abi/dispatch.hh
M src/sim/guest_abi/layout.hh
3 files changed, 51 insertions(+), 112 deletions(-)



diff --git a/src/sim/guest_abi.hh b/src/sim/guest_abi.hh
index ea3325f..75c4e00 100644
--- a/src/sim/guest_abi.hh
+++ b/src/sim/guest_abi.hh
@@ -51,7 +51,7 @@
     // types will be zero initialized.
     auto state = GuestABI::initializeState<ABI>(tc);
     GuestABI::prepareForFunction<ABI, Ret, Args...>(tc, state);
- return GuestABI::callFrom<ABI, store_ret, Ret, Args...>(tc, state, target); + return GuestABI::callFrom<ABI, Ret, store_ret, Args...>(tc, state, target);
 }

 template <typename ABI, typename Ret, typename ...Args>
@@ -86,7 +86,7 @@
     // types will be zero initialized.
     auto state = GuestABI::initializeState<ABI>(tc);
     GuestABI::prepareForArguments<ABI, Args...>(tc, state);
-    GuestABI::callFrom<ABI, Args...>(tc, state, target);
+    GuestABI::callFrom<ABI, void, false, Args...>(tc, state, target);
 }

 template <typename ABI, typename ...Args>
@@ -113,7 +113,7 @@

     GuestABI::prepareForFunction<ABI, Ret, Args...>(tc, state);
     ss << name;
-    GuestABI::dumpArgsFrom<ABI, Ret, Args...>(0, ss, tc, state);
+    GuestABI::dumpArgsFrom<ABI, Ret, Args...>(ss, tc, state);
     return ss.str();
 }

diff --git a/src/sim/guest_abi/dispatch.hh b/src/sim/guest_abi/dispatch.hh
index bc365b9..93480db 100644
--- a/src/sim/guest_abi/dispatch.hh
+++ b/src/sim/guest_abi/dispatch.hh
@@ -30,8 +30,11 @@

 #include <functional>
 #include <sstream>
+#include <tuple>
 #include <type_traits>
+#include <utility>

+#include "base/compiler.hh"
 #include "sim/guest_abi/definition.hh"
 #include "sim/guest_abi/layout.hh"

@@ -50,114 +53,63 @@
  * still possible to support by redefining these functions..
  */

-// With no arguments to gather, call the target function and store the
-// result.
-template <typename ABI, bool store_ret, typename Ret>
-static typename std::enable_if_t<!std::is_void<Ret>::value && store_ret, Ret>
-callFrom(ThreadContext *tc, typename ABI::State &state,
-        std::function<Ret(ThreadContext *)> target)
+template <typename ABI, typename Ret, bool store_ret, typename Target,
+         typename State, typename Args, std::size_t... I>
+static inline typename std::enable_if_t<!store_ret, Ret>
+callFromHelper(Target &target, ThreadContext *tc, State &state, Args &&args,
+        std::index_sequence<I...>)
 {
-    Ret ret = target(tc);
+    return target(tc, std::get<I>(args)...);
+}
+
+template <typename ABI, typename Ret, bool store_ret, typename Target,
+         typename State, typename Args, std::size_t... I>
+static inline typename std::enable_if_t<store_ret, Ret>
+callFromHelper(Target &target, ThreadContext *tc, State &state, Args &&args,
+        std::index_sequence<I...>)
+{
+    Ret ret = target(tc, std::get<I>(args)...);
     storeResult<ABI, Ret>(tc, ret, state);
     return ret;
 }

-template <typename ABI, bool store_ret, typename Ret>
-static typename std::enable_if_t<!std::is_void<Ret>::value && !store_ret, Ret>
+template <typename ABI, typename Ret, bool store_ret, typename ...Args>
+static inline Ret
 callFrom(ThreadContext *tc, typename ABI::State &state,
-        std::function<Ret(ThreadContext *)> target)
+        std::function<Ret(ThreadContext *, Args...)> target)
 {
-    return target(tc);
+ // Extract all the arguments from the thread context. Braced initializers
+    // are evaluated from left to right.
+    auto args = std::tuple<Args...>{getArgument<ABI, Args>(tc, state)...};
+
+    // Call the wrapper which will call target.
+    return callFromHelper<ABI, Ret, store_ret>(
+            target, tc, state, std::move(args),
+            std::make_index_sequence<sizeof...(Args)>{});
 }

-// With no arguments to gather and nothing to return, call the target function.
-template <typename ABI>
-static void
-callFrom(ThreadContext *tc, typename ABI::State &state,
-        std::function<void(ThreadContext *)> target)
-{
-    target(tc);
-}
-
-// Recursively gather arguments for target from tc until we get to the base
-// case above.
-template <typename ABI, bool store_ret, typename Ret,
-          typename NextArg, typename ...Args>
-static typename std::enable_if_t<!std::is_void<Ret>::value, Ret>
-callFrom(ThreadContext *tc, typename ABI::State &state,
-        std::function<Ret(ThreadContext *, NextArg, Args...)> target)
-{
-    // Extract the next argument from the thread context.
-    NextArg next = getArgument<ABI, NextArg>(tc, state);
-
-    // Build a partial function which adds the next argument to the call.
-    std::function<Ret(ThreadContext *, Args...)> partial =
-        [target,next](ThreadContext *_tc, Args... args) {
-            return target(_tc, next, args...);
-        };
-
-    // Recursively handle any remaining arguments.
-    return callFrom<ABI, store_ret, Ret, Args...>(tc, state, partial);
-}
-
-// Recursively gather arguments for target from tc until we get to the base
-// case above. This version is for functions that don't return anything.
-template <typename ABI, typename NextArg, typename ...Args>
-static void
-callFrom(ThreadContext *tc, typename ABI::State &state,
-        std::function<void(ThreadContext *, NextArg, Args...)> target)
-{
-    // Extract the next argument from the thread context.
-    NextArg next = getArgument<ABI, NextArg>(tc, state);
-
-    // Build a partial function which adds the next argument to the call.
-    std::function<void(ThreadContext *, Args...)> partial =
-        [target,next](ThreadContext *_tc, Args... args) {
-            target(_tc, next, args...);
-        };
-
-    // Recursively handle any remaining arguments.
-    callFrom<ABI, Args...>(tc, state, partial);
-}
-
-

 /*
- * These functions are like the ones above, except they print the arguments
+ * This function is like the ones above, except it prints the arguments
  * a target function would be called with instead of actually calling it.
  */

-// With no arguments to print, add the closing parenthesis and return.
-template <typename ABI, typename Ret>
+template <typename ABI, typename Ret, typename ...Args>
 static void
-dumpArgsFrom(int count, std::ostream &os, ThreadContext *tc,
-             typename ABI::State &state)
+dumpArgsFrom(std::ostream &os, M5_VAR_USED ThreadContext *tc,
+        typename ABI::State &state)
 {
+    int count = 0;
+    // Extract all the arguments from the thread context and print them,
+    // prefixed with either a ( or a , as appropriate. Braced initializers
+    // are evaluated from left to right.
+    M5_VAR_USED int dummy[] = {
+        (os << (count++ ? ", " : "(") <<
+                getArgument<ABI, Args>(tc, state), 0)...};
+    // Close off the parenthesis.
     os << ")";
 }

-// Recursively gather arguments for target from tc until we get to the base
-// case above, and append those arguments to the string stream being
-// constructed.
-template <typename ABI, typename Ret, typename NextArg, typename ...Args>
-static void
-dumpArgsFrom(int count, std::ostream &os, ThreadContext *tc,
-             typename ABI::State &state)
-{
- // Either open the parenthesis or add a comma, depending on where we are
-    // in the argument list.
-    os << (count ? ", " : "(");
-
-    // Extract the next argument from the thread context.
-    NextArg next = getArgument<ABI, NextArg>(tc, state);
-
-    // Add this argument to the list.
-    os << next;
-
-    // Recursively handle any remaining arguments.
-    dumpArgsFrom<ABI, Ret, Args...>(count + 1, os, tc, state);
-}
-
 } // namespace GuestABI

 #endif // __SIM_GUEST_ABI_DISPATCH_HH__
diff --git a/src/sim/guest_abi/layout.hh b/src/sim/guest_abi/layout.hh
index bb46d62..8145bf3 100644
--- a/src/sim/guest_abi/layout.hh
+++ b/src/sim/guest_abi/layout.hh
@@ -100,29 +100,22 @@
 };

 template <typename ABI, typename Ret, typename Enabled=void>
-static void
+static inline void
 prepareForResult(ThreadContext *tc, typename ABI::State &state)
 {
     Preparer<ABI, Result, Ret>::prepare(tc, state);
 }

-template <typename ABI>
-static void
-prepareForArguments(ThreadContext *tc, typename ABI::State &state)
+template <typename ABI, typename ...Args>
+static inline void
+prepareForArguments(M5_VAR_USED ThreadContext *tc, typename ABI::State &state)
 {
-    return;
-}
-
-template <typename ABI, typename NextArg, typename ...Args>
-static void
-prepareForArguments(ThreadContext *tc, typename ABI::State &state)
-{
-    Preparer<ABI, Argument, NextArg>::prepare(tc, state);
-    prepareForArguments<ABI, Args...>(tc, state);
+    M5_VAR_USED int dummy[] = {
+        (Preparer<ABI, Argument, Args>::prepare(tc, state), 0)...};
 }

 template <typename ABI, typename Ret, typename ...Args>
-static void
+static inline void
 prepareForFunction(ThreadContext *tc, typename ABI::State &state)
 {
     prepareForResult<ABI, Ret>(tc, state);
@@ -144,12 +137,6 @@
     }
 };

-template <typename Ret, typename State>
-std::true_type foo(void (*)(ThreadContext *, const Ret &ret, State &state));
-
-template <typename Ret>
-std::false_type foo(void (*)(ThreadContext *, const Ret &ret));
-
 template <typename ABI, typename Ret>
 struct ResultStorer<ABI, Ret, typename std::enable_if_t<
std::is_same<void (*)(ThreadContext *, const Ret &, typename ABI::State &),

--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/41600
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: If7c9234cc1014101211474c2ec20362702cf78c2
Gerrit-Change-Number: 41600
Gerrit-PatchSet: 1
Gerrit-Owner: Gabe Black <gabe.bl...@gmail.com>
Gerrit-MessageType: newchange
_______________________________________________
gem5-dev mailing list -- gem5-dev@gem5.org
To unsubscribe send an email to gem5-dev-le...@gem5.org
%(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s

Reply via email to