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

runtime/vm/unit_test.h

 // Copyright (c) 2011, the Dart project authors.  Please see the AUTHORS file
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.
 
 #ifndef VM_UNIT_TEST_H_
 #define VM_UNIT_TEST_H_
 
 #include "include/dart_native_api.h"
 
 #include "platform/globals.h"
@@ skipping 303 matching lines @@
   }
   Isolate* isolate() const { return isolate_; }
 
  private:
   Isolate* isolate_;
 
   DISALLOW_COPY_AND_ASSIGN(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)
       : name_(name),
         assembler_(assembler),
         code_(Code::ZoneHandle()) {
     ASSERT(name != NULL);
     ASSERT(assembler != NULL);
   }
   ~AssemblerTest() { }
 
   Assembler* assembler() const { return assembler_; }
 
   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();
 
  private:
   const char* name_;
   Assembler* assembler_;
   Code& code_;
   uword entry_;
 
@@ skipping 74 matching lines @@
       }                                                                        \
     } else {                                                                   \
       dart::Expect(__FILE__, __LINE__).Fail("expected True, but was '%s'\n",   \
           #handle);                                                            \
     }                                                                          \
   } while (0)
 
 }  // namespace dart
 
 #endif  // VM_UNIT_TEST_H_