[interpreter] Ensure optimizations preserve source positions.

test/cctest/interpreter/test-source-positions.cc

+// Copyright 2015 the V8 project authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "src/v8.h"
+
+#include "src/compiler/pipeline.h"
+#include "src/handles.h"
+#include "src/interpreter/bytecode-generator.h"
+#include "src/interpreter/interpreter.h"
+#include "src/isolate.h"
+#include "src/parsing/parser.h"
+#include "test/cctest/cctest.h"
+#include "test/cctest/interpreter/source-position-matcher.h"
+
+namespace v8 {
+namespace internal {
+namespace interpreter {
+
+struct BytecodeFrameInfo {
+  explicit BytecodeFrameInfo(int locals_count = -1)
+      : number_of_locals(locals_count) {}
+
+  int number_of_locals;
+};
+
+class OptimizedBytecodeSourcePositionTester final {
+ public:
+  explicit OptimizedBytecodeSourcePositionTester(Isolate* isolate)
+      : isolate_(isolate) {
+    optimization_flags_ = {&FLAG_ignition_peephole, &FLAG_ignition_reo};
+    SaveFlags();
+  }
+  ~OptimizedBytecodeSourcePositionTester() { RestoreFlags(); }
+
+  bool SourcePositionsMatch(const char* function_body,
+                            const char* function_decl_params = "",
+                            const char* function_args = "");
+
+ private:
+  Handle<BytecodeArray> MakeBytecode(size_t optimization_flags,
+                                     const char* function_body,
+                                     const char* function_decl_params,
+                                     const char* function_args,
+                                     BytecodeFrameInfo* frame_info);
+  static std::string MakeFunctionName(size_t flags);
+  static std::string MakeScript(const char* function_name,
+                                const char* function_body,
+                                const char* function_decl_params,
+                                const char* function_args);
+
+  void SetOptimizationFlags(size_t bitmask);
+  void SaveFlags();
+  void RestoreFlags();
+
+  Isolate* isolate() const { return isolate_; }
+
+  Isolate* isolate_;
+  std::vector<bool*> optimization_flags_;
+  std::vector<bool> saved_flags_;
+};
+
+// static
+std::string OptimizedBytecodeSourcePositionTester::MakeFunctionName(
+    size_t flags) {
+  std::ostringstream os;
+  os << "test_function_" << flags;
+  return os.str();
+}
+
+// static
+std::string OptimizedBytecodeSourcePositionTester::MakeScript(
+    const char* function_name, const char* function_body,
+    const char* function_decl_params, const char* function_args) {
+  std::ostringstream os;
+  os << "function " << function_name << "(" << function_decl_params << ") {";
+  os << function_body;
+  os << "}";
+  os << function_name << "(" << function_args << ");";
+  return os.str();
+}
+
+Handle<BytecodeArray> OptimizedBytecodeSourcePositionTester::MakeBytecode(
+    size_t flags, const char* function_body, const char* function_decl_params,
+    const char* function_args, BytecodeFrameInfo* frame_info) {
+  std::string function_name = MakeFunctionName(flags);
+  std::string script = MakeScript(function_name.c_str(), function_body,
+                                  function_decl_params, function_args);
+  SetOptimizationFlags(flags);
+  CompileRun(script.c_str());
+
+  // Get number of locals and store in frame_info.
+  Local<Function> api_function =
+      Local<Function>::Cast(CcTest::global()
+                                ->Get(CcTest::isolate()->GetCurrentContext(),
+                                      v8_str(function_name.c_str()))
+                                .ToLocalChecked());
+  Handle<JSFunction> function =
+      Handle<JSFunction>::cast(v8::Utils::OpenHandle(*api_function));
+  frame_info->number_of_locals =
+      function->shared()->scope_info()->StackSlotCount();
+
+  return handle(function->shared()->bytecode_array());
+}
+
+void OptimizedBytecodeSourcePositionTester::SetOptimizationFlags(
+    size_t bitmask) {

[rmcilroy, 2016/06/07 09:46:11]

Could we just have bools for each of the flags or an ENUM rather than a bitmap,
I think that would be much easier to understand.

[oth, 2016/06/07 13:46:18]

The intent was adding a new optimization stage would just entail editing a
single place - the array of flags. Per comment below, looking to see if some
kind of parameterized test in google test can help.

+  size_t bit = 1;
+  for (size_t i = 0; i < optimization_flags_.size(); ++i) {
+    *optimization_flags_[i] = bitmask & bit ? true : false;
+    bit <<= 1;
+  }
+}
+
+void OptimizedBytecodeSourcePositionTester::SaveFlags() {
+  saved_flags_.resize(optimization_flags_.size());
+  for (size_t i = 0; i < optimization_flags_.size(); ++i) {
+    saved_flags_[i] = *optimization_flags_[i];
+  }
+}
+
+void OptimizedBytecodeSourcePositionTester::RestoreFlags() {
+  for (size_t i = 0; i < optimization_flags_.size(); ++i) {
+    *optimization_flags_[i] = saved_flags_[i];
+  }
+}
+
+bool OptimizedBytecodeSourcePositionTester::SourcePositionsMatch(
+    const char* function_body, const char* function_decl_params,
+    const char* function_args) {
+  BytecodeFrameInfo unoptimized_frame_info;
+  Handle<BytecodeArray> unoptimized_bytecode =
+      MakeBytecode(0, function_body, function_decl_params, function_args,
+                   &unoptimized_frame_info);
+  size_t count = static_cast<size_t>(1u) << optimization_flags_.size();
+  for (size_t i = 1; i < count; ++i) {

[rmcilroy, 2016/06/07 09:46:11]

I'd prefer we had seperate tests for each optimization level. E.g.,

TEST(SourcePositionsEquivalentPeepHole) {
  SourcePositionsEquivalent(true, false);
}

TEST(SourcePositionsEquivalentReoPeepHole) {
  SourcePositionsEquivalent(true, true);
}

To make it obvious if one fails which optimization stage is at fault.

[oth, 2016/06/07 13:46:18]

I'm not sure this scales. The code currently test all combinations of relevant
flags. There's at least two more flags in the pipeline meaning 15 TEST(xxx) to
get full coverage versus one test that gets everything.

I'm looking around to see if there's a better way to do this in googletest.

+    BytecodeFrameInfo optimized_frame_info;
+    Handle<BytecodeArray> optimized_bytecode =
+        MakeBytecode(i, function_body, function_decl_params, function_args,
+                     &optimized_frame_info);
+    SourcePositionMatcher matcher(unoptimized_frame_info.number_of_locals);
+    if (!matcher.Match(unoptimized_bytecode, optimized_bytecode)) {
+      printf("Unoptimized:\n");

[rmcilroy, 2016/06/07 09:46:11]

remove logging

[oth, 2016/06/07 13:46:18]

Done.

+      unoptimized_bytecode->Print();
+      printf("Optimized:\n");
+      optimized_bytecode->Print();
+      return false;
+    }
+  }
+  return true;
+}
+
+class ScopedFlag {
+ public:
+  explicit ScopedFlag(bool* flag) : flag_(flag), saved_value_(flag) {}
+  ~ScopedFlag() { *flag_ = saved_value_; }
+
+  ScopedFlag& operator=(bool value) {
+    *flag_ = value;
+    return *this;
+  }
+
+ private:
+  bool* flag_;
+  bool saved_value_;
+
+  DISALLOW_COPY_AND_ASSIGN(ScopedFlag);
+};
+
+TEST(SourcePositionsEquivalent) {
+  HandleAndZoneScope handles;
+
+  ScopedFlag always_opt_flag(&FLAG_always_opt);
+  always_opt_flag = false;
+
+  ScopedFlag ignition_flag(&FLAG_ignition);
+  ignition_flag = true;

[rmcilroy, 2016/06/07 09:46:11]

nit - You could set these flags (and initialize the interpreter) in
OptimizedBytecodeSourcePositionTester

[oth, 2016/06/07 13:46:18]

Done.

+
+  // Ensure handler table is generated.
+  handles.main_isolate()->interpreter()->Initialize();
+
+  const char* test_scripts[] = {
+      "var x = (y = 3) + (x = y); return x + y;",
+
+      "var x = 55;\n"
+      "var y = x + (x = 1) + (x = 2) + (x = 3);\n"
+      "return y;",
+
+      "var x = 10; return x >>> 3;",
+
+      "var x = 0; return x || (1, 2, 3);",
+
+      "return a || (a, b, a, b, c = 5, 3); ",
+
+      "var a = 3; var b = 4; a = b; b = a; a = b; return a;",
+
+      "var a = 1; return [[a, 2], [a + 2]];",
+
+      "var a = 1; if (a || a < 0) { return 1; }",
+
+      "var b;"
+      "b = a.name;"
+      "b = a.name;"
+      "a.name = a;"
+      "b = a.name;"
+      "a.name = a;"
+      "return b;",
+
+      "var sum = 0;\n"
+      "outer: {\n"
+      "  for (var x = 0; x < 10; ++x) {\n"
+      "    for (var y = 0; y < 3; ++y) {\n"
+      "      ++sum;\n"
+      "      if (x + y == 12) { break outer; }\n"
+      "    }\n"
+      "  }\n"
+      "}\n"
+      "return sum;\n",
+
+      "var a = 1;"
+      "switch (a) {"
+      "  case 1: return a * a + 1;"
+      "  case 1: break;"
+      "  case 2: return (a = 3) * a + (a = 4);"
+      "  case 3:"
+      "}"
+      "return a;",
+
+      "for (var p of [0, 1, 2]) {}",
+
+      "var x = { 'a': 1, 'b': 2 };"
+      "for (x['a'] of [1,2,3]) { return x['a']; }",
+
+      "while (x == 4) {\n"
+      "  var y = x + 1;\n"
+      "  if (y == 2) break;\n"
+      "  for (z['a'] of [0]) {\n"
+      "    x += (x *= 3) + y;"
+      "  }\n"
+      "}\n",
+
+      "function g(a, b) { return a.func(b + b, b); }\n"
+      "g(new (function Obj() { this.func = function() { return; }})(), 1)\n"};
+
+  OptimizedBytecodeSourcePositionTester tester(handles.main_isolate());
+  for (size_t i = 0; i < arraysize(test_scripts); ++i) {
+    CHECK(tester.SourcePositionsMatch(test_scripts[i]));
+  }
+}
+
+}  // namespace interpreter
+}  // namespace internal
+}  // namespace v8