VM: Refactor assembler test that rely on the constant pool.

runtime/vm/unit_test.h

Index: runtime/vm/unit_test.h
diff --git a/runtime/vm/unit_test.h b/runtime/vm/unit_test.h
index 01ba00926e5dbba4eb3a75a2e2a04ec5ed2c76e5..cb2feb2110393556eae03adb12d3bfd6ecee0a77 100644
--- a/runtime/vm/unit_test.h
+++ b/runtime/vm/unit_test.h
@@ -321,6 +321,26 @@ class TestIsolateScope {
 };
 
 
+template<typename T> struct is_void {
+  static const bool value = false;
+};
+
+
+template<> struct is_void<void> {
+  static const bool value = true;
+};
+
+
+template<typename T> struct is_double {
+  static const bool value = false;
+};
+
+
+template<> struct is_double<double> {
+  static const bool value = true;
+};
+
+
 class AssemblerTest {
  public:
   AssemblerTest(const char* name, Assembler* assembler)
@@ -337,6 +357,59 @@ class AssemblerTest {
   const Code& code() const { return code_; }
 
   uword entry() const { return entry_; }
+#if defined(USING_SIMULATOR)

[srdjan, 2015/08/10 16:23:51]

I am not sure I found this more readable and maintainable than the original. It
look smart though ;-).

[zra, 2015/08/10 16:43:32]

This makes writing the assembler tests a bit easier, but please add comments
about how to use Invoke, and the restrictions on the template arguments.

What does the compiler error look like if you try to use a two-argument version
of Invoke that doesn't exist? If it looks bad, maybe add a comment or TODO
somewhere explaining how to fix.

[Florian Schneider, 2015/08/11 07:58:28]

Yes, I just added the variants necessary.
The compiler would complain about not finding a methods with the signature
requested. e.g. adding the 2-template-argument variant is very straight-forward
using the same pattern.

[Florian Schneider, 2015/08/11 07:58:28]

There may be even more elegant ways to solve this >) I just needed a common
place for the actual invocation (Invoke method)

I only addressed the tests that need the constant pool, since this will change
when we change the calling convention and move the constant pool out of the
Instructions object.

+  template<typename ResultType> ResultType Invoke() {
+    const bool fp_return = is_double<ResultType>::value;
+    const bool fp_args = false;
+    return static_cast<ResultType>(Simulator::Current()->Call(
+        bit_cast<int64_t, uword>(entry()), 0, 0, 0, 0, fp_return, fp_args));
+  }

[zra, 2015/08/10 16:43:32]

Missing newline

+  template<typename ResultType, typename Arg1Type>
+  ResultType Invoke(Arg1Type arg1) {
+    const bool fp_return = is_double<ResultType>::value;
+    COMPILE_ASSERT(!is_double<Arg1Type>::value);
+    const bool fp_args = false;
+    return static_cast<ResultType>(Simulator::Current()->Call(
+        bit_cast<int64_t, uword>(entry()), reinterpret_cast<int64_t>(arg1),
+        0, 0, 0, fp_return, fp_args));
+  }

[zra, 2015/08/10 16:43:32]

Missing newline

[Florian Schneider, 2015/08/11 07:58:28]

Not between template functions

+  template<typename ResultType,
+           typename Arg1Type,
+           typename Arg2Type,
+           typename Arg3Type>
+  ResultType Invoke(Arg1Type arg1, Arg2Type arg2, Arg3Type arg3) {

[zra, 2015/08/10 16:43:32]

The first two Invokes appear to be used for assembler tests, while this one is
used for something else. Maybe give it a different name to avoid confusion.

[Florian Schneider, 2015/08/11 07:58:28]

This one is used ony for one test right now (StoreIntoObject), from
assembler_test.cc

+    COMPILE_ASSERT(is_void<ResultType>::value);
+    const bool fp_return = false;
+    COMPILE_ASSERT(!is_double<Arg1Type>::value);
+    const bool fp_args = false;
+    return static_cast<ResultType>(Simulator::Current()->Call(
+        bit_cast<int64_t, uword>(entry()),
+        reinterpret_cast<int64_t>(arg1),
+        reinterpret_cast<int64_t>(arg2),
+        reinterpret_cast<int64_t>(arg3),
+        0, fp_return, fp_args));
+  }
+#else
+  template<typename ResultType> ResultType Invoke() {
+    typedef ResultType (*FunctionType) ();
+    return reinterpret_cast<FunctionType>(entry())();
+  }
+
+  template<typename ResultType, typename Arg1Type>
+  ResultType Invoke(Arg1Type arg1) {
+    typedef ResultType (*FunctionType) (Arg1Type);

[zra, 2015/08/10 16:43:31]

Is there some reason not to have the same asserts here as in the simulated
version?

[Florian Schneider, 2015/08/11 07:58:28]

No, regular invocation just works with all types. For simulator-calls we need to
pass true/false for fp_return and fp_args accordingly. I didn't want to eagerly
add all variants, so I just added the asserts for now.

[zra, 2015/08/11 19:48:17]

I think it would be good to have consistent behavior between simulated and
non-simulated builds. My suggestion would be to have the asserts here, too.

[Florian Schneider, 2015/08/12 07:30:53]

Why add assertions that don't guard against anything? That would be even more
confusing.

Also I'd rather not want this CL to grow beyond what's I need for making
changing the calling convention as smooth as possible.

[zra, 2015/08/17 16:35:43]

The assertions guard against doing something in architecture/simulator agnostic
code, like in assembler_test.cc, that won't work on the simulator. You will find
out early that the use of Invoke won't work.

+    return reinterpret_cast<FunctionType>(entry())(arg1);
+  }
+
+  template<typename ResultType,
+           typename Arg1Type,
+           typename Arg2Type,
+           typename Arg3Type>
+  ResultType Invoke(Arg1Type arg1, Arg2Type arg2, Arg3Type arg3) {
+    typedef ResultType (*FunctionType) (Arg1Type, Arg2Type, Arg3Type);

[zra, 2015/08/10 16:43:32]

Same question.

+    return reinterpret_cast<FunctionType>(entry())(arg1, arg2, arg3);
+  }
+#endif
 
   // Assemble test and set code_ and entry_.
   void Assemble();