Fix deoptmization of inlined TF instanceOf to call ToBoolean

This CL leverages and extends the deopt-to-stub mechanisms previously
introduced to support deopting from CSA-built builtins (e.g. Array.prototype.forEach).

BUG=v8:6373
LOG=N

Review-Url: https://codereview.chromium.org/2890363002
Cr-Commit-Position: refs/heads/master@{#46144}

src/builtins/builtins-conversion-gen.cc

@@ -259,6 +259,22 @@ TF_BUILTIN(ToBoolean, CodeStubAssembler) {
   Return(BooleanConstant(false));
 }
 
+// ES6 section 7.1.2 ToBoolean ( argument )
+// Requires parameter on stack so that it can be used as a continuation from a
+// LAZY deopt.
+TF_BUILTIN(ToBooleanLazyDeoptContinuation, CodeStubAssembler) {
+  Node* value = Parameter(Descriptor::kArgument);
+
+  Label return_true(this), return_false(this);
+  BranchIfToBooleanIsTrue(value, &return_true, &return_false);
+
+  BIND(&return_true);
+  Return(BooleanConstant(true));
+
+  BIND(&return_false);
+  Return(BooleanConstant(false));
+}
+
 TF_BUILTIN(ToLength, CodeStubAssembler) {
   Node* context = Parameter(Descriptor::kContext);
 

src/builtins/builtins-definitions.h

@@ -228,6 +228,9 @@ namespace internal {
   TFC(Typeof, Typeof, 1)                                                       \
   TFC(GetSuperConstructor, Typeof, 1)                                          \
                                                                                \
+  /* Type conversions continuations */                                         \
+  TFC(ToBooleanLazyDeoptContinuation, TypeConversionStackParameter, 1)         \
+                                                                               \
   /* Handlers */                                                               \
   TFH(LoadICProtoArray, BUILTIN, kNoExtraICState, LoadICProtoArray)            \
   TFH(LoadICProtoArrayThrowIfNonexistent, BUILTIN, kNoExtraICState,            \

src/compiler/frame-states.cc

@@ -110,11 +110,11 @@ Node* CreateBuiltinContinuationFrameStateCommon(
   FrameStateType frame_type =
       function.is_null() ? FrameStateType::kBuiltinContinuation
                          : FrameStateType::kJavaScriptBuiltinContinuation;
-  Handle<SharedFunctionInfo> shared(
-      Handle<SharedFunctionInfo>(function->shared()));
   const FrameStateFunctionInfo* state_info =
-      common->CreateFrameStateFunctionInfo(frame_type, parameter_count, 0,
-                                           shared);
+      common->CreateFrameStateFunctionInfo(
+          frame_type, parameter_count, 0,
+          function.is_null() ? Handle<SharedFunctionInfo>()
+                             : Handle<SharedFunctionInfo>(function->shared()));
   const Operator* op = common->FrameState(
       bailout_id, OutputFrameStateCombine::Ignore(), state_info);
 
@@ -140,8 +140,12 @@ Node* CreateStubBuiltinContinuationFrameState(JSGraph* js_graph,
   CallInterfaceDescriptor descriptor = callable.descriptor();
 
   std::vector<Node*> actual_parameters;
-  // Stack parameters first
-  for (int i = 0; i < descriptor.GetStackParameterCount(); ++i) {
+  // Stack parameters first. If the deoptimization is LAZY, the final parameter
+  // is added by the deoptimizer and isn't explicitly passed in the frame state.
+  int stack_parameter_count =
+      descriptor.GetRegisterParameterCount() -
+      (mode == ContinuationFrameStateMode::LAZY ? 1 : 0);
+  for (int i = 0; i < stack_parameter_count; ++i) {
     actual_parameters.push_back(
         parameters[descriptor.GetRegisterParameterCount() + i]);
   }
@@ -152,7 +156,7 @@ Node* CreateStubBuiltinContinuationFrameState(JSGraph* js_graph,
   }
 
   return CreateBuiltinContinuationFrameStateCommon(
-      js_graph, name, context, &actual_parameters[0],
+      js_graph, name, context, actual_parameters.data(),
       static_cast<int>(actual_parameters.size()), outer_frame_state,
       Handle<JSFunction>());
 }

src/compiler/js-native-context-specialization.cc

@@ -161,6 +161,7 @@ Reduction JSNativeContextSpecialization::ReduceJSInstanceOf(Node* node) {
   Node* constructor = NodeProperties::GetValueInput(node, 1);
   Node* context = NodeProperties::GetContextInput(node);
   Node* effect = NodeProperties::GetEffectInput(node);
+  Node* frame_state = NodeProperties::GetFrameStateInput(node);
   Node* control = NodeProperties::GetControlInput(node);
 
   // Check if the right hand side is a known {receiver}.
@@ -234,11 +235,22 @@ Reduction JSNativeContextSpecialization::ReduceJSInstanceOf(Node* node) {
     access_builder.BuildCheckMaps(constructor, &effect, control,
                                   access_info.receiver_maps());
 
+    // Create a nested frame state inside the current method's most-recent frame
+    // state that will ensure that deopts that happen after this point will not
+    // fallback to the last Checkpoint--which would completely re-execute the
+    // instanceof logic--but rather create an activation of a version of the
+    // ToBoolean stub that finishes the remaining work of instanceof and returns
+    // to the caller without duplicating side-effects upon a lazy deopt.
+    Node* continuation_frame_state = CreateStubBuiltinContinuationFrameState(
+        jsgraph(), Builtins::kToBooleanLazyDeoptContinuation, context, nullptr,
+        0, frame_state, ContinuationFrameStateMode::LAZY);
+
     // Call the @@hasInstance handler.
     Node* target = jsgraph()->Constant(constant);
     node->InsertInput(graph()->zone(), 0, target);
     node->ReplaceInput(1, constructor);
     node->ReplaceInput(2, object);
+    node->ReplaceInput(4, continuation_frame_state);
     node->ReplaceInput(5, effect);
     NodeProperties::ChangeOp(
         node, javascript()->Call(3, CallFrequency(), VectorSlotPair(),

src/deoptimizer.cc

@@ -1981,8 +1981,11 @@ void Deoptimizer::DoComputeBuiltinContinuation(
 
   if (trace_scope_ != NULL) {
     PrintF(trace_scope_->file(),
-           "  translating BuiltinContinuation to %s, stack param count %d\n",
-           Builtins::name(builtin_name), stack_param_count);
+           "  translating BuiltinContinuation to %s,"
+           " register param count %d,"
+           " stack param count %d\n",
+           Builtins::name(builtin_name), register_parameter_count,
+           stack_param_count);
   }
 
   unsigned output_frame_offset = output_frame_size;
@@ -3203,7 +3206,6 @@ TranslatedFrame TranslatedFrame::ConstructStubFrame(
 
 TranslatedFrame TranslatedFrame::BuiltinContinuationFrame(
     BailoutId bailout_id, SharedFunctionInfo* shared_info, int height) {
-  base::OS::DebugBreak();
   TranslatedFrame frame(kBuiltinContinuation, shared_info->GetIsolate(),
                         shared_info, height);
   frame.node_id_ = bailout_id;
@@ -3337,8 +3339,11 @@ TranslatedFrame TranslatedState::CreateNextTranslatedFrame(
         PrintF(trace_file, " => bailout_id=%d, height=%d; inputs:\n",
                bailout_id.ToInt(), height);
       }
+      // Add one to the height to account for the context which was implicitly
+      // added to the translation during code generation.
+      int height_with_context = height + 1;
       return TranslatedFrame::BuiltinContinuationFrame(bailout_id, shared_info,
-                                                       height);
+                                                       height_with_context);
     }
 
     case Translation::JAVA_SCRIPT_BUILTIN_CONTINUATION_FRAME: {
@@ -3353,8 +3358,11 @@ TranslatedFrame TranslatedState::CreateNextTranslatedFrame(
         PrintF(trace_file, " => bailout_id=%d, height=%d; inputs:\n",
                bailout_id.ToInt(), height);
       }
+      // Add one to the height to account for the context which was implicitly
+      // added to the translation during code generation.
+      int height_with_context = height + 1;
       return TranslatedFrame::JavaScriptBuiltinContinuationFrame(
-          bailout_id, shared_info, height + 1);
+          bailout_id, shared_info, height_with_context);
     }
 
     case Translation::GETTER_STUB_FRAME: {

src/interface-descriptors.cc

@@ -278,6 +278,16 @@ void TypeConversionDescriptor::InitializePlatformSpecific(
   data->InitializePlatformSpecific(arraysize(registers), registers);
 }
 
+void TypeConversionStackParameterDescriptor::InitializePlatformSpecific(
+    CallInterfaceDescriptorData* data) {
+  data->InitializePlatformSpecific(0, nullptr);
+}
+
+void TypeConversionStackParameterDescriptor::InitializePlatformIndependent(
+    CallInterfaceDescriptorData* data) {
+  data->InitializePlatformIndependent(data->register_param_count(), 1, NULL);
+}
+
 void MathPowTaggedDescriptor::InitializePlatformSpecific(
     CallInterfaceDescriptorData* data) {
   Register registers[] = {exponent()};

src/interface-descriptors.h

@@ -35,6 +35,7 @@ class PlatformInterfaceDescriptor;
   V(FastNewObject)                         \
   V(FastNewArguments)                      \
   V(TypeConversion)                        \
+  V(TypeConversionStackParameter)          \
   V(Typeof)                                \
   V(FastCloneRegExp)                       \
   V(FastCloneShallowArray)                 \
@@ -513,6 +514,14 @@ class TypeConversionDescriptor final : public CallInterfaceDescriptor {
   static const Register ArgumentRegister();
 };
 
+class TypeConversionStackParameterDescriptor final
+    : public CallInterfaceDescriptor {
+ public:
+  DEFINE_PARAMETERS(kArgument)
+  DECLARE_DESCRIPTOR_WITH_CUSTOM_FUNCTION_TYPE(
+      TypeConversionStackParameterDescriptor, CallInterfaceDescriptor)
+};
+
 class ForInPrepareDescriptor final : public CallInterfaceDescriptor {
  public:
   DEFINE_PARAMETERS(kObject)

test/mjsunit/regress/regress-6373.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
+
+var A = {}
+
+A[Symbol.hasInstance] = function(x) {
+  %DeoptimizeFunction(foo);
+  return 1;
+}
+
+var a = {}
+
+function foo(o) {
+  return o instanceof A;
+}
+
+foo(a);
+foo(a);
+assertTrue(foo(a) !== 1);
+%OptimizeFunctionOnNextCall(foo);
+assertTrue(foo(a) !== 1);