[Interpreter] Adds implementation of bytecode graph builder for LoadICSloppy/Strict

test/unittests/compiler/bytecode-graph-builder-unittest.cc

Index: test/unittests/compiler/bytecode-graph-builder-unittest.cc
diff --git a/test/unittests/compiler/bytecode-graph-builder-unittest.cc b/test/unittests/compiler/bytecode-graph-builder-unittest.cc
index 27ff4ca359a25a327d6518f192d780fffb164927..273650a8151afc7f5266d184cca24acca29466e3 100644
--- a/test/unittests/compiler/bytecode-graph-builder-unittest.cc
+++ b/test/unittests/compiler/bytecode-graph-builder-unittest.cc
@@ -11,7 +11,9 @@
 #include "src/compiler/instruction-selector.h"
 #include "src/compiler/js-graph.h"
 #include "src/compiler/js-operator.h"
+#include "src/compiler/linkage.h"
 #include "src/interpreter/bytecode-array-builder.h"
+#include "src/interpreter/interpreter.h"
 #include "src/parser.h"
 #include "test/unittests/compiler/compiler-test-utils.h"
 #include "test/unittests/compiler/graph-unittest.h"
@@ -24,6 +26,125 @@ namespace v8 {
 namespace internal {
 namespace compiler {
 
+#define SPACE()
+
+#define REPEAT_2(SEP, ...) __VA_ARGS__ SEP() __VA_ARGS__
+#define REPEAT_4(SEP, ...) \
+  REPEAT_2(SEP, __VA_ARGS__) SEP() REPEAT_2(SEP, __VA_ARGS__)
+#define REPEAT_8(SEP, ...) \
+  REPEAT_4(SEP, __VA_ARGS__) SEP() REPEAT_4(SEP, __VA_ARGS__)
+#define REPEAT_16(SEP, ...) \
+  REPEAT_8(SEP, __VA_ARGS__) SEP() REPEAT_8(SEP, __VA_ARGS__)
+#define REPEAT_32(SEP, ...) \
+  REPEAT_16(SEP, __VA_ARGS__) SEP() REPEAT_16(SEP, __VA_ARGS__)
+#define REPEAT_64(SEP, ...) \
+  REPEAT_32(SEP, __VA_ARGS__) SEP() REPEAT_32(SEP, __VA_ARGS__)
+#define REPEAT_128(SEP, ...) \
+  REPEAT_64(SEP, __VA_ARGS__) SEP() REPEAT_64(SEP, __VA_ARGS__)
+#define REPEAT_256(SEP, ...) \
+  REPEAT_128(SEP, __VA_ARGS__) SEP() REPEAT_128(SEP, __VA_ARGS__)
+
+#define REPEAT_127(SEP, ...)  \
+  REPEAT_64(SEP, __VA_ARGS__) \
+  SEP()                       \
+  REPEAT_32(SEP, __VA_ARGS__) \
+  SEP()                       \
+  REPEAT_16(SEP, __VA_ARGS__) \
+  SEP()                       \
+  REPEAT_8(SEP, __VA_ARGS__)  \
+  SEP()                       \
+  REPEAT_4(SEP, __VA_ARGS__) SEP() REPEAT_2(SEP, __VA_ARGS__) SEP() __VA_ARGS__
+
+
+class GraphGeneratorHelper {

[rmcilroy, 2015/11/10 11:23:19]

As discussed offline, pull the BytecodeArray code into this helper in order to
share code (like InterpreterTester).

[mythria, 2015/11/10 17:02:51]

Done.

+ public:
+  const char* kFunctionName = "f";
+
+  GraphGeneratorHelper(Isolate* isolate, Zone* zone)
+      : isolate_(isolate), zone_(zone) {
+    i::FLAG_ignition = true;
+    i::FLAG_always_opt = false;
+    i::FLAG_vector_stores = true;
+    // Set ignition filter flag via SetFlagsFromString to avoid double-free
+    // (or potential leak with StrDup() based on ownership confusion).
+    ScopedVector<char> ignition_filter(64);
+    SNPrintF(ignition_filter, "--ignition-filter=%s", kFunctionName);
+    FlagList::SetFlagsFromString(ignition_filter.start(),
+                                 ignition_filter.length());
+    // Ensure handler table is generated.
+    isolate_->interpreter()->Initialize();
+    v8::Context::New(v8::Isolate::GetCurrent())->Enter();

[rmcilroy, 2015/11/10 11:23:19]

nit - move this down to MakeGraph I think (you also need to exit the context -
either in the destructor if you enter it here, or at the end of MakeGraph if you
enter it there.

[mythria, 2015/11/10 17:02:51]

Done.

+  }
+
+  Graph* MakeGraph(const char* script, const char* function_name) {

[rmcilroy, 2015/11/10 11:23:19]

nit - private

[mythria, 2015/11/10 17:02:51]

I removed this function after merging graph generation from bytecode into this
helper.

+    v8::Local<v8::Context> context =
+        v8::Isolate::GetCurrent()->GetCurrentContext();
+    v8::Local<v8::String> v8_function_name =
+        v8::String::NewFromUtf8(v8::Isolate::GetCurrent(), function_name,
+                                v8::NewStringType::kNormal)
+            .ToLocalChecked();
+
+    CompileRun(script);
+
+    Local<Function> api_function = Local<Function>::Cast(
+        context->Global()->Get(context, v8_function_name).ToLocalChecked());
+    Handle<JSFunction> function =
+        Handle<JSFunction>::cast(v8::Utils::OpenHandle(*api_function));
+    CHECK(function->shared()->HasBytecodeArray());
+
+    ParseInfo parse_info(zone_, function);
+    CompilationInfo compilation_info(&parse_info);
+
+    MachineOperatorBuilder* machine = new (zone_) MachineOperatorBuilder(
+        zone_, kMachPtr, InstructionSelector::SupportedMachineOperatorFlags());
+    CommonOperatorBuilder* common = new (zone_) CommonOperatorBuilder(zone_);
+    JSOperatorBuilder* javascript = new (zone_) JSOperatorBuilder(zone_);
+    Graph* graph = new (zone_) Graph(zone_);
+    JSGraph* jsgraph = new (zone_)
+        JSGraph(isolate_, graph, common, javascript, nullptr, machine);
+
+    BytecodeGraphBuilder graph_builder(zone_, &compilation_info, jsgraph);
+    graph_builder.CreateGraph();
+    return graph;
+  }
+
+  Graph* MakeGraphForFunctionBody(const char* body) {
+    ScopedVector<char> program(3072);
+    SNPrintF(program, "function %s() { %s }\n%s();", kFunctionName, body,
+             kFunctionName);
+    return MakeGraph(program.start(), kFunctionName);
+  }
+
+  Graph* MakeGraphForFunction(const char* function) {
+    ScopedVector<char> program(3072);
+    SNPrintF(program, "%s\n%s();", function, kFunctionName);
+    return MakeGraph(program.start(), kFunctionName);
+  }
+
+  Graph* MakeGraphForScript(const char* script) {
+    ScopedVector<char> program(3072);
+    SNPrintF(program, "%s", script);
+    return MakeGraph(program.start(), kFunctionName);
+  }
+
+ private:
+  Isolate* isolate_;
+  Zone* zone_;
+
+  static Local<Value> CompileRun(const char* script) {
+    v8::Local<v8::String> v8_script =
+        v8::String::NewFromUtf8(v8::Isolate::GetCurrent(), script,
+                                v8::NewStringType::kNormal)
+            .ToLocalChecked();
+    v8::Local<v8::Context> context =
+        v8::Isolate::GetCurrent()->GetCurrentContext();
+    v8::Local<v8::Script> compiled_script =
+        (v8::Script::Compile(context, v8_script).ToLocalChecked());
+    return compiled_script->Run(context).ToLocalChecked();
+  }
+};
+
+
 class BytecodeGraphBuilderTest : public TestWithIsolateAndZone {
  public:
   BytecodeGraphBuilderTest() : array_builder_(isolate(), zone()) {}
@@ -35,6 +156,8 @@ class BytecodeGraphBuilderTest : public TestWithIsolateAndZone {
   Matcher<Node*> IsTheHoleConstant();
   Matcher<Node*> IsFalseConstant();
   Matcher<Node*> IsTrueConstant();
+  Matcher<Node*> IsIntPtrConstant(int value);
+  Matcher<Node*> IsFeedbackVector(Node* effect, Node* control);
 
   interpreter::BytecodeArrayBuilder* array_builder() { return &array_builder_; }
 
@@ -96,6 +219,28 @@ Matcher<Node*> BytecodeGraphBuilderTest::IsTrueConstant() {
 }
 
 
+Matcher<Node*> BytecodeGraphBuilderTest::IsIntPtrConstant(int value) {
+  if (kPointerSize == 8) {
+    return IsInt64Constant(value);
+  } else {
+    return IsInt32Constant(value);
+  }
+}
+
+
+Matcher<Node*> BytecodeGraphBuilderTest::IsFeedbackVector(Node* effect,
+                                                          Node* control) {
+  int offset = SharedFunctionInfo::kFeedbackVectorOffset - kHeapObjectTag;
+  int offset1 = JSFunction::kSharedFunctionInfoOffset - kHeapObjectTag;
+
+  return IsLoad(kMachAnyTagged,
+                IsLoad(kMachAnyTagged,
+                       IsParameter(Linkage::kJSFunctionCallClosureParamIndex),
+                       IsIntPtrConstant(offset1), effect, control),
+                IsIntPtrConstant(offset), effect, control);
+}
+
+
 TEST_F(BytecodeGraphBuilderTest, ReturnUndefined) {
   array_builder()->set_locals_count(0);
   array_builder()->set_context_count(0);
@@ -254,6 +399,83 @@ TEST_F(BytecodeGraphBuilderTest, SimpleExpressionWithRegister) {
                     _, _));
 }
 
+
+TEST_F(BytecodeGraphBuilderTest, NamedLoadSloppy) {
+  GraphGeneratorHelper helper(isolate(), zone());
+  const char* code_snippet = "function f(p1) {return p1.val;}; f({val:10});";
+  Graph* graph = helper.MakeGraphForScript(code_snippet);
+
+  Node* ret = graph->end()->InputAt(0);
+  Node* effect = graph->start();
+  Node* control = graph->start();
+  Handle<Name> name = StringTable::LookupString(
+      isolate(), factory()->NewStringFromStaticChars("val"));
+
+  EXPECT_THAT(ret, IsReturn(IsJSLoadNamed(name, IsParameter(1),

[rmcilroy, 2015/11/10 11:23:19]

As discussed offline, pull this out into matchers to make it easier to read.

[mythria, 2015/11/10 17:02:51]

Done.

+                                          IsFeedbackVector(effect, control),
+                                          effect, control),
+                            _, _));
+}
+
+
+TEST_F(BytecodeGraphBuilderTest, NamedLoadStrict) {
+  GraphGeneratorHelper helper(isolate(), zone());
+  const char* code_snippet =
+      "function f(p1) {'use strict'; return p1.val;}; f({val:10});";
+  Graph* graph = helper.MakeGraphForScript(code_snippet);
+
+  Node* ret = graph->end()->InputAt(0);
+  Node* effect = graph->start();
+  Node* control = graph->start();
+  Handle<Name> name = StringTable::LookupString(
+      isolate(), factory()->NewStringFromStaticChars("val"));
+
+  EXPECT_THAT(ret, IsReturn(IsJSLoadNamed(name, IsParameter(1),
+                                          IsFeedbackVector(effect, control),
+                                          effect, control),
+                            _, _));
+}
+
+
+TEST_F(BytecodeGraphBuilderTest, NamedLoadSloppyWide) {
+  GraphGeneratorHelper helper(isolate(), zone());
+  const char code_snippet[] = "function f(p1) {var b; "
+                              REPEAT_127(SPACE, " b = p1.val; ")
+                              "return p1.val;}; f({val:10});";

[rmcilroy, 2015/11/10 11:23:19]

As discussed offline, let's make the repeated and wide property loads take
different names to make it obvious which one we are matching on.

[mythria, 2015/11/10 17:02:51]

Done.

+  Graph* graph = helper.MakeGraphForScript(code_snippet);
+
+  Node* ret = graph->end()->InputAt(0);
+  Node* start = graph->start();
+  Handle<Name> name = StringTable::LookupString(
+      isolate(), factory()->NewStringFromStaticChars("val"));

[rmcilroy, 2015/11/10 11:23:19]

nit - make a static helper for this in GraphGeneratorHelper::GetName (same as
InterpreterTester::GetName()).

[mythria, 2015/11/10 17:02:51]

Done.

+
+  EXPECT_THAT(ret,
+              IsReturn(IsJSLoadNamed(
+                           name, IsParameter(1), IsFeedbackVector(start, start),
+                           IsJSLoadNamed(name, _, _, _, _), IsIfSuccess(_)),
+                       _, _));
+}
+
+
+TEST_F(BytecodeGraphBuilderTest, NamedLoadStrictWide) {
+  GraphGeneratorHelper helper(isolate(), zone());
+  const char code_snippet[] = "function f(p1) {'use strict'; var b;"
+                              REPEAT_127(SPACE, " b = p1.val; ")
+                              "return p1.val;}; f({val:10});";
+  Graph* graph = helper.MakeGraphForScript(code_snippet);
+
+  Node* ret = graph->end()->InputAt(0);
+  Node* start = graph->start();
+  Handle<Name> name = StringTable::LookupString(
+      isolate(), factory()->NewStringFromStaticChars("val"));
+
+  EXPECT_THAT(ret,
+              IsReturn(IsJSLoadNamed(
+                           name, IsParameter(1), IsFeedbackVector(start, start),
+                           IsJSLoadNamed(name, _, _, _, _), IsIfSuccess(_)),
+                       _, _));
+}
+
 }  // namespace compiler
 }  // namespace internal
 }  // namespace v8