Finalize Reactor Function creation at Routine acquisition
This enables generating multiple functions within the same scope, as
excercised by the Trampoline unit test.
Also adds a unit test for functions which are never finalized (useful
for when a code path is not supported and we want to fall back to a C++
implementation).
Also adds a unit test for the common pattern of deriving from Function<>
and using Reactor variables as members.
Bug: b/177999324
Change-Id: I269946e9c8a791eab84ff756c9c588ca8ce26337
Reviewed-on: https://swiftshader-review.googlesource.com/c/SwiftShader/+/51988
Tested-by: Nicolas Capens <nicolascapens@google.com>
Kokoro-Result: kokoro <noreply+kokoro@google.com>
Reviewed-by: Antonio Maiorano <amaiorano@google.com>
diff --git a/src/Reactor/Coroutine.hpp b/src/Reactor/Coroutine.hpp
index 3375787..a28f765 100644
--- a/src/Reactor/Coroutine.hpp
+++ b/src/Reactor/Coroutine.hpp
@@ -154,9 +154,8 @@
template<typename Return, typename... Arguments>
Coroutine<Return(Arguments...)>::Coroutine()
+ : core(new Nucleus())
{
- core.reset(new Nucleus());
-
std::vector<Type *> types = { CToReactorT<Arguments>::type()... };
for(auto type : types)
{
diff --git a/src/Reactor/Reactor.cpp b/src/Reactor/Reactor.cpp
index 0853519..3c4a146 100644
--- a/src/Reactor/Reactor.cpp
+++ b/src/Reactor/Reactor.cpp
@@ -124,7 +124,13 @@
Variable::~Variable()
{
- unmaterializedVariables->remove(this);
+ // `unmaterializedVariables` can be null at this point due to the function
+ // already having been finalized, while classes derived from `Function<>`
+ // can have member `Variable` fields which are destructed afterwards.
+ if(unmaterializedVariables)
+ {
+ unmaterializedVariables->remove(this);
+ }
}
void Variable::materialize() const
diff --git a/src/Reactor/Reactor.hpp b/src/Reactor/Reactor.hpp
index 40a596a..4871c6e 100644
--- a/src/Reactor/Reactor.hpp
+++ b/src/Reactor/Reactor.hpp
@@ -2610,8 +2610,6 @@
public:
Function();
- virtual ~Function();
-
template<int index>
Argument<typename std::tuple_element<index, std::tuple<Arguments...>>::type> Arg() const
{
@@ -2623,7 +2621,7 @@
std::shared_ptr<Routine> operator()(const Config::Edit &cfg, const char *name, ...);
protected:
- Nucleus *core;
+ std::unique_ptr<Nucleus> core;
std::vector<Type *> arguments;
};
@@ -3175,9 +3173,8 @@
template<typename Return, typename... Arguments>
Function<Return(Arguments...)>::Function()
+ : core(new Nucleus())
{
- core = new Nucleus();
-
Type *types[] = { Arguments::type()... };
for(Type *type : types)
{
@@ -3191,12 +3188,6 @@
}
template<typename Return, typename... Arguments>
-Function<Return(Arguments...)>::~Function()
-{
- delete core;
-}
-
-template<typename Return, typename... Arguments>
std::shared_ptr<Routine> Function<Return(Arguments...)>::operator()(const char *name, ...)
{
char fullName[1024 + 1];
@@ -3206,7 +3197,10 @@
vsnprintf(fullName, 1024, name, vararg);
va_end(vararg);
- return core->acquireRoutine(fullName, Config::Edit::None);
+ auto routine = core->acquireRoutine(fullName, Config::Edit::None);
+ core.reset(nullptr);
+
+ return routine;
}
template<typename Return, typename... Arguments>
@@ -3219,7 +3213,10 @@
vsnprintf(fullName, 1024, name, vararg);
va_end(vararg);
- return core->acquireRoutine(fullName, cfg);
+ auto routine = core->acquireRoutine(fullName, cfg);
+ core.reset(nullptr);
+
+ return routine;
}
template<class T, class S>
diff --git a/tests/ReactorUnitTests/ReactorUnitTests.cpp b/tests/ReactorUnitTests/ReactorUnitTests.cpp
index fb7da84..b615756 100644
--- a/tests/ReactorUnitTests/ReactorUnitTests.cpp
+++ b/tests/ReactorUnitTests/ReactorUnitTests.cpp
@@ -117,23 +117,21 @@
SecondaryGeneratorFunc secondaryGenerator = (SecondaryGeneratorFunc)generateSecondary;
using PrimaryFunc = int(int, int, int);
- RoutineT<PrimaryFunc> routine;
+
+ FunctionT<PrimaryFunc> primary;
{
- FunctionT<PrimaryFunc> primary;
- {
- Int x = primary.Arg<0>();
- Int y = primary.Arg<1>();
- Int z = primary.Arg<2>();
+ Int x = primary.Arg<0>();
+ Int y = primary.Arg<1>();
+ Int z = primary.Arg<2>();
- Pointer<Byte> secondary = Call(secondaryGenerator, z);
- Int r = Call<SecondaryFunc>(secondary, x, y);
+ Pointer<Byte> secondary = Call(secondaryGenerator, z);
+ Int r = Call<SecondaryFunc>(secondary, x, y);
- Return(r);
- }
-
- routine = primary((testName() + "_primary").c_str());
+ Return(r);
}
+ auto routine = primary((testName() + "_primary").c_str());
+
int result = routine(100, 20, -3);
EXPECT_EQ(result, 80);
}
@@ -202,6 +200,72 @@
EXPECT_EQ(result, 20);
}
+// Stopping in the middle of a `Function<>` is supported and should not affect
+// subsequent complete ones.
+TEST(ReactorUnitTests, UnfinishedFunction)
+{
+ do
+ {
+ FunctionT<int(int)> function;
+ {
+ Int a = function.Arg<0>();
+ Int z = 4;
+
+ break; // Terminate do-while early.
+
+ Return(a + z);
+ }
+ } while(true);
+
+ FunctionT<int(int)> function;
+ {
+ Int a = function.Arg<0>();
+ Int z = 4;
+
+ Return(a - z);
+ }
+
+ auto routine = function(testName().c_str());
+
+ int result = routine(16);
+ EXPECT_EQ(result, 12);
+}
+
+// Deriving from `Function<>` and using Reactor variables as members can be a
+// convenient way to 'name' function arguments and compose complex functions
+// with helper methods. This test checks the interactions between the lifetime
+// of the `Function<>` and the variables belonging to the derived class.
+struct FunctionMembers : FunctionT<int(int)>
+{
+ FunctionMembers()
+ : level(Arg<0>())
+ {
+ For(Int i = 0, i < 3, i++)
+ {
+ pourSomeMore();
+ }
+
+ Return(level);
+ }
+
+ void pourSomeMore()
+ {
+ level += 2;
+ }
+
+ Int level;
+};
+
+TEST(ReactorUnitTests, FunctionMembers)
+{
+ FunctionMembers function;
+
+ auto routine = function(testName().c_str());
+
+ int result = routine(3);
+ EXPECT_EQ(result, 9);
+}
+
TEST(ReactorUnitTests, VariableAddress)
{
FunctionT<int(int)> function;
