Reland "[Interpreter] Add CreateClosure to BytecodeGraphBuilder."

Original issue's description:
> [Interpreter] Add CreateClosure to BytecodeGraphBuilder.
>
> Adds code and tests to support CreateClosure bytecode when building
> graphs.
>
> Committed: https://crrev.com/4cceb11b0929abcbc82bf0854554a9b66003335d
> Cr-Commit-Position: refs/heads/master@{#32224}

BUG=v8:4280
LOG=N

Review URL: https://codereview.chromium.org/1475793003

Cr-Commit-Position: refs/heads/master@{#32257}

src/compiler/bytecode-graph-builder.cc

@@ -643,7 +643,20 @@ void BytecodeGraphBuilder::VisitPopContext(
 
 void BytecodeGraphBuilder::VisitCreateClosure(
     const interpreter::BytecodeArrayIterator& iterator) {
-  UNIMPLEMENTED();
+  Handle<SharedFunctionInfo> shared_info =
+      Handle<SharedFunctionInfo>::cast(iterator.GetConstantForIndexOperand(0));
+  PretenureFlag tenured =
+      iterator.GetImmediateOperand(1) ? TENURED : NOT_TENURED;
+  const Operator* op = javascript()->CreateClosure(shared_info, tenured);
+  Node* closure = NewNode(op);
+  AddEmptyFrameStateInputs(closure);
+  environment()->BindAccumulator(closure);
+}
+
+
+void BytecodeGraphBuilder::VisitCreateClosureWide(
+    const interpreter::BytecodeArrayIterator& iterator) {
+  VisitCreateClosure(iterator);
 }
 
 

src/interpreter/bytecode-array-builder.cc

@@ -444,9 +444,18 @@ BytecodeArrayBuilder& BytecodeArrayBuilder::StoreKeyedProperty(
 
 
 BytecodeArrayBuilder& BytecodeArrayBuilder::CreateClosure(
-    PretenureFlag tenured) {
+    Handle<SharedFunctionInfo> shared_info, PretenureFlag tenured) {
+  size_t entry = GetConstantPoolEntry(shared_info);
   DCHECK(FitsInImm8Operand(tenured));
-  Output(Bytecode::kCreateClosure, static_cast<uint8_t>(tenured));
+  if (FitsInIdx8Operand(entry)) {
+    Output(Bytecode::kCreateClosure, static_cast<uint8_t>(entry),
+           static_cast<uint8_t>(tenured));
+  } else if (FitsInIdx16Operand(entry)) {
+    Output(Bytecode::kCreateClosureWide, static_cast<uint16_t>(entry),
+           static_cast<uint8_t>(tenured));
+  } else {
+    UNIMPLEMENTED();
+  }
   return *this;
 }
 

src/interpreter/bytecode-array-builder.h

@@ -116,8 +116,9 @@ class BytecodeArrayBuilder {
                                            int feedback_slot,
                                            LanguageMode language_mode);
 
-  // Create a new closure for the SharedFunctionInfo in the accumulator.
-  BytecodeArrayBuilder& CreateClosure(PretenureFlag tenured);
+  // Create a new closure for the SharedFunctionInfo.
+  BytecodeArrayBuilder& CreateClosure(Handle<SharedFunctionInfo> shared_info,
+                                      PretenureFlag tenured);
 
   // Create a new arguments object in the accumulator.
   BytecodeArrayBuilder& CreateArguments(CreateArgumentsType type);

src/interpreter/bytecode-generator.cc

@@ -971,10 +971,8 @@ void BytecodeGenerator::VisitFunctionLiteral(FunctionLiteral* expr) {
   Handle<SharedFunctionInfo> shared_info =
       Compiler::GetSharedFunctionInfo(expr, info()->script(), info());
   CHECK(!shared_info.is_null());  // TODO(rmcilroy): Set stack overflow?
-
-  builder()
-      ->LoadLiteral(shared_info)
-      .CreateClosure(expr->pretenure() ? TENURED : NOT_TENURED);
+  builder()->CreateClosure(shared_info,
+                           expr->pretenure() ? TENURED : NOT_TENURED);
   execution_result()->SetResultInAccumulator();
 }
 

src/interpreter/bytecodes.h

@@ -161,7 +161,8 @@ namespace interpreter {
   V(CreateObjectLiteral, OperandType::kIdx8, OperandType::kImm8)               \
                                                                                \
   /* Closure allocation */                                                     \
-  V(CreateClosure, OperandType::kImm8)                                         \
+  V(CreateClosure, OperandType::kIdx8, OperandType::kImm8)                     \
+  V(CreateClosureWide, OperandType::kIdx16, OperandType::kImm8)                \
                                                                                \
   /* Arguments allocation */                                                   \
   V(CreateMappedArguments, OperandType::kNone)                                 \

src/interpreter/interpreter.cc

@@ -1365,15 +1365,16 @@ void Interpreter::DoCreateObjectLiteral(
 }
 
 
-// CreateClosure <tenured>
+// CreateClosure <index> <tenured>
 //
-// Creates a new closure for SharedFunctionInfo in the accumulator with the
-// PretenureFlag <tenured>.
+// Creates a new closure for SharedFunctionInfo at position |index| in the
+// constant pool and with the PretenureFlag <tenured>.
 void Interpreter::DoCreateClosure(compiler::InterpreterAssembler* assembler) {
   // TODO(rmcilroy): Possibly call FastNewClosureStub when possible instead of
   // calling into the runtime.
-  Node* shared = __ GetAccumulator();
-  Node* tenured_raw = __ BytecodeOperandImm(0);
+  Node* index = __ BytecodeOperandIdx(0);
+  Node* shared = __ LoadConstantPoolEntry(index);
+  Node* tenured_raw = __ BytecodeOperandImm(1);
   Node* tenured = __ SmiTag(tenured_raw);
   Node* result =
       __ CallRuntime(Runtime::kInterpreterNewClosure, shared, tenured);
@@ -1382,6 +1383,16 @@ void Interpreter::DoCreateClosure(compiler::InterpreterAssembler* assembler) {
 }
 
 
+// CreateClosureWide <index> <tenured>
+//
+// Creates a new closure for SharedFunctionInfo at position |index| in the
+// constant pool and with the PretenureFlag <tenured>.
+void Interpreter::DoCreateClosureWide(
+    compiler::InterpreterAssembler* assembler) {
+  return DoCreateClosure(assembler);
+}
+
+
 // CreateMappedArguments
 //
 // Creates a new mapped arguments object.

test/cctest/compiler/test-run-bytecode-graph-builder.cc

@@ -575,6 +575,45 @@ TEST(BytecodeGraphBuilderCallNew) {
 }
 
 
+TEST(BytecodeGraphBuilderCreateClosure) {
+  HandleAndZoneScope scope;
+  Isolate* isolate = scope.main_isolate();
+  Zone* zone = scope.main_zone();
+  Factory* factory = isolate->factory();
+
+  ExpectedSnippet<0> snippets[] = {
+      {"function f() {\n"
+       "  function counter() { this.count = 20; }\n"
+       "  var c = new counter();\n"
+       "  return c.count;\n"
+       "}; f()",
+       {factory->NewNumberFromInt(20)}},
+      {"function f() {\n"
+       "  function counter(arg0) { this.count = 17; this.x = arg0; }\n"
+       "  var c = new counter(6);\n"
+       "  return c.count + c.x;\n"
+       "}; f()",
+       {factory->NewNumberFromInt(23)}},
+      {"function f() {\n"
+       "  function counter(arg0, arg1) {\n"
+       "    this.count = 17; this.x = arg0; this.y = arg1;\n"
+       "  }\n"
+       "  var c = new counter(3, 5);\n"
+       "  return c.count + c.x + c.y;\n"
+       "}; f()",
+       {factory->NewNumberFromInt(25)}},
+  };
+
+  size_t num_snippets = sizeof(snippets) / sizeof(snippets[0]);
+  for (size_t i = 0; i < num_snippets; i++) {
+    BytecodeGraphTester tester(isolate, zone, snippets[i].code_snippet);
+    auto callable = tester.GetCallable<>();
+    Handle<Object> return_value = callable().ToHandleChecked();
+    CHECK(return_value->SameValue(*snippets[i].return_value()));
+  }
+}
+
+
 TEST(BytecodeGraphBuilderCallRuntime) {
   HandleAndZoneScope scope;
   Isolate* isolate = scope.main_isolate();

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

@@ -40,6 +40,12 @@ class BytecodeGraphBuilderTest : public TestWithIsolateAndZone {
  public:
   BytecodeGraphBuilderTest() {}
 
+  std::pair<Graph*, Handle<SharedFunctionInfo>> GetCompletedGraphAndSharedInfo(
+      Handle<BytecodeArray> bytecode_array,
+      MaybeHandle<TypeFeedbackVector> feedback_vector =
+          MaybeHandle<TypeFeedbackVector>(),
+      LanguageMode language_mode = LanguageMode::SLOPPY);
+
   Graph* GetCompletedGraph(Handle<BytecodeArray> bytecode_array,
                            MaybeHandle<TypeFeedbackVector> feedback_vector =
                                MaybeHandle<TypeFeedbackVector>(),
@@ -63,7 +69,8 @@ class BytecodeGraphBuilderTest : public TestWithIsolateAndZone {
 };
 
 
-Graph* BytecodeGraphBuilderTest::GetCompletedGraph(
+std::pair<Graph*, Handle<SharedFunctionInfo>>
+BytecodeGraphBuilderTest::GetCompletedGraphAndSharedInfo(
     Handle<BytecodeArray> bytecode_array,
     MaybeHandle<TypeFeedbackVector> feedback_vector,
     LanguageMode language_mode) {
@@ -91,7 +98,17 @@ Graph* BytecodeGraphBuilderTest::GetCompletedGraph(
 
   BytecodeGraphBuilder graph_builder(zone(), &info, jsgraph);
   graph_builder.CreateGraph();
-  return graph;
+  return std::make_pair(graph_builder.graph(), shared_info);
+}
+
+
+Graph* BytecodeGraphBuilderTest::GetCompletedGraph(
+    Handle<BytecodeArray> bytecode_array,
+    MaybeHandle<TypeFeedbackVector> feedback_vector,
+    LanguageMode language_mode) {
+  return GetCompletedGraphAndSharedInfo(bytecode_array, feedback_vector,
+                                        language_mode)
+      .first;
 }
 
 
@@ -807,6 +824,39 @@ TEST_F(BytecodeGraphBuilderTest, New) {
   EXPECT_THAT(ret, IsReturn(call_construct, call_construct, IsIfSuccess(_)));
 }
 
+
+TEST_F(BytecodeGraphBuilderTest, CreateClosure) {
+  PretenureFlag kPretenureFlags[] = {NOT_TENURED, TENURED};
+  TRACED_FOREACH(PretenureFlag, pretenure_flag, kPretenureFlags) {
+    interpreter::BytecodeArrayBuilder inner_builder(isolate(), zone());
+    inner_builder.set_locals_count(0);
+    inner_builder.set_context_count(0);
+    inner_builder.set_parameter_count(3);
+    inner_builder.LoadAccumulatorWithRegister(inner_builder.Parameter(2))
+        .BinaryOperation(Token::Value::ADD, inner_builder.Parameter(1),
+                         Strength::WEAK)
+        .Return();
+
+    std::pair<Graph*, Handle<SharedFunctionInfo>> inner_graph_and_shared_info =
+        GetCompletedGraphAndSharedInfo(inner_builder.ToBytecodeArray());
+    Handle<SharedFunctionInfo> shared_info = inner_graph_and_shared_info.second;
+
+    interpreter::BytecodeArrayBuilder builder(isolate(), zone());
+    builder.set_locals_count(4);
+    builder.set_context_count(0);
+    builder.set_parameter_count(3);
+    builder.CreateClosure(shared_info, pretenure_flag).Return();
+
+    Graph* graph = GetCompletedGraph(builder.ToBytecodeArray());
+    Node* start = graph->start();
+    Node* ret = graph->end()->InputAt(0);
+
+    Matcher<Node*> create_closure =
+        IsCreateClosure(shared_info, pretenure_flag, start, start);
+    EXPECT_THAT(ret, IsReturn(create_closure, create_closure, start));
+  }
+}
+
 }  // namespace compiler
 }  // namespace internal
 }  // namespace v8

test/unittests/compiler/node-test-utils.cc

@@ -27,6 +27,7 @@ bool operator==(Handle<HeapObject> const& lhs, Handle<HeapObject> const& rhs) {
   return lhs.is_identical_to(rhs);
 }
 
+
 namespace compiler {
 
 namespace {
@@ -1858,6 +1859,56 @@ class IsJSCallMatcher final : public NodeMatcher {
   const Matcher<Node*> control_matcher_;
 };
 
+
+class IsCreateClosureMatcher final : public NodeMatcher {
+ public:
+  IsCreateClosureMatcher(
+      const Matcher<Handle<SharedFunctionInfo>>& shared_info_matcher,
+      const Matcher<PretenureFlag>& pretenure_matcher,
+      const Matcher<Node*>& effect_matcher,
+      const Matcher<Node*>& control_matcher)
+      : NodeMatcher(IrOpcode::Value::kJSCreateClosure),
+        shared_info_matcher_(shared_info_matcher),
+        pretenure_matcher_(pretenure_matcher),
+        effect_matcher_(effect_matcher),
+        control_matcher_(control_matcher) {}
+
+  void DescribeTo(std::ostream* os) const final {
+    NodeMatcher::DescribeTo(os);
+    *os << " whose value (";
+    shared_info_matcher_.DescribeTo(os);
+    *os << ",";
+    pretenure_matcher_.DescribeTo(os);
+    *os << "), effect (";
+    effect_matcher_.DescribeTo(os);
+    *os << ") and control (";
+    control_matcher_.DescribeTo(os);
+    *os << ")";
+  }
+
+  bool MatchAndExplain(Node* node, MatchResultListener* listener) const final {
+    if (!NodeMatcher::MatchAndExplain(node, listener)) {
+      return false;
+    }
+    return (PrintMatchAndExplain(
+                OpParameter<const CreateClosureParameters>(node).shared_info(),
+                "value", shared_info_matcher_, listener) &&
+            PrintMatchAndExplain(
+                OpParameter<CreateClosureParameters>(node).pretenure(), "value",
+                pretenure_matcher_, listener) &&
+            PrintMatchAndExplain(NodeProperties::GetEffectInput(node), "effect",
+                                 effect_matcher_, listener) &&
+            PrintMatchAndExplain(NodeProperties::GetControlInput(node),
+                                 "control", control_matcher_, listener));
+  }
+
+ private:
+  const Matcher<Handle<SharedFunctionInfo>> shared_info_matcher_;
+  const Matcher<PretenureFlag> pretenure_matcher_;
+  const Matcher<Node*> effect_matcher_;
+  const Matcher<Node*> control_matcher_;
+};
+
 }  // namespace
 
 
@@ -2535,6 +2586,15 @@ Matcher<Node*> IsJSCallRuntime(std::vector<Matcher<Node*>> value_matchers,
 }
 
 
+Matcher<Node*> IsCreateClosure(const Handle<SharedFunctionInfo> shared_info,
+                               PretenureFlag pretenure,
+                               const Matcher<Node*>& effect_matcher,
+                               const Matcher<Node*>& control_matcher) {
+  return MakeMatcher(new IsCreateClosureMatcher(
+      shared_info, pretenure, effect_matcher, control_matcher));
+}
+
+
 #define IS_BINOP_MATCHER(Name)                                            \
   Matcher<Node*> Is##Name(const Matcher<Node*>& lhs_matcher,              \
                           const Matcher<Node*>& rhs_matcher) {            \

test/unittests/compiler/node-test-utils.h

@@ -17,6 +17,7 @@ class ExternalReference;
 template <typename T>
 class Handle;
 class HeapObject;
+class SharedFunctionInfo;
 template <class>
 class TypeImpl;
 enum TypeofMode : int;
@@ -405,6 +406,10 @@ Matcher<Node*> IsJSDeleteProperty(const Matcher<Node*>& object_value_matcher,
                                   const Matcher<Node*>& key_matcher,
                                   const Matcher<Node*>& effect_matcher,
                                   const Matcher<Node*>& control_matcher);
+Matcher<Node*> IsCreateClosure(const Handle<SharedFunctionInfo> shared_info,
+                               PretenureFlag pretenure,
+                               const Matcher<Node*>& effect_matcher,
+                               const Matcher<Node*>& control_matcher);
 
 }  // namespace compiler
 }  // namespace internal

test/unittests/interpreter/bytecode-array-builder-unittest.cc

@@ -94,7 +94,11 @@ TEST_F(BytecodeArrayBuilderTest, AllBytecodesGenerated) {
       .StoreKeyedProperty(reg, reg, 2056, LanguageMode::STRICT);
 
   // Emit closure operations.
-  builder.CreateClosure(NOT_TENURED);
+  Factory* factory = isolate()->factory();
+  Handle<SharedFunctionInfo> shared_info = factory->NewSharedFunctionInfo(
+      factory->NewStringFromStaticChars("function_a"), MaybeHandle<Code>(),
+      false);
+  builder.CreateClosure(shared_info, NOT_TENURED);
 
   // Emit argument creation operations.
   builder.CreateArguments(CreateArgumentsType::kMappedArguments)
@@ -215,6 +219,12 @@ TEST_F(BytecodeArrayBuilderTest, AllBytecodesGenerated) {
   }
   builder.LoadLiteral(Smi::FromInt(20000000));
 
+  // CreateClosureWide
+  Handle<SharedFunctionInfo> shared_info2 = factory->NewSharedFunctionInfo(
+      factory->NewStringFromStaticChars("function_b"), MaybeHandle<Code>(),
+      false);
+  builder.CreateClosure(shared_info2, NOT_TENURED);
+
   builder.Return();
 
   // Generate BytecodeArray.