Revert "[turbofan] Implement lowering of {JSCreateClosure}."

This reverts commit 9d3c4b4b.

Reason for revert: Breaks cctest/test-debug/NoBreakWhenBootstrapping in no-snap mode.

Original change's description:
> [turbofan] Implement lowering of {JSCreateClosure}.
> 
> This adds support for inline allocation of {JSFunction} objects as part
> of closures instantiation for {JSCreateClosure} nodes. The lowering is
> limited to instantiation sites which have already seen more than one
> previous instantiation, this avoids the need to increment the respective
> counter.
> 
> R=​jarin@chromium.org
> 
> Change-Id: I462c557453fe58bc5f09020a3d5ebdf11c2ea68b
> Reviewed-on: https://chromium-review.googlesource.com/594287
> Commit-Queue: Michael Starzinger <mstarzinger@chromium.org>
> Reviewed-by: Jaroslav Sevcik <jarin@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#48176}

TBR=mstarzinger@chromium.org,jarin@chromium.org

Change-Id: Id52281f6a3c0b7c2603053ecf002777d5b0d6f1f
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/686534Reviewed-by: Michael Starzinger <mstarzinger@chromium.org>
Commit-Queue: Michael Starzinger <mstarzinger@chromium.org>
Cr-Commit-Position: refs/heads/master@{#48178}

src/accessors.cc

@@ -933,99 +933,47 @@ static inline bool AllowAccessToFunction(Context* current_context,
 class FrameFunctionIterator {
  public:
   explicit FrameFunctionIterator(Isolate* isolate)
-      : isolate_(isolate), frame_iterator_(isolate), inlined_frame_index_(-1) {
+      : isolate_(isolate), frame_iterator_(isolate) {
     GetFrames();
   }
-
-  // Iterate through functions until the first occurrence of 'function'.
-  // Returns true if one is found, and false if the iterator ends before.
-  bool Find(Handle<JSFunction> function) {
-    do {
-      if (!next().ToHandle(&function_)) return false;
-    } while (!function_.is_identical_to(function));
-    return true;
-  }
-
-  // Iterate through functions until the next non-toplevel one is found.
-  // Returns true if one is found, and false if the iterator ends before.
-  bool FindNextNonTopLevel() {
-    do {
-      if (!next().ToHandle(&function_)) return false;
-    } while (function_->shared()->is_toplevel());
-    return true;
-  }
-
-  // Iterate through function until the first native or user-provided function
-  // is found. Functions not defined in user-provided scripts are not visible
-  // unless directly exposed, in which case the native flag is set on them.
-  // Returns true if one is found, and false if the iterator ends before.
-  bool FindFirstNativeOrUserJavaScript() {
-    while (!function_->shared()->native() &&
-           !function_->shared()->IsUserJavaScript()) {
-      if (!next().ToHandle(&function_)) return false;
-    }
-    return true;
-  }
-
-  // In case of inlined frames the function could have been materialized from
-  // deoptimization information. If that is the case we need to make sure that
-  // subsequent call will see the same function, since we are about to hand out
-  // the value to JavaScript. Make sure to store the materialized value and
-  // trigger a deoptimization of the underlying frame.
-  Handle<JSFunction> MaterializeFunction() {
-    if (inlined_frame_index_ == 0) return function_;
-
-    JavaScriptFrame* frame = frame_iterator_.frame();
-    TranslatedState translated_values(frame);
-    translated_values.Prepare(frame->fp());
-
-    TranslatedFrame* translated_frame =
-        translated_values.GetFrameFromJSFrameIndex(inlined_frame_index_);
-    TranslatedFrame::iterator iter = translated_frame->begin();
-
-    // First value is the function.
-    bool should_deoptimize = iter->IsMaterializedObject();
-    Handle<Object> value = iter->GetValue();
-    if (should_deoptimize) {
-      translated_values.StoreMaterializedValuesAndDeopt(frame);
-    }
-
-    return Handle<JSFunction>::cast(value);
-  }
-
- private:
   MaybeHandle<JSFunction> next() {
     while (true) {
-      inlined_frame_index_--;
-      if (inlined_frame_index_ == -1) {
-        if (!frame_iterator_.done()) {
-          frame_iterator_.Advance();
-          frames_.clear();
-          GetFrames();
-        }
-        if (inlined_frame_index_ == -1) return MaybeHandle<JSFunction>();
-        inlined_frame_index_--;
-      }
+      if (frames_.empty()) return MaybeHandle<JSFunction>();
       Handle<JSFunction> next_function =
-          frames_[inlined_frame_index_].AsJavaScript().function();
+          frames_.back().AsJavaScript().function();
+      frames_.pop_back();
+      if (frames_.empty()) {
+        GetFrames();
+      }
       // Skip functions from other origins.
       if (!AllowAccessToFunction(isolate_->context(), *next_function)) continue;
       return next_function;
     }
   }
+
+  // Iterate through functions until the first occurrence of 'function'.
+  // Returns true if 'function' is found, and false if the iterator ends
+  // without finding it.
+  bool Find(Handle<JSFunction> function) {
+    Handle<JSFunction> next_function;
+    do {
+      if (!next().ToHandle(&next_function)) return false;
+    } while (!next_function.is_identical_to(function));
+    return true;
+  }
+
+ private:
   void GetFrames() {
-    DCHECK_EQ(-1, inlined_frame_index_);
+    DCHECK(frames_.empty());
     if (frame_iterator_.done()) return;
     JavaScriptFrame* frame = frame_iterator_.frame();
     frame->Summarize(&frames_);
-    inlined_frame_index_ = static_cast<int>(frames_.size());
-    DCHECK_LT(0, inlined_frame_index_);
+    DCHECK(!frames_.empty());
+    frame_iterator_.Advance();
   }
   Isolate* isolate_;
-  Handle<JSFunction> function_;
   JavaScriptFrameIterator frame_iterator_;
   std::vector<FrameSummary> frames_;
-  int inlined_frame_index_;
 };
 
 
@@ -1035,27 +983,28 @@ MaybeHandle<JSFunction> FindCaller(Isolate* isolate,
   if (function->shared()->native()) {
     return MaybeHandle<JSFunction>();
   }
-  // Find the function from the frames. Return null in case no frame
-  // corresponding to the given function was found.
+  // Find the function from the frames.
   if (!it.Find(function)) {
+    // No frame corresponding to the given function found. Return null.
     return MaybeHandle<JSFunction>();
   }
   // Find previously called non-toplevel function.
-  if (!it.FindNextNonTopLevel()) {
-    return MaybeHandle<JSFunction>();
+  Handle<JSFunction> caller;
+  do {
+    if (!it.next().ToHandle(&caller)) return MaybeHandle<JSFunction>();
+  } while (caller->shared()->is_toplevel());
+
+  // If caller is not user code and caller's caller is also not user code,
+  // use that instead.
+  MaybeHandle<JSFunction> potential_caller = caller;
+  while (!potential_caller.is_null() &&
+         !potential_caller.ToHandleChecked()->shared()->IsUserJavaScript()) {
+    caller = potential_caller.ToHandleChecked();
+    potential_caller = it.next();
   }
-  // Find the first user-land JavaScript function (or the entry point into
-  // native JavaScript builtins in case such a builtin was the caller).
-  if (!it.FindFirstNativeOrUserJavaScript()) {
-    return MaybeHandle<JSFunction>();
+  if (!caller->shared()->native() && !potential_caller.is_null()) {
+    caller = potential_caller.ToHandleChecked();
   }
-
-  // Materialize the function that the iterator is currently sitting on. Note
-  // that this might trigger deoptimization in case the function was actually
-  // materialized. Identity of the function must be preserved because we are
-  // going to return it to JavaScript after this point.
-  Handle<JSFunction> caller = it.MaterializeFunction();
-
   // Censor if the caller is not a sloppy mode function.
   // Change from ES5, which used to throw, see:
   // https://bugs.ecmascript.org/show_bug.cgi?id=310

src/compiler/js-create-lowering.cc

@@ -214,8 +214,6 @@ Reduction JSCreateLowering::Reduce(Node* node) {
       return ReduceJSCreateArguments(node);
     case IrOpcode::kJSCreateArray:
       return ReduceJSCreateArray(node);
-    case IrOpcode::kJSCreateClosure:
-      return ReduceJSCreateClosure(node);
     case IrOpcode::kJSCreateIterResultObject:
       return ReduceJSCreateIterResultObject(node);
     case IrOpcode::kJSCreateKeyValueArray:
@@ -795,48 +793,6 @@ Reduction JSCreateLowering::ReduceJSCreateArray(Node* node) {
   return ReduceNewArrayToStubCall(node, site);
 }
 
-Reduction JSCreateLowering::ReduceJSCreateClosure(Node* node) {
-  DCHECK_EQ(IrOpcode::kJSCreateClosure, node->opcode());
-  CreateClosureParameters const& p = CreateClosureParametersOf(node->op());
-  Handle<SharedFunctionInfo> shared = p.shared_info();
-  Node* effect = NodeProperties::GetEffectInput(node);
-  Node* control = NodeProperties::GetControlInput(node);
-  Node* context = NodeProperties::GetContextInput(node);
-
-  // Use inline allocation of closures only for instantiation sites that have
-  // seen more than one instantiation, this simplifies the generated code and
-  // also serves as a heuristic of which allocation sites benefit from it.
-  FeedbackSlot slot(FeedbackVector::ToSlot(p.feedback().index()));
-  Handle<Cell> vector_cell(Cell::cast(p.feedback().vector()->Get(slot)));
-  if (vector_cell->map() == isolate()->heap()->many_closures_cell_map()) {
-    Node* function_map = jsgraph()->HeapConstant(
-        handle(Map::cast(native_context()->get(shared->function_map_index()))));
-    Node* lazy_compile_builtin = jsgraph()->HeapConstant(
-        handle(isolate()->builtins()->builtin(Builtins::kCompileLazy)));
-
-    // Emit code to allocate the JSFunction instance.
-    AllocationBuilder a(jsgraph(), effect, control);
-    a.Allocate(JSFunction::kSize);
-    a.Store(AccessBuilder::ForMap(), function_map);
-    a.Store(AccessBuilder::ForJSObjectPropertiesOrHash(),
-            jsgraph()->EmptyFixedArrayConstant());
-    a.Store(AccessBuilder::ForJSObjectElements(),
-            jsgraph()->EmptyFixedArrayConstant());
-    a.Store(AccessBuilder::ForJSFunctionPrototypeOrInitialMap(),
-            jsgraph()->TheHoleConstant());
-    a.Store(AccessBuilder::ForJSFunctionSharedFunctionInfo(), shared);
-    a.Store(AccessBuilder::ForJSFunctionContext(), context);
-    a.Store(AccessBuilder::ForJSFunctionFeedbackVector(), vector_cell);
-    a.Store(AccessBuilder::ForJSFunctionCode(), lazy_compile_builtin);
-    STATIC_ASSERT(JSFunction::kSize == 8 * kPointerSize);
-    RelaxControls(node);
-    a.FinishAndChange(node);
-    return Changed(node);
-  }
-
-  return NoChange();
-}
-
 Reduction JSCreateLowering::ReduceJSCreateIterResultObject(Node* node) {
   DCHECK_EQ(IrOpcode::kJSCreateIterResultObject, node->opcode());
   Node* value = NodeProperties::GetValueInput(node, 0);

src/compiler/js-create-lowering.h

@@ -50,7 +50,6 @@ class V8_EXPORT_PRIVATE JSCreateLowering final
   Reduction ReduceJSCreate(Node* node);
   Reduction ReduceJSCreateArguments(Node* node);
   Reduction ReduceJSCreateArray(Node* node);
-  Reduction ReduceJSCreateClosure(Node* node);
   Reduction ReduceJSCreateIterResultObject(Node* node);
   Reduction ReduceJSCreateKeyValueArray(Node* node);
   Reduction ReduceJSCreateLiteralArrayOrObject(Node* node);

src/deoptimizer.cc

@@ -573,8 +573,9 @@ int LookupCatchHandler(TranslatedFrame* translated_frame, int* data_out) {
   switch (translated_frame->kind()) {
     case TranslatedFrame::kInterpretedFunction: {
       int bytecode_offset = translated_frame->node_id().ToInt();
-      BytecodeArray* bytecode =
-          translated_frame->raw_shared_info()->bytecode_array();
+      JSFunction* function =
+          JSFunction::cast(translated_frame->begin()->GetRawValue());
+      BytecodeArray* bytecode = function->shared()->bytecode_array();
       HandlerTable* table = HandlerTable::cast(bytecode->handler_table());
       return table->LookupRange(bytecode_offset, data_out, nullptr);
     }
@@ -746,8 +747,7 @@ void Deoptimizer::DoComputeInterpretedFrame(TranslatedFrame* translated_frame,
   // add to the frame height here.
   if (is_topmost) height_in_bytes += kPointerSize;
 
-  TranslatedFrame::iterator function_iterator = value_iterator;
-  Object* function = value_iterator->GetRawValue();
+  JSFunction* function = JSFunction::cast(value_iterator->GetRawValue());
   value_iterator++;
   input_index++;
   if (trace_scope_ != NULL) {
@@ -887,12 +887,6 @@ void Deoptimizer::DoComputeInterpretedFrame(TranslatedFrame* translated_frame,
   output_offset -= kPointerSize;
   value = reinterpret_cast<intptr_t>(function);
   WriteValueToOutput(function, 0, frame_index, output_offset, "function    ");
-  if (function == isolate_->heap()->arguments_marker()) {
-    Address output_address =
-        reinterpret_cast<Address>(output_[frame_index]->GetTop()) +
-        output_offset;
-    values_to_materialize_.push_back({output_address, function_iterator});
-  }
 
   // Set the bytecode array pointer.
   output_offset -= kPointerSize;
@@ -1019,8 +1013,7 @@ void Deoptimizer::DoComputeArgumentsAdaptorFrame(
 
   unsigned height = translated_frame->height();
   unsigned height_in_bytes = height * kPointerSize;
-  TranslatedFrame::iterator function_iterator = value_iterator;
-  Object* function = value_iterator->GetRawValue();
+  JSFunction* function = JSFunction::cast(value_iterator->GetRawValue());
   value_iterator++;
   input_index++;
   if (trace_scope_ != NULL) {
@@ -1106,12 +1099,6 @@ void Deoptimizer::DoComputeArgumentsAdaptorFrame(
   output_offset -= kPointerSize;
   value = reinterpret_cast<intptr_t>(function);
   WriteValueToOutput(function, 0, frame_index, output_offset, "function    ");
-  if (function == isolate_->heap()->arguments_marker()) {
-    Address output_address =
-        reinterpret_cast<Address>(output_[frame_index]->GetTop()) +
-        output_offset;
-    values_to_materialize_.push_back({output_address, function_iterator});
-  }
 
   // Number of incoming arguments.
   output_offset -= kPointerSize;
@@ -3626,32 +3613,6 @@ Handle<Object> TranslatedState::MaterializeCapturedObjectAt(
       object->set_bound_arguments(FixedArray::cast(*bound_arguments));
       return object;
     }
-    case JS_FUNCTION_TYPE: {
-      Handle<JSFunction> object =
-          isolate_->factory()->NewFunctionFromSharedFunctionInfo(
-              handle(isolate_->object_function()->shared()),
-              handle(isolate_->context()), NOT_TENURED);
-      slot->value_ = object;
-      // We temporarily allocated a JSFunction for the {Object} function
-      // within the current context, to break cycles in the object graph.
-      // The correct function and context will be set below once available.
-      Handle<Object> properties = materializer.FieldAt(value_index);
-      Handle<Object> elements = materializer.FieldAt(value_index);
-      Handle<Object> prototype = materializer.FieldAt(value_index);
-      Handle<Object> shared = materializer.FieldAt(value_index);
-      Handle<Object> context = materializer.FieldAt(value_index);
-      Handle<Object> vector_cell = materializer.FieldAt(value_index);
-      Handle<Object> code = materializer.FieldAt(value_index);
-      object->set_map(*map);
-      object->set_raw_properties_or_hash(*properties);
-      object->set_elements(FixedArrayBase::cast(*elements));
-      object->set_prototype_or_initial_map(*prototype);
-      object->set_shared(SharedFunctionInfo::cast(*shared));
-      object->set_context(Context::cast(*context));
-      object->set_feedback_vector_cell(Cell::cast(*vector_cell));
-      object->ReplaceCode(Code::cast(*code));
-      return object;
-    }
     case JS_GENERATOR_OBJECT_TYPE: {
       Handle<JSGeneratorObject> object = Handle<JSGeneratorObject>::cast(
           isolate_->factory()->NewJSObjectFromMap(map, NOT_TENURED));
@@ -3813,6 +3774,7 @@ Handle<Object> TranslatedState::MaterializeCapturedObjectAt(
     case JS_API_OBJECT_TYPE:
     case JS_SPECIAL_API_OBJECT_TYPE:
     case JS_VALUE_TYPE:
+    case JS_FUNCTION_TYPE:
     case JS_MESSAGE_OBJECT_TYPE:
     case JS_DATE_TYPE:
     case JS_CONTEXT_EXTENSION_OBJECT_TYPE:
@@ -3946,19 +3908,6 @@ Handle<Object> TranslatedState::MaterializeObjectAt(int object_index) {
   return MaterializeAt(pos.frame_index_, &(pos.value_index_));
 }
 
-TranslatedFrame* TranslatedState::GetFrameFromJSFrameIndex(int jsframe_index) {
-  for (size_t i = 0; i < frames_.size(); i++) {
-    if (frames_[i].kind() == TranslatedFrame::kInterpretedFunction) {
-      if (jsframe_index > 0) {
-        jsframe_index--;
-      } else {
-        return &(frames_[i]);
-      }
-    }
-  }
-  return nullptr;
-}
-
 TranslatedFrame* TranslatedState::GetArgumentsInfoFromJSFrameIndex(
     int jsframe_index, int* args_count) {
   for (size_t i = 0; i < frames_.size(); i++) {

src/deoptimizer.h

@@ -265,7 +265,6 @@ class TranslatedState {
 
   std::vector<TranslatedFrame>& frames() { return frames_; }
 
-  TranslatedFrame* GetFrameFromJSFrameIndex(int jsframe_index);
   TranslatedFrame* GetArgumentsInfoFromJSFrameIndex(int jsframe_index,
                                                     int* arguments_count);
 

test/mjsunit/compiler/deopt-closure.js

-// Copyright 2017 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.
-
-// Flags: --allow-natives-syntax
-
-(function TestMaterializeTargetOfInterpretedFrame() {
-  function f(x) {
-    function g() {
-      %_DeoptimizeNow();
-      return x + 1;
-    }
-    return g();
-  }
-  assertEquals(24, f(23));
-  assertEquals(43, f(42));
-  %OptimizeFunctionOnNextCall(f);
-  assertEquals(66, f(65));
-})();
-
-(function TestMaterializeTargetOfArgumentsAdaptorFrame() {
-  function f(x) {
-    function g(a, b, c) {
-      %_DeoptimizeNow();
-      return x + 1;
-    }
-    return g();
-  }
-  assertEquals(24, f(23));
-  assertEquals(43, f(42));
-  %OptimizeFunctionOnNextCall(f);
-  assertEquals(66, f(65));
-})();
-
-(function TestMaterializeTargetOfConstructStubFrame() {
-  function f(x) {
-    function g() {
-      %_DeoptimizeNow();
-      this.val = x + 1;
-    }
-    return new g();
-  }
-  assertEquals({ val: 24 }, f(23));
-  assertEquals({ val: 43 }, f(42));
-  %OptimizeFunctionOnNextCall(f);
-  assertEquals({ val: 66 }, f(65));
-})();

test/mjsunit/compiler/function-caller.js

-// Copyright 2017 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.
-
-// Flags: --allow-natives-syntax
-
-(function TestInlineAllocatedCaller() {
-  function g() {
-    var caller = g.caller;
-    caller.foo = 23;
-    assertEquals(23, caller.foo);
-    assertEquals(23, g.caller.foo);
-    assertSame(caller, g.caller);
-  }
-  %NeverOptimizeFunction(g);
-
-  function f() {
-    (function caller() { g() })();
-  }
-
-  f();
-  f();
-  %OptimizeFunctionOnNextCall(f);
-  f();
-})();