[Interpreter] Add support for CallRuntimeForPair to Bytecode Graph Builder.

Adds support for the CallRuntimeForPair bytecode to the Bytecode Graph
Builder. Modifies the FrameState support to allow updating of output
registers.

Also adds Eval tests to test-run-bytecode-graph-builder since these are
enabled by CallRuntimeForPair support.

BUG=v8:4280
LOG=N

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

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

src/compiler/bytecode-graph-builder.cc

@@ -18,22 +18,28 @@ class BytecodeGraphBuilder::FrameStateBeforeAndAfter {
  public:
   FrameStateBeforeAndAfter(BytecodeGraphBuilder* builder,
                            const interpreter::BytecodeArrayIterator& iterator)
-      : builder_(builder), id_after_(BailoutId::None()) {
+      : builder_(builder),
+        id_after_(BailoutId::None()),
+        added_to_node_(false),
+        output_poke_offset_(0),
+        output_poke_count_(0) {
     BailoutId id_before(iterator.current_offset());
     frame_state_before_ = builder_->environment()->Checkpoint(
-        id_before, AccumulatorUpdateMode::kOutputIgnored);
+        id_before, OutputFrameStateCombine::Ignore());
     id_after_ = BailoutId(id_before.ToInt() + iterator.current_bytecode_size());
   }
 
   ~FrameStateBeforeAndAfter() {
-    DCHECK(builder_->environment()->StateValuesAreUpToDate(
-        accumulator_update_mode_));
+    DCHECK(added_to_node_);
+    DCHECK(builder_->environment()->StateValuesAreUpToDate(output_poke_offset_,
+                                                           output_poke_count_));
   }
 
  private:
   friend class Environment;
 
-  void AddToNode(Node* node, AccumulatorUpdateMode update_mode) {
+  void AddToNode(Node* node, OutputFrameStateCombine combine) {
+    DCHECK(!added_to_node_);
     int count = OperatorProperties::GetFrameStateInputCount(node->op());
     DCHECK_LE(count, 2);
     if (count >= 1) {
@@ -41,7 +47,7 @@ class BytecodeGraphBuilder::FrameStateBeforeAndAfter {
       DCHECK_EQ(IrOpcode::kDead,
                 NodeProperties::GetFrameStateInput(node, 0)->opcode());
       Node* frame_state_after =
-          builder_->environment()->Checkpoint(id_after_, update_mode);
+          builder_->environment()->Checkpoint(id_after_, combine);
       NodeProperties::ReplaceFrameStateInput(node, 0, frame_state_after);
     }
 
@@ -51,13 +57,21 @@ class BytecodeGraphBuilder::FrameStateBeforeAndAfter {
                 NodeProperties::GetFrameStateInput(node, 1)->opcode());
       NodeProperties::ReplaceFrameStateInput(node, 1, frame_state_before_);
     }
-    accumulator_update_mode_ = update_mode;
+
+    if (!combine.IsOutputIgnored()) {
+      output_poke_offset_ = static_cast<int>(combine.GetOffsetToPokeAt());
+      output_poke_count_ = node->op()->ValueOutputCount();
+    }
+    added_to_node_ = true;
   }
 
   BytecodeGraphBuilder* builder_;
   Node* frame_state_before_;
   BailoutId id_after_;
-  AccumulatorUpdateMode accumulator_update_mode_;
+
+  bool added_to_node_;
+  int output_poke_offset_;
+  int output_poke_count_;
 };
 
 
@@ -134,10 +148,8 @@ int BytecodeGraphBuilder::Environment::RegisterToValuesIndex(
 }
 
 
-void BytecodeGraphBuilder::Environment::BindRegister(
-    interpreter::Register the_register, Node* node) {
-  int values_index = RegisterToValuesIndex(the_register);
-  values()->at(values_index) = node;
+Node* BytecodeGraphBuilder::Environment::LookupAccumulator() const {
+  return values()->at(accumulator_base_);
 }
 
 
@@ -156,33 +168,55 @@ Node* BytecodeGraphBuilder::Environment::LookupRegister(
 }
 
 
+void BytecodeGraphBuilder::Environment::ExchangeRegisters(
+    interpreter::Register reg0, interpreter::Register reg1) {
+  int reg0_index = RegisterToValuesIndex(reg0);
+  int reg1_index = RegisterToValuesIndex(reg1);
+  Node* saved_reg0_value = values()->at(reg0_index);
+  values()->at(reg0_index) = values()->at(reg1_index);
+  values()->at(reg1_index) = saved_reg0_value;
+}
+
+
 void BytecodeGraphBuilder::Environment::BindAccumulator(
     Node* node, FrameStateBeforeAndAfter* states) {
   if (states) {
-    states->AddToNode(node, AccumulatorUpdateMode::kOutputInAccumulator);
+    states->AddToNode(node, OutputFrameStateCombine::PokeAt(0));
   }
   values()->at(accumulator_base_) = node;
 }
 
 
-void BytecodeGraphBuilder::Environment::RecordAfterState(
-    Node* node, FrameStateBeforeAndAfter* states) {
-  states->AddToNode(node, AccumulatorUpdateMode::kOutputIgnored);
+void BytecodeGraphBuilder::Environment::BindRegister(
+    interpreter::Register the_register, Node* node,
+    FrameStateBeforeAndAfter* states) {
+  int values_index = RegisterToValuesIndex(the_register);
+  if (states) {
+    states->AddToNode(node, OutputFrameStateCombine::PokeAt(accumulator_base_ -
+                                                            values_index));
+  }
+  values()->at(values_index) = node;
 }
 
 
-void BytecodeGraphBuilder::Environment::ExchangeRegisters(
-    interpreter::Register reg0, interpreter::Register reg1) {
-  int reg0_index = RegisterToValuesIndex(reg0);
-  int reg1_index = RegisterToValuesIndex(reg1);
-  Node* saved_reg0_value = values()->at(reg0_index);
-  values()->at(reg0_index) = values()->at(reg1_index);
-  values()->at(reg1_index) = saved_reg0_value;
+void BytecodeGraphBuilder::Environment::BindRegistersToProjections(
+    interpreter::Register first_reg, Node* node,
+    FrameStateBeforeAndAfter* states) {
+  int values_index = RegisterToValuesIndex(first_reg);
+  if (states) {
+    states->AddToNode(node, OutputFrameStateCombine::PokeAt(accumulator_base_ -
+                                                            values_index));
+  }
+  for (int i = 0; i < node->op()->ValueOutputCount(); i++) {
+    values()->at(values_index + i) =
+        builder()->NewNode(common()->Projection(i), node);
+  }
 }
 
 
-Node* BytecodeGraphBuilder::Environment::LookupAccumulator() const {
-  return values()->at(accumulator_base_);
+void BytecodeGraphBuilder::Environment::RecordAfterState(
+    Node* node, FrameStateBeforeAndAfter* states) {
+  states->AddToNode(node, OutputFrameStateCombine::Ignore());
 }
 
 
@@ -290,7 +324,7 @@ void BytecodeGraphBuilder::Environment::UpdateStateValues(Node** state_values,
 
 
 Node* BytecodeGraphBuilder::Environment::Checkpoint(
-    BailoutId bailout_id, AccumulatorUpdateMode update_mode) {
+    BailoutId bailout_id, OutputFrameStateCombine combine) {
   if (!builder()->info()->is_deoptimization_enabled()) {
     return builder()->jsgraph()->EmptyFrameState();
   }
@@ -301,13 +335,8 @@ Node* BytecodeGraphBuilder::Environment::Checkpoint(
                     register_count());
   UpdateStateValues(&accumulator_state_values_, accumulator_base(), 1);
 
-  OutputFrameStateCombine combine =
-      update_mode == AccumulatorUpdateMode::kOutputIgnored
-          ? OutputFrameStateCombine::Ignore()
-          : OutputFrameStateCombine::PokeAt(0);
   const Operator* op = common()->FrameState(
       bailout_id, combine, builder()->frame_state_function_info());
-
   Node* result = graph()->NewNode(
       op, parameters_state_values_, registers_state_values_,
       accumulator_state_values_, Context(), builder()->GetFunctionClosure(),
@@ -318,14 +347,32 @@ Node* BytecodeGraphBuilder::Environment::Checkpoint(
 
 
 bool BytecodeGraphBuilder::Environment::StateValuesAreUpToDate(
-    AccumulatorUpdateMode update_mode) {
-  return !StateValuesRequireUpdate(&parameters_state_values_, 0,
-                                   parameter_count()) &&
-         !StateValuesRequireUpdate(&registers_state_values_, register_base(),
-                                   register_count()) &&
-         (update_mode == AccumulatorUpdateMode::kOutputInAccumulator ||
-          !StateValuesRequireUpdate(&accumulator_state_values_,
-                                    accumulator_base(), 1));
+    Node** state_values, int offset, int count, int output_poke_start,
+    int output_poke_end) {
+  DCHECK_LE(static_cast<size_t>(offset + count), values()->size());
+  for (int i = 0; i < count; i++, offset++) {
+    if (offset < output_poke_start || offset >= output_poke_end) {
+      if ((*state_values)->InputAt(i) != values()->at(offset)) {
+        return false;
+      }
+    }
+  }
+  return true;
+}
+
+
+bool BytecodeGraphBuilder::Environment::StateValuesAreUpToDate(
+    int output_poke_offset, int output_poke_count) {
+  // Poke offset is relative to the top of the stack (i.e., the accumulator).
+  int output_poke_start = accumulator_base() - output_poke_offset;
+  int output_poke_end = output_poke_start + output_poke_count;
+  return StateValuesAreUpToDate(&parameters_state_values_, 0, parameter_count(),
+                                output_poke_start, output_poke_end) &&
+         StateValuesAreUpToDate(&registers_state_values_, register_base(),
+                                register_count(), output_poke_start,
+                                output_poke_end) &&
+         StateValuesAreUpToDate(&accumulator_state_values_, accumulator_base(),
+                                1, output_poke_start, output_poke_end);
 }
 
 
@@ -421,7 +468,10 @@ Node* BytecodeGraphBuilder::BuildLoadFeedbackVector() {
 
 VectorSlotPair BytecodeGraphBuilder::CreateVectorSlotPair(int slot_id) {
   Handle<TypeFeedbackVector> feedback_vector = info()->feedback_vector();
-  FeedbackVectorSlot slot = feedback_vector->ToSlot(slot_id);
+  FeedbackVectorSlot slot;
+  if (slot_id >= TypeFeedbackVector::kReservedIndexCount) {
+    slot = feedback_vector->ToSlot(slot_id);
+  }
   return VectorSlotPair(feedback_vector, slot);
 }
 
@@ -1201,7 +1251,17 @@ void BytecodeGraphBuilder::VisitCallRuntime(
 
 void BytecodeGraphBuilder::VisitCallRuntimeForPair(
     const interpreter::BytecodeArrayIterator& iterator) {
-  UNIMPLEMENTED();
+  FrameStateBeforeAndAfter states(this, iterator);
+  Runtime::FunctionId functionId =
+      static_cast<Runtime::FunctionId>(iterator.GetIndexOperand(0));
+  interpreter::Register first_arg = iterator.GetRegisterOperand(1);
+  size_t arg_count = iterator.GetCountOperand(2);
+  interpreter::Register first_return = iterator.GetRegisterOperand(3);
+
+  // Create node to perform the runtime call.
+  const Operator* call = javascript()->CallRuntime(functionId, arg_count);
+  Node* return_pair = ProcessCallRuntimeArguments(call, first_arg, arg_count);
+  environment()->BindRegistersToProjections(first_return, return_pair, &states);
 }
 
 

src/compiler/bytecode-graph-builder.h

@@ -28,11 +28,6 @@ class BytecodeGraphBuilder {
   Graph* graph() const { return jsgraph_->graph(); }
 
  private:
-  enum class AccumulatorUpdateMode {
-    kOutputIgnored,
-    kOutputInAccumulator,
-  };
-
   class Environment;
   class FrameStateBeforeAndAfter;
 
@@ -263,14 +258,17 @@ class BytecodeGraphBuilder::Environment : public ZoneObject {
   int parameter_count() const { return parameter_count_; }
   int register_count() const { return register_count_; }
 
-  void BindRegister(interpreter::Register the_register, Node* node);
+  Node* LookupAccumulator() const;
   Node* LookupRegister(interpreter::Register the_register) const;
+
   void ExchangeRegisters(interpreter::Register reg0,
                          interpreter::Register reg1);
 
   void BindAccumulator(Node* node, FrameStateBeforeAndAfter* states = nullptr);
-  Node* LookupAccumulator() const;
-
+  void BindRegister(interpreter::Register the_register, Node* node,
+                    FrameStateBeforeAndAfter* states = nullptr);
+  void BindRegistersToProjections(interpreter::Register first_reg, Node* node,
+                                  FrameStateBeforeAndAfter* states = nullptr);
   void RecordAfterState(Node* node, FrameStateBeforeAndAfter* states);
 
   bool IsMarkedAsUnreachable() const;
@@ -284,12 +282,11 @@ class BytecodeGraphBuilder::Environment : public ZoneObject {
 
   // Preserve a checkpoint of the environment for the IR graph. Any
   // further mutation of the environment will not affect checkpoints.
-  Node* Checkpoint(BailoutId ast_id, AccumulatorUpdateMode update_mode);
+  Node* Checkpoint(BailoutId bytecode_offset, OutputFrameStateCombine combine);
 
   // Returns true if the state values are up to date with the current
-  // environment. If update_mode is AccumulatorUpdateMode::kOutputInAccumulator
-  // then accumulator state can be different from the environment.
-  bool StateValuesAreUpToDate(AccumulatorUpdateMode update_mode);
+  // environment.
+  bool StateValuesAreUpToDate(int output_poke_offset, int output_poke_count);
 
   // Control dependency tracked by this environment.
   Node* GetControlDependency() const { return control_dependency_; }
@@ -307,6 +304,8 @@ class BytecodeGraphBuilder::Environment : public ZoneObject {
  private:
   explicit Environment(const Environment* copy);
   void PrepareForLoop();
+  bool StateValuesAreUpToDate(Node** state_values, int offset, int count,
+                              int output_poke_start, int output_poke_end);
   bool StateValuesRequireUpdate(Node** state_values, int offset, int count);
   void UpdateStateValues(Node** state_values, int offset, int count);
 

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

@@ -70,6 +70,7 @@ class BytecodeGraphTester {
     i::FLAG_ignition = true;
     i::FLAG_always_opt = false;
     i::FLAG_allow_natives_syntax = true;
+    i::FLAG_ignition_fallback_on_eval_and_catch = false;
     // Set ignition filter flag via SetFlagsFromString to avoid double-free
     // (or potential leak with StrDup() based on ownership confusion).
     ScopedVector<char> ignition_filter(64);
@@ -1102,6 +1103,117 @@ TEST(BytecodeGraphBuilderLookupSlotWide) {
 }
 
 
+TEST(BytecodeGraphBuilderEval) {
+  HandleAndZoneScope scope;
+  Isolate* isolate = scope.main_isolate();
+  Zone* zone = scope.main_zone();
+  Factory* factory = isolate->factory();
+
+  ExpectedSnippet<0> snippets[] = {
+      {"return eval('1;');", {handle(Smi::FromInt(1), isolate)}},
+      {"return eval('100 * 20;');", {handle(Smi::FromInt(2000), isolate)}},
+      {"var x = 10; return eval('x + 20;');",
+       {handle(Smi::FromInt(30), isolate)}},
+      {"var x = 10; eval('x = 33;'); return x;",
+       {handle(Smi::FromInt(33), isolate)}},
+      {"'use strict'; var x = 20; var z = 0;\n"
+       "eval('var x = 33; z = x;'); return x + z;",
+       {handle(Smi::FromInt(53), isolate)}},
+      {"eval('var x = 33;'); eval('var y = x + 20'); return x + y;",
+       {handle(Smi::FromInt(86), isolate)}},
+      {"var x = 1; eval('for(i = 0; i < 10; i++) x = x + 1;'); return x",
+       {handle(Smi::FromInt(11), isolate)}},
+      {"var x = 10; eval('var x = 20;'); return x;",
+       {handle(Smi::FromInt(20), isolate)}},
+      {"var x = 1; eval('\"use strict\"; var x = 2;'); return x;",
+       {handle(Smi::FromInt(1), isolate)}},
+      {"'use strict'; var x = 1; eval('var x = 2;'); return x;",
+       {handle(Smi::FromInt(1), isolate)}},
+      {"var x = 10; eval('x + 20;'); return typeof x;",
+       {factory->NewStringFromStaticChars("number")}},
+      {"eval('var y = 10;'); return typeof unallocated;",
+       {factory->NewStringFromStaticChars("undefined")}},
+      {"'use strict'; eval('var y = 10;'); return typeof unallocated;",
+       {factory->NewStringFromStaticChars("undefined")}},
+      {"eval('var x = 10;'); return typeof x;",
+       {factory->NewStringFromStaticChars("number")}},
+      {"var x = {}; eval('var x = 10;'); return typeof x;",
+       {factory->NewStringFromStaticChars("number")}},
+      {"'use strict'; var x = {}; eval('var x = 10;'); return typeof x;",
+       {factory->NewStringFromStaticChars("object")}},
+  };
+
+  size_t num_snippets = sizeof(snippets) / sizeof(snippets[0]);
+  for (size_t i = 0; i < num_snippets; i++) {
+    ScopedVector<char> script(1024);
+    SNPrintF(script, "function %s() { %s }\n%s();", kFunctionName,
+             snippets[i].code_snippet, kFunctionName);
+    BytecodeGraphTester tester(isolate, zone, script.start());
+    auto callable = tester.GetCallable<>();
+    Handle<Object> return_value = callable().ToHandleChecked();
+    CHECK(return_value->SameValue(*snippets[i].return_value()));
+  }
+}
+
+
+TEST(BytecodeGraphBuilderEvalParams) {
+  HandleAndZoneScope scope;
+  Isolate* isolate = scope.main_isolate();
+  Zone* zone = scope.main_zone();
+
+  ExpectedSnippet<1> snippets[] = {
+      {"var x = 10; return eval('x + p1;');",
+       {handle(Smi::FromInt(30), isolate), handle(Smi::FromInt(20), isolate)}},
+      {"var x = 10; eval('p1 = x;'); return p1;",
+       {handle(Smi::FromInt(10), isolate), handle(Smi::FromInt(20), isolate)}},
+      {"var a = 10;"
+       "function inner() { return eval('a + p1;');}"
+       "return inner();",
+       {handle(Smi::FromInt(30), isolate), handle(Smi::FromInt(20), isolate)}},
+  };
+
+  size_t num_snippets = sizeof(snippets) / sizeof(snippets[0]);
+  for (size_t i = 0; i < num_snippets; i++) {
+    ScopedVector<char> script(1024);
+    SNPrintF(script, "function %s(p1) { %s }\n%s(0);", kFunctionName,
+             snippets[i].code_snippet, kFunctionName);
+    BytecodeGraphTester tester(isolate, zone, script.start());
+    auto callable = tester.GetCallable<Handle<Object>>();
+    Handle<Object> return_value =
+        callable(snippets[i].parameter(0)).ToHandleChecked();
+    CHECK(return_value->SameValue(*snippets[i].return_value()));
+  }
+}
+
+
+TEST(BytecodeGraphBuilderEvalGlobal) {
+  HandleAndZoneScope scope;
+  Isolate* isolate = scope.main_isolate();
+  Zone* zone = scope.main_zone();
+  Factory* factory = isolate->factory();
+
+  ExpectedSnippet<0> snippets[] = {
+      {"function add_global() { eval('function f() { z = 33; }; f()'); };"
+       "function f() { add_global(); return z; }; f();",
+       {handle(Smi::FromInt(33), isolate)}},
+      {"function add_global() {\n"
+       " eval('\"use strict\"; function f() { y = 33; };"
+       "      try { f() } catch(e) {}');\n"
+       "}\n"
+       "function f() { add_global(); return typeof y; } f();",
+       {factory->NewStringFromStaticChars("undefined")}},
+  };
+
+  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()));
+  }
+}
+
+
 bool get_compare_result(Token::Value opcode, Handle<Object> lhs_value,
                         Handle<Object> rhs_value) {
   switch (opcode) {

test/mjsunit/compiler/deopt-during-eval-lookup.js

+// Copyright 2015 the V8 project authors. All rights reserved.
+// Redistribution and use in source and binary forms, with or without
+// modification, are permitted provided that the following conditions are
+// met:
+//
+//     * Redistributions of source code must retain the above copyright
+//       notice, this list of conditions and the following disclaimer.
+//     * Redistributions in binary form must reproduce the above
+//       copyright notice, this list of conditions and the following
+//       disclaimer in the documentation and/or other materials provided
+//       with the distribution.
+//     * Neither the name of Google Inc. nor the names of its
+//       contributors may be used to endorse or promote products derived
+//       from this software without specific prior written permission.
+//
+// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+// Flags: --allow-natives-syntax
+
+function g() {
+  return 100;
+}
+
+function getter() {
+  // Test that we can deopt during the CallRuntimeForPair call to LoadLookupSlot
+  %DeoptimizeFunction(f);
+  return g;
+}
+
+Object.defineProperty(this, "eval", {get: getter });
+
+function f() {
+  return eval("200");
+}
+
+%OptimizeFunctionOnNextCall(f);
+assertEquals(100, f());