[gem5-dev] Change in gem5/gem5[develop]: sim: Simplify some code in the guest ABI mechanism.
Gabe Black has submitted this change. (
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
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/41600
Reviewed-by: Gabe Black
Maintainer: Gabe Black
Tested-by: kokoro
---
M src/sim/guest_abi.hh
M src/sim/guest_abi/dispatch.hh
M src/sim/guest_abi/layout.hh
3 files changed, 48 insertions(+), 112 deletions(-)
Approvals:
Gabe Black: Looks good to me, approved; Looks good to me, approved
kokoro: Regressions pass
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(tc);
GuestABI::prepareForFunction(tc, state);
-return GuestABI::callFrom(tc, state,
target);
+return GuestABI::callFrom(tc, state,
target);
}
template
@@ -86,7 +86,7 @@
// types will be zero initialized.
auto state = GuestABI::initializeState(tc);
GuestABI::prepareForArguments(tc, state);
-GuestABI::callFrom(tc, state, target);
+GuestABI::callFrom(tc, state, target);
}
template
@@ -113,7 +113,7 @@
GuestABI::prepareForFunction(tc, state);
ss << name;
-GuestABI::dumpArgsFrom(0, ss, tc, state);
+GuestABI::dumpArgsFrom(ss, tc, state);
return ss.str();
}
diff --git a/src/sim/guest_abi/dispatch.hh b/src/sim/guest_abi/dispatch.hh
index bc365b9..8f3a4ac 100644
--- a/src/sim/guest_abi/dispatch.hh
+++ b/src/sim/guest_abi/dispatch.hh
@@ -30,8 +30,11 @@
#include
#include
+#include
#include
+#include
+#include "base/compiler.hh"
#include "sim/guest_abi/definition.hh"
#include "sim/guest_abi/layout.hh"
@@ -50,114 +53,60 @@
* still possible to support by redefining these functions..
*/
-// With no arguments to gather, call the target function and store the
-// result.
-template
-static typename std::enable_if_t::value && store_ret,
Ret>
-callFrom(ThreadContext *tc, typename ABI::State &state,
-std::function target)
+template
+static inline typename std::enable_if_t
+callFromHelper(Target &target, ThreadContext *tc, State &state, Args
&&args,
+std::index_sequence)
{
-Ret ret = target(tc);
+return target(tc, std::get(args)...);
+}
+
+template
+static inline typename std::enable_if_t
+callFromHelper(Target &target, ThreadContext *tc, State &state, Args
&&args,
+std::index_sequence)
+{
+Ret ret = target(tc, std::get(args)...);
storeResult(tc, ret, state);
return ret;
}
-template
-static typename std::enable_if_t::value && !store_ret,
Ret>
+template
+static inline Ret
callFrom(ThreadContext *tc, typename ABI::State &state,
-std::function target)
+std::function target)
{
-return target(tc);
+// Extract all the arguments from the thread context. Braced
initializers
+// are evaluated from left to right.
+auto args = std::tuple{getArgument(tc, state)...};
+
+// Call the wrapper which will call target.
+return callFromHelper(
+target, tc, state, std::move(args),
+std::make_index_sequence{});
}
-// With no arguments to gather and nothing to return, call the target
function.
-template
-static void
-callFrom(ThreadContext *tc, typename ABI::State &state,
-std::function target)
-{
-target(tc);
-}
-
-// Recursively gather arguments for target from tc until we get to the base
-// case above.
-template
-static typename std::enable_if_t::value, Ret>
-callFrom(ThreadContext *tc, typename ABI::State &state,
-std::function target)
-{
-// Extract the next argument from the thread cont
[gem5-dev] Change in gem5/gem5[develop]: sim: Simplify some code in the guest ABI mechanism.
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(tc);
GuestABI::prepareForFunction(tc, state);
-return GuestABI::callFrom(tc, state,
target);
+return GuestABI::callFrom(tc, state,
target);
}
template
@@ -86,7 +86,7 @@
// types will be zero initialized.
auto state = GuestABI::initializeState(tc);
GuestABI::prepareForArguments(tc, state);
-GuestABI::callFrom(tc, state, target);
+GuestABI::callFrom(tc, state, target);
}
template
@@ -113,7 +113,7 @@
GuestABI::prepareForFunction(tc, state);
ss << name;
-GuestABI::dumpArgsFrom(0, ss, tc, state);
+GuestABI::dumpArgsFrom(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
#include
+#include
#include
+#include
+#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
-static typename std::enable_if_t::value && store_ret,
Ret>
-callFrom(ThreadContext *tc, typename ABI::State &state,
-std::function target)
+template
+static inline typename std::enable_if_t
+callFromHelper(Target &target, ThreadContext *tc, State &state, Args
&&args,
+std::index_sequence)
{
-Ret ret = target(tc);
+return target(tc, std::get(args)...);
+}
+
+template
+static inline typename std::enable_if_t
+callFromHelper(Target &target, ThreadContext *tc, State &state, Args
&&args,
+std::index_sequence)
+{
+Ret ret = target(tc, std::get(args)...);
storeResult(tc, ret, state);
return ret;
}
-template
-static typename std::enable_if_t::value && !store_ret,
Ret>
+template
+static inline Ret
callFrom(ThreadContext *tc, typename ABI::State &state,
-std::function target)
+std::function target)
{
-return target(tc);
+// Extract all the arguments from the thread context. Braced
initializers
+// are evaluated from left to right.
+auto args = std::tuple{getArgument(tc, state)...};
+
+// Call the wrapper which will call target.
+return callFromHelper(
+target, tc, state, std::move(args),
+std::make_index_sequence{});
}
-// With no arguments to gather and nothing to return, call the target
function.
-template
-static void
-callFrom(ThreadContext *tc, typename ABI::State &state,
-std::function target)
-{
-target(tc);
-}
-
-// Recursively gather arguments for target from tc until we get to the base
-// case above.
-template
-static typename std::enable_if_t::value, Ret>
-callFrom(ThreadContext *tc, typename ABI::State &state,
-std::function target)
-{
-// Extract the next argument from the thread context.
-NextArg next = getArgument(tc, state);
-
-// Build a partial function which adds the next argument to the call.
-std::function partial =
-[target,next](ThreadContext *_tc, Args... args) {
-return tar
