[deoptimizer] Unify deoptimizer continuation builtins.

This switches all deoptimization events to go through use one single
continuation builtin (i.e. {Builtins::kNotifyDeoptimized}) instead of
handling builtin continuation specially. Fewer moving pieces.

R=jarin@chromium.org

Change-Id: Ic8a2316fa2f5c8717b4d50d1a619b87a38011564
Reviewed-on: https://chromium-review.googlesource.com/712156
Commit-Queue: Michael Starzinger <mstarzinger@chromium.org>
Reviewed-by: Jaroslav Sevcik <jarin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#48496}

src/builtins/arm/builtins-arm.cc

@@ -1503,19 +1503,6 @@ void Builtins::Generate_InstantiateAsmJs(MacroAssembler* masm) {
   __ Jump(r4);
 }
 
-void Builtins::Generate_NotifyBuiltinContinuation(MacroAssembler* masm) {
-  {
-    FrameAndConstantPoolScope scope(masm, StackFrame::INTERNAL);
-    // Preserve possible return result from lazy deopt.
-    __ push(r0);
-    // Pass the function and deoptimization type to the runtime system.
-    __ CallRuntime(Runtime::kNotifyStubFailure, false);
-    __ pop(r0);
-  }
-
-  __ mov(pc, lr);                         // Jump to ContinueToBuiltin stub
-}
-
 namespace {
 void Generate_ContinueToBuiltinHelper(MacroAssembler* masm,
                                       bool java_script_builtin,

src/builtins/arm64/builtins-arm64.cc

@@ -1507,21 +1507,6 @@ void Builtins::Generate_InstantiateAsmJs(MacroAssembler* masm) {
   __ Jump(x4);
 }
 
-void Builtins::Generate_NotifyBuiltinContinuation(MacroAssembler* masm) {
-  {
-    FrameScope scope(masm, StackFrame::INTERNAL);
-    // Preserve possible return result from lazy deopt.
-    __ Push(x0, padreg);
-    // Pass the function and deoptimization type to the runtime system.
-    __ CallRuntime(Runtime::kNotifyStubFailure, false);
-    __ Pop(padreg, x0);
-  }
-
-  // Jump to the ContinueToBuiltin stub. Deoptimizer::EntryGenerator::Generate
-  // loads this into lr before it jumps here.
-  __ Br(lr);
-}
-
 namespace {
 void Generate_ContinueToBuiltinHelper(MacroAssembler* masm,
                                       bool java_script_builtin,

src/builtins/builtins-definitions.h

@@ -124,7 +124,6 @@ namespace internal {
   ASM(DeserializeLazy)                                                         \
   ASM(InstantiateAsmJs)                                                        \
   ASM(NotifyDeoptimized)                                                       \
-  ASM(NotifyBuiltinContinuation)                                               \
                                                                                \
   /* Trampolines called when returning from a deoptimization that expects   */ \
   /* to continue in a JavaScript builtin to finish the functionality of a   */ \

src/builtins/ia32/builtins-ia32.cc

@@ -1551,20 +1551,6 @@ void Builtins::Generate_InstantiateAsmJs(MacroAssembler* masm) {
   __ jmp(ecx);
 }
 
-void Builtins::Generate_NotifyBuiltinContinuation(MacroAssembler* masm) {
-  // Enter an internal frame.
-  {
-    FrameScope scope(masm, StackFrame::INTERNAL);
-    // Preserve possible return result from lazy deopt.
-    __ push(eax);
-    __ CallRuntime(Runtime::kNotifyStubFailure, false);
-    __ pop(eax);
-    // Tear down internal frame.
-  }
-
-  __ Ret();  // Return to ContinueToBuiltin stub still on stack.
-}
-
 namespace {
 void Generate_ContinueToBuiltinHelper(MacroAssembler* masm,
                                       bool java_script_builtin,

src/builtins/mips/builtins-mips.cc

@@ -1478,19 +1478,6 @@ void Builtins::Generate_InstantiateAsmJs(MacroAssembler* masm) {
   __ Jump(t0, Code::kHeaderSize - kHeapObjectTag);
 }
 
-void Builtins::Generate_NotifyBuiltinContinuation(MacroAssembler* masm) {
-  {
-    FrameScope scope(masm, StackFrame::INTERNAL);
-    // Preserve possible return result from lazy deopt.
-    __ Push(v0);
-    // Pass the function and deoptimization type to the runtime system.
-    __ CallRuntime(Runtime::kNotifyStubFailure, false);
-    __ Pop(v0);
-  }
-
-  __ Jump(ra);                             // Jump to the ContinueToBuiltin stub
-}
-
 namespace {
 void Generate_ContinueToBuiltinHelper(MacroAssembler* masm,
                                       bool java_script_builtin,

src/builtins/mips64/builtins-mips64.cc

@@ -1482,19 +1482,6 @@ void Builtins::Generate_InstantiateAsmJs(MacroAssembler* masm) {
   __ Jump(t0);
 }
 
-void Builtins::Generate_NotifyBuiltinContinuation(MacroAssembler* masm) {
-  {
-    FrameScope scope(masm, StackFrame::INTERNAL);
-    // Preserve possible return result from lazy deopt.
-    __ push(v0);
-    // Pass the function and deoptimization type to the runtime system.
-    __ CallRuntime(Runtime::kNotifyStubFailure, false);
-    __ pop(v0);
-  }
-
-  __ Jump(ra);  // Jump to the ContinueToBuiltin stub
-}
-
 namespace {
 void Generate_ContinueToBuiltinHelper(MacroAssembler* masm,
                                       bool java_script_builtin,

src/builtins/ppc/builtins-ppc.cc

@@ -1520,19 +1520,6 @@ void Builtins::Generate_InstantiateAsmJs(MacroAssembler* masm) {
   __ JumpToJSEntry(ip);
 }
 
-void Builtins::Generate_NotifyBuiltinContinuation(MacroAssembler* masm) {
-  {
-    FrameAndConstantPoolScope scope(masm, StackFrame::INTERNAL);
-    // Preserve possible return result from lazy deopt.
-    __ push(r3);
-    // Pass the function and deoptimization type to the runtime system.
-    __ CallRuntime(Runtime::kNotifyStubFailure, false);
-    __ pop(r3);
-  }
-
-  __ blr();                                // Jump to ContinueToBuiltin stub
-}
-
 namespace {
 void Generate_ContinueToBuiltinHelper(MacroAssembler* masm,
                                       bool java_script_builtin,

src/builtins/s390/builtins-s390.cc

@@ -1515,19 +1515,6 @@ void Builtins::Generate_InstantiateAsmJs(MacroAssembler* masm) {
   __ JumpToJSEntry(ip);
 }
 
-void Builtins::Generate_NotifyBuiltinContinuation(MacroAssembler* masm) {
-  {
-    FrameAndConstantPoolScope scope(masm, StackFrame::INTERNAL);
-    // Preserve possible return result from lazy deopt.
-    __ push(r2);
-    // Pass the function and deoptimization type to the runtime system.
-    __ CallRuntime(Runtime::kNotifyStubFailure, false);
-    __ pop(r2);
-  }
-
-  __ Ret();                                // Jump to ContinueToBuiltin stub
-}
-
 namespace {
 void Generate_ContinueToBuiltinHelper(MacroAssembler* masm,
                                       bool java_script_builtin,

src/builtins/x64/builtins-x64.cc

@@ -1529,20 +1529,6 @@ void Builtins::Generate_InstantiateAsmJs(MacroAssembler* masm) {
   __ jmp(rcx);
 }
 
-void Builtins::Generate_NotifyBuiltinContinuation(MacroAssembler* masm) {
-  // Enter an internal frame.
-  {
-    FrameScope scope(masm, StackFrame::INTERNAL);
-    // Preserve possible return result from lazy deopt.
-    __ pushq(rax);
-    __ CallRuntime(Runtime::kNotifyStubFailure, false);
-    __ popq(rax);
-    // Tear down internal frame.
-  }
-
-  __ ret(0);  // Return to ContinueToBuiltin stub still on stack.
-}
-
 namespace {
 void Generate_ContinueToBuiltinHelper(MacroAssembler* masm,
                                       bool java_script_builtin,

src/deoptimizer.cc

@@ -407,6 +407,7 @@ Deoptimizer::Deoptimizer(Isolate* isolate, JSFunction* function,
       function_(function),
       bailout_id_(bailout_id),
       bailout_type_(type),
+      preserve_optimized_(false),
       from_(from),
       fp_to_sp_delta_(fp_to_sp_delta),
       deoptimizing_throw_(false),
@@ -1595,6 +1596,16 @@ void Deoptimizer::DoComputeBuiltinContinuation(
                       padding_slot_count) +
       BuiltinContinuationFrameConstants::kFixedFrameSize;
 
+  // If the builtins frame appears to be topmost we should ensure that the
+  // value of result register is preserved during continuation execution.
+  // We do this here by "pushing" the result of callback function to the
+  // top of the reconstructed stack and popping it in
+  // {Builtins::kNotifyDeoptimized}.
+  if (is_topmost) {
+    output_frame_size += kPointerSize;
+    if (PadTopOfStackRegister()) output_frame_size += kPointerSize;
+  }
+
   // Validate types of parameters. They must all be tagged except for argc for
   // JS builtins.
   bool has_argc = false;
@@ -1655,14 +1666,6 @@ void Deoptimizer::DoComputeBuiltinContinuation(
 
   intptr_t value;
 
-  Register result_reg = kReturnRegister0;
-  if (must_handle_result) {
-    value = input_->GetRegister(result_reg.code());
-  } else {
-    value = reinterpret_cast<intptr_t>(isolate()->heap()->undefined_value());
-  }
-  output_frame->SetRegister(result_reg.code(), value);
-
   int translated_stack_parameters =
       must_handle_result ? stack_param_count - 1 : stack_param_count;
 
@@ -1791,8 +1794,37 @@ void Deoptimizer::DoComputeBuiltinContinuation(
     }
   }
 
+  // Some architectures must pad the stack frame with extra stack slots
+  // to ensure the stack frame is aligned.
+  for (int i = 0; i < padding_slot_count; ++i) {
+    output_frame_offset -= kPointerSize;
+    WriteValueToOutput(isolate()->heap()->the_hole_value(), 0, frame_index,
+                       output_frame_offset, "padding ");
+  }
+
+  if (is_topmost) {
+    if (PadTopOfStackRegister()) {
+      output_frame_offset -= kPointerSize;
+      WriteValueToOutput(isolate()->heap()->the_hole_value(), 0, frame_index,
+                         output_frame_offset, "padding ");
+    }
+    // Ensure the result is restored back when we return to the stub.
+    output_frame_offset -= kPointerSize;
+    Register result_reg = kReturnRegister0;
+    if (must_handle_result) {
+      value = input_->GetRegister(result_reg.code());
+    } else {
+      value = reinterpret_cast<intptr_t>(isolate()->heap()->undefined_value());
+    }
+    output_frame->SetFrameSlot(output_frame_offset, value);
+    DebugPrintOutputSlot(value, frame_index, output_frame_offset,
+                         "callback result\n");
+  }
+
+  CHECK_EQ(0u, output_frame_offset);
+
   // Clear the context register. The context might be a de-materialized object
-  // and will be materialized by {Runtime_NotifyStubFailure}. For additional
+  // and will be materialized by {Runtime_NotifyDeoptimized}. For additional
   // safety we use Smi(0) instead of the potential {arguments_marker} here.
   if (is_topmost) {
     intptr_t context_value = reinterpret_cast<intptr_t>(Smi::kZero);
@@ -1800,19 +1832,18 @@ void Deoptimizer::DoComputeBuiltinContinuation(
     output_frame->SetRegister(context_reg.code(), context_value);
   }
 
+  // TODO(6898): For eager deopts within builtin stub frames we currently skip
+  // marking the underlying function as deoptimized. This is to avoid deopt
+  // loops where we would just generate the same optimized code all over again.
+  if (is_topmost && bailout_type_ != LAZY) {
+    preserve_optimized_ = true;
+  }
+
   // Ensure the frame pointer register points to the callee's frame. The builtin
   // will build its own frame once we continue to it.
   Register fp_reg = JavaScriptFrame::fp_register();
   output_frame->SetRegister(fp_reg.code(), output_[frame_index - 1]->GetFp());
 
-  // Some architectures must pad the stack frame with extra stack slots
-  // to ensure the stack frame is aligned.
-  for (int i = 0; i < padding_slot_count; ++i) {
-    output_frame_offset -= kPointerSize;
-    WriteValueToOutput(isolate()->heap()->the_hole_value(), 0, frame_index,
-                       output_frame_offset, "padding ");
-  }
-
   Code* continue_to_builtin =
       java_script_builtin
           ? (must_handle_result
@@ -1829,7 +1860,7 @@ void Deoptimizer::DoComputeBuiltinContinuation(
       reinterpret_cast<intptr_t>(continue_to_builtin->instruction_start()));
 
   Code* continuation =
-      isolate()->builtins()->builtin(Builtins::kNotifyBuiltinContinuation);
+      isolate()->builtins()->builtin(Builtins::kNotifyDeoptimized);
   output_frame->SetContinuation(
       reinterpret_cast<intptr_t>(continuation->entry()));
 }

src/deoptimizer.h

@@ -371,6 +371,7 @@ class Deoptimizer : public Malloced {
   Handle<JSFunction> function() const;
   Handle<Code> compiled_code() const;
   BailoutType bailout_type() const { return bailout_type_; }
+  bool preserve_optimized() const { return preserve_optimized_; }
 
   // Number of created JS frames. Not all created frames are necessarily JS.
   int jsframe_count() const { return jsframe_count_; }
@@ -536,6 +537,7 @@ class Deoptimizer : public Malloced {
   Code* compiled_code_;
   unsigned bailout_id_;
   BailoutType bailout_type_;
+  bool preserve_optimized_;
   Address from_;
   int fp_to_sp_delta_;
   bool deoptimizing_throw_;

src/runtime/runtime-compiler.cc

@@ -128,12 +128,21 @@ RUNTIME_FUNCTION(Runtime_InstantiateAsmJs) {
   return Smi::kZero;
 }
 
-namespace {
-
-void MaterializeHeapObjectsAndDeleteDeoptimizer(Isolate* isolate,
-                                                Deoptimizer* deoptimizer) {
+RUNTIME_FUNCTION(Runtime_NotifyDeoptimized) {
+  HandleScope scope(isolate);
+  DCHECK_EQ(0, args.length());
+  Deoptimizer* deoptimizer = Deoptimizer::Grab(isolate);
+  DCHECK(deoptimizer->compiled_code()->kind() == Code::OPTIMIZED_FUNCTION);
+  DCHECK(deoptimizer->compiled_code()->is_turbofanned());
   DCHECK(AllowHeapAllocation::IsAllowed());
   DCHECK_NULL(isolate->context());
+
+  TimerEventScope<TimerEventDeoptimizeCode> timer(isolate);
+  TRACE_EVENT0("v8", "V8.DeoptimizeCode");
+  Handle<JSFunction> function = deoptimizer->function();
+  Deoptimizer::BailoutType type = deoptimizer->bailout_type();
+  bool preserve_optimized_code = deoptimizer->preserve_optimized();
+
   // TODO(turbofan): We currently need the native context to materialize
   // the arguments object, but only to get to its map.
   isolate->set_context(deoptimizer->function()->native_context());
@@ -146,39 +155,9 @@ void MaterializeHeapObjectsAndDeleteDeoptimizer(Isolate* isolate,
   JavaScriptFrameIterator top_it(isolate);
   JavaScriptFrame* top_frame = top_it.frame();
   isolate->set_context(Context::cast(top_frame->context()));
-}
-
-}  // namespace
-
-RUNTIME_FUNCTION(Runtime_NotifyStubFailure) {
-  HandleScope scope(isolate);
-  DCHECK_EQ(0, args.length());
-  Deoptimizer* deoptimizer = Deoptimizer::Grab(isolate);
-  DCHECK(deoptimizer->compiled_code()->kind() == Code::OPTIMIZED_FUNCTION);
-  DCHECK(deoptimizer->compiled_code()->is_turbofanned());
-  MaterializeHeapObjectsAndDeleteDeoptimizer(isolate, deoptimizer);
-  return isolate->heap()->undefined_value();
-}
-
-RUNTIME_FUNCTION(Runtime_NotifyDeoptimized) {
-  HandleScope scope(isolate);
-  DCHECK_EQ(0, args.length());
-  Deoptimizer* deoptimizer = Deoptimizer::Grab(isolate);
-  DCHECK(deoptimizer->compiled_code()->kind() == Code::OPTIMIZED_FUNCTION);
-  DCHECK(deoptimizer->compiled_code()->is_turbofanned());
-
-  TimerEventScope<TimerEventDeoptimizeCode> timer(isolate);
-  TRACE_EVENT0("v8", "V8.DeoptimizeCode");
-  Handle<JSFunction> function = deoptimizer->function();
-  Deoptimizer::BailoutType type = deoptimizer->bailout_type();
-
-  MaterializeHeapObjectsAndDeleteDeoptimizer(isolate, deoptimizer);
 
-  // TODO(mstarzinger): The marking of the function for deoptimization is the
-  // only difference to {Runtime_NotifyStubFailure} by now and we should also
-  // do this if the top-most frame is a builtin stub to avoid deoptimization
-  // loops. This would also unify the two runtime functions.
-  if (type != Deoptimizer::LAZY) {
+  // Invalidate the underlying optimized code on non-lazy deopts.
+  if (type != Deoptimizer::LAZY && !preserve_optimized_code) {
     Deoptimizer::DeoptimizeFunction(*function);
   }
 

src/runtime/runtime.h

@@ -118,7 +118,6 @@ namespace internal {
   F(CompileOptimized_Concurrent, 1, 1)    \
   F(CompileOptimized_NotConcurrent, 1, 1) \
   F(EvictOptimizedCodeSlot, 1, 1)         \
-  F(NotifyStubFailure, 0, 1)              \
   F(NotifyDeoptimized, 0, 1)              \
   F(CompileForOnStackReplacement, 1, 1)   \
   F(TryInstallOptimizedCode, 1, 1)        \