[compiler] Remove --ignition-preserve-bytecode flag.

This removes the deprecated flag in question which has been enabled by
default a while ago. All components can by now deal with activations of
a single function being mixed between Ignition and other compilers. The
maintenance overhead to support a mode that clears bytecode is no longer
warranted.

R=rmcilroy@chromium.org
BUG=v8:4280

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

src/builtins/arm/builtins-arm.cc

@@ -1146,31 +1146,6 @@ void Builtins::Generate_InterpreterEntryTrampoline(MacroAssembler* masm) {
   __ Jump(r4);
 }
 
-void Builtins::Generate_InterpreterMarkBaselineOnReturn(MacroAssembler* masm) {
-  // Save the function and context for call to CompileBaseline.
-  __ ldr(r1, MemOperand(fp, StandardFrameConstants::kFunctionOffset));
-  __ ldr(kContextRegister,
-         MemOperand(fp, StandardFrameConstants::kContextOffset));
-
-  // Leave the frame before recompiling for baseline so that we don't count as
-  // an activation on the stack.
-  LeaveInterpreterFrame(masm, r2);
-
-  {
-    FrameScope frame_scope(masm, StackFrame::INTERNAL);
-    // Push return value.
-    __ push(r0);
-
-    // Push function as argument and compile for baseline.
-    __ push(r1);
-    __ CallRuntime(Runtime::kCompileBaseline);
-
-    // Restore return value.
-    __ pop(r0);
-  }
-  __ Jump(lr);
-}
-
 static void Generate_StackOverflowCheck(MacroAssembler* masm, Register num_args,
                                         Register scratch,
                                         Label* stack_overflow) {

src/builtins/arm64/builtins-arm64.cc

@@ -1154,31 +1154,6 @@ void Builtins::Generate_InterpreterEntryTrampoline(MacroAssembler* masm) {
   __ Jump(x7);
 }
 
-void Builtins::Generate_InterpreterMarkBaselineOnReturn(MacroAssembler* masm) {
-  // Save the function and context for call to CompileBaseline.
-  __ ldr(x1, MemOperand(fp, StandardFrameConstants::kFunctionOffset));
-  __ ldr(kContextRegister,
-         MemOperand(fp, StandardFrameConstants::kContextOffset));
-
-  // Leave the frame before recompiling for baseline so that we don't count as
-  // an activation on the stack.
-  LeaveInterpreterFrame(masm, x2);
-
-  {
-    FrameScope frame_scope(masm, StackFrame::INTERNAL);
-    // Push return value.
-    __ push(x0);
-
-    // Push function as argument and compile for baseline.
-    __ push(x1);
-    __ CallRuntime(Runtime::kCompileBaseline);
-
-    // Restore return value.
-    __ pop(x0);
-  }
-  __ Ret();
-}
-
 static void Generate_StackOverflowCheck(MacroAssembler* masm, Register num_args,
                                         Register scratch,
                                         Label* stack_overflow) {

src/builtins/builtins.h

@@ -109,7 +109,6 @@ namespace internal {
                                                                               \
   /* Interpreter */                                                           \
   ASM(InterpreterEntryTrampoline)                                             \
-  ASM(InterpreterMarkBaselineOnReturn)                                        \
   ASM(InterpreterPushArgsAndCall)                                             \
   ASM(InterpreterPushArgsAndCallFunction)                                     \
   ASM(InterpreterPushArgsAndTailCall)                                         \

src/builtins/ia32/builtins-ia32.cc

@@ -685,31 +685,6 @@ void Builtins::Generate_InterpreterEntryTrampoline(MacroAssembler* masm) {
   __ jmp(ecx);
 }
 
-void Builtins::Generate_InterpreterMarkBaselineOnReturn(MacroAssembler* masm) {
-  // Save the function and context for call to CompileBaseline.
-  __ mov(edi, Operand(ebp, StandardFrameConstants::kFunctionOffset));
-  __ mov(kContextRegister,
-         Operand(ebp, StandardFrameConstants::kContextOffset));
-
-  // Leave the frame before recompiling for baseline so that we don't count as
-  // an activation on the stack.
-  LeaveInterpreterFrame(masm, ebx, ecx);
-
-  {
-    FrameScope frame_scope(masm, StackFrame::INTERNAL);
-    // Push return value.
-    __ push(eax);
-
-    // Push function as argument and compile for baseline.
-    __ push(edi);
-    __ CallRuntime(Runtime::kCompileBaseline);
-
-    // Restore return value.
-    __ pop(eax);
-  }
-  __ ret(0);
-}
-
 static void Generate_StackOverflowCheck(MacroAssembler* masm, Register num_args,
                                         Register scratch1, Register scratch2,
                                         Label* stack_overflow,

src/builtins/mips/builtins-mips.cc

@@ -1145,31 +1145,6 @@ void Builtins::Generate_InterpreterEntryTrampoline(MacroAssembler* masm) {
   __ Jump(t0);
 }
 
-void Builtins::Generate_InterpreterMarkBaselineOnReturn(MacroAssembler* masm) {
-  // Save the function and context for call to CompileBaseline.
-  __ lw(a1, MemOperand(fp, StandardFrameConstants::kFunctionOffset));
-  __ lw(kContextRegister,
-        MemOperand(fp, StandardFrameConstants::kContextOffset));
-
-  // Leave the frame before recompiling for baseline so that we don't count as
-  // an activation on the stack.
-  LeaveInterpreterFrame(masm, t0);
-
-  {
-    FrameScope frame_scope(masm, StackFrame::INTERNAL);
-    // Push return value.
-    __ push(v0);
-
-    // Push function as argument and compile for baseline.
-    __ push(a1);
-    __ CallRuntime(Runtime::kCompileBaseline);
-
-    // Restore return value.
-    __ pop(v0);
-  }
-  __ Jump(ra);
-}
-
 static void Generate_StackOverflowCheck(MacroAssembler* masm, Register num_args,
                                         Register scratch1, Register scratch2,
                                         Label* stack_overflow) {

src/builtins/mips64/builtins-mips64.cc

@@ -1137,31 +1137,6 @@ void Builtins::Generate_InterpreterEntryTrampoline(MacroAssembler* masm) {
   __ Jump(a4);
 }
 
-void Builtins::Generate_InterpreterMarkBaselineOnReturn(MacroAssembler* masm) {
-  // Save the function and context for call to CompileBaseline.
-  __ ld(a1, MemOperand(fp, StandardFrameConstants::kFunctionOffset));
-  __ ld(kContextRegister,
-        MemOperand(fp, StandardFrameConstants::kContextOffset));
-
-  // Leave the frame before recompiling for baseline so that we don't count as
-  // an activation on the stack.
-  LeaveInterpreterFrame(masm, t0);
-
-  {
-    FrameScope frame_scope(masm, StackFrame::INTERNAL);
-    // Push return value.
-    __ push(v0);
-
-    // Push function as argument and compile for baseline.
-    __ push(a1);
-    __ CallRuntime(Runtime::kCompileBaseline);
-
-    // Restore return value.
-    __ pop(v0);
-  }
-  __ Jump(ra);
-}
-
 static void Generate_StackOverflowCheck(MacroAssembler* masm, Register num_args,
                                         Register scratch1, Register scratch2,
                                         Label* stack_overflow) {

src/builtins/ppc/builtins-ppc.cc

@@ -1173,31 +1173,6 @@ void Builtins::Generate_InterpreterEntryTrampoline(MacroAssembler* masm) {
   __ JumpToJSEntry(r7);
 }
 
-void Builtins::Generate_InterpreterMarkBaselineOnReturn(MacroAssembler* masm) {
-  // Save the function and context for call to CompileBaseline.
-  __ LoadP(r4, MemOperand(fp, StandardFrameConstants::kFunctionOffset));
-  __ LoadP(kContextRegister,
-           MemOperand(fp, StandardFrameConstants::kContextOffset));
-
-  // Leave the frame before recompiling for baseline so that we don't count as
-  // an activation on the stack.
-  LeaveInterpreterFrame(masm, r5);
-
-  {
-    FrameScope frame_scope(masm, StackFrame::INTERNAL);
-    // Push return value.
-    __ push(r3);
-
-    // Push function as argument and compile for baseline.
-    __ push(r4);
-    __ CallRuntime(Runtime::kCompileBaseline);
-
-    // Restore return value.
-    __ pop(r3);
-  }
-  __ blr();
-}
-
 static void Generate_StackOverflowCheck(MacroAssembler* masm, Register num_args,
                                         Register scratch,
                                         Label* stack_overflow) {

src/builtins/s390/builtins-s390.cc

@@ -1176,31 +1176,6 @@ void Builtins::Generate_InterpreterEntryTrampoline(MacroAssembler* masm) {
   __ JumpToJSEntry(r6);
 }
 
-void Builtins::Generate_InterpreterMarkBaselineOnReturn(MacroAssembler* masm) {
-  // Save the function and context for call to CompileBaseline.
-  __ LoadP(r3, MemOperand(fp, StandardFrameConstants::kFunctionOffset));
-  __ LoadP(kContextRegister,
-           MemOperand(fp, StandardFrameConstants::kContextOffset));
-
-  // Leave the frame before recompiling for baseline so that we don't count as
-  // an activation on the stack.
-  LeaveInterpreterFrame(masm, r4);
-
-  {
-    FrameScope frame_scope(masm, StackFrame::INTERNAL);
-    // Push return value.
-    __ push(r2);
-
-    // Push function as argument and compile for baseline.
-    __ push(r3);
-    __ CallRuntime(Runtime::kCompileBaseline);
-
-    // Restore return value.
-    __ pop(r2);
-  }
-  __ Ret();
-}
-
 static void Generate_StackOverflowCheck(MacroAssembler* masm, Register num_args,
                                         Register scratch,
                                         Label* stack_overflow) {

src/builtins/x64/builtins-x64.cc

@@ -766,31 +766,6 @@ void Builtins::Generate_InterpreterEntryTrampoline(MacroAssembler* masm) {
   __ jmp(rcx);
 }
 
-void Builtins::Generate_InterpreterMarkBaselineOnReturn(MacroAssembler* masm) {
-  // Save the function and context for call to CompileBaseline.
-  __ movp(rdi, Operand(rbp, StandardFrameConstants::kFunctionOffset));
-  __ movp(kContextRegister,
-          Operand(rbp, StandardFrameConstants::kContextOffset));
-
-  // Leave the frame before recompiling for baseline so that we don't count as
-  // an activation on the stack.
-  LeaveInterpreterFrame(masm, rbx, rcx);
-
-  {
-    FrameScope frame_scope(masm, StackFrame::INTERNAL);
-    // Push return value.
-    __ Push(rax);
-
-    // Push function as argument and compile for baseline.
-    __ Push(rdi);
-    __ CallRuntime(Runtime::kCompileBaseline);
-
-    // Restore return value.
-    __ Pop(rax);
-  }
-  __ ret(0);
-}
-
 static void Generate_StackOverflowCheck(MacroAssembler* masm, Register num_args,
                                         Register scratch1, Register scratch2,
                                         Label* stack_overflow) {

src/builtins/x87/builtins-x87.cc

@@ -686,31 +686,6 @@ void Builtins::Generate_InterpreterEntryTrampoline(MacroAssembler* masm) {
   __ jmp(ecx);
 }
 
-void Builtins::Generate_InterpreterMarkBaselineOnReturn(MacroAssembler* masm) {
-  // Save the function and context for call to CompileBaseline.
-  __ mov(edi, Operand(ebp, StandardFrameConstants::kFunctionOffset));
-  __ mov(kContextRegister,
-         Operand(ebp, StandardFrameConstants::kContextOffset));
-
-  // Leave the frame before recompiling for baseline so that we don't count as
-  // an activation on the stack.
-  LeaveInterpreterFrame(masm, ebx, ecx);
-
-  {
-    FrameScope frame_scope(masm, StackFrame::INTERNAL);
-    // Push return value.
-    __ push(eax);
-
-    // Push function as argument and compile for baseline.
-    __ push(edi);
-    __ CallRuntime(Runtime::kCompileBaseline);
-
-    // Restore return value.
-    __ pop(eax);
-  }
-  __ ret(0);
-}
-
 static void Generate_StackOverflowCheck(MacroAssembler* masm, Register num_args,
                                         Register scratch1, Register scratch2,
                                         Label* stack_overflow,

src/compiler.cc

@@ -20,7 +20,6 @@
 #include "src/crankshaft/hydrogen.h"
 #include "src/debug/debug.h"
 #include "src/debug/liveedit.h"
-#include "src/deoptimizer.h"
 #include "src/frames-inl.h"
 #include "src/full-codegen/full-codegen.h"
 #include "src/globals.h"
@@ -778,72 +777,6 @@ CompilationJob::Status FinalizeOptimizedCompilationJob(CompilationJob* job) {
   return CompilationJob::FAILED;
 }
 
-class InterpreterActivationsFinder : public ThreadVisitor,
-                                     public OptimizedFunctionVisitor {
- public:
-  explicit InterpreterActivationsFinder(SharedFunctionInfo* shared)
-      : shared_(shared), has_activations_(false) {}
-
-  void VisitThread(Isolate* isolate, ThreadLocalTop* top) {
-    Address* activation_pc_address = nullptr;
-    JavaScriptFrameIterator it(isolate, top);
-    for (; !it.done(); it.Advance()) {
-      JavaScriptFrame* frame = it.frame();
-      if (FLAG_ignition_osr && frame->is_optimized() &&
-          frame->function()->shared() == shared_) {
-        // There might be optimized OSR code active on the stack that is not
-        // reachable through a function. We count this as an activation.
-        has_activations_ = true;
-      }
-      if (frame->is_interpreted() && frame->function()->shared() == shared_) {
-        has_activations_ = true;
-        activation_pc_address = frame->pc_address();
-      }
-    }
-
-    if (activation_pc_address) {
-      activation_pc_addresses_.push_back(activation_pc_address);
-    }
-  }
-
-  void VisitFunction(JSFunction* function) {
-    if (function->Inlines(shared_)) has_activations_ = true;
-  }
-
-  void EnterContext(Context* context) {}
-  void LeaveContext(Context* context) {}
-
-  bool MarkActivationsForBaselineOnReturn(Isolate* isolate) {
-    if (activation_pc_addresses_.empty()) return false;
-
-    for (Address* activation_pc_address : activation_pc_addresses_) {
-      DCHECK(isolate->inner_pointer_to_code_cache()
-                 ->GetCacheEntry(*activation_pc_address)
-                 ->code->is_interpreter_trampoline_builtin());
-      *activation_pc_address =
-          isolate->builtins()->InterpreterMarkBaselineOnReturn()->entry();
-    }
-    return true;
-  }
-
-  bool has_activations() { return has_activations_; }
-
- private:
-  SharedFunctionInfo* shared_;
-  bool has_activations_;
-  std::vector<Address*> activation_pc_addresses_;
-};
-
-bool HasInterpreterActivations(
-    Isolate* isolate, InterpreterActivationsFinder* activations_finder) {
-  activations_finder->VisitThread(isolate, isolate->thread_local_top());
-  isolate->thread_manager()->IterateArchivedThreads(activations_finder);
-  // There might be optimized functions that rely on bytecode being around. We
-  // need to prevent switching the given function to baseline code.
-  Deoptimizer::VisitAllOptimizedFunctions(isolate, activations_finder);
-  return activations_finder->has_activations();
-}
-
 MaybeHandle<Code> GetBaselineCode(Handle<JSFunction> function) {
   Isolate* isolate = function->GetIsolate();
   VMState<COMPILER> state(isolate);
@@ -879,31 +812,6 @@ MaybeHandle<Code> GetBaselineCode(Handle<JSFunction> function) {
     return MaybeHandle<Code>();
   }
 
-  // TODO(4280): For now we disable switching to baseline code in the presence
-  // of interpreter activations of the given function. The reasons is that the
-  // underlying bytecode is cleared below. Note that this only applies in case
-  // the --ignition-preserve-bytecode flag is not passed.
-  if (!FLAG_ignition_preserve_bytecode) {
-    InterpreterActivationsFinder activations_finder(function->shared());
-    if (HasInterpreterActivations(isolate, &activations_finder)) {
-      if (FLAG_trace_opt) {
-        OFStream os(stdout);
-        os << "[unable to switch " << Brief(*function) << " due to activations]"
-           << std::endl;
-      }
-
-      if (activations_finder.MarkActivationsForBaselineOnReturn(isolate)) {
-        if (FLAG_trace_opt) {
-          OFStream os(stdout);
-          os << "[marking " << Brief(function->shared())
-             << " for baseline recompilation on return]" << std::endl;
-        }
-      }
-
-      return MaybeHandle<Code>();
-    }
-  }
-
   if (FLAG_trace_opt) {
     OFStream os(stdout);
     os << "[switching method " << Brief(*function) << " to baseline code]"
@@ -922,12 +830,6 @@ MaybeHandle<Code> GetBaselineCode(Handle<JSFunction> function) {
     return MaybeHandle<Code>();
   }
 
-  // TODO(4280): For now we play it safe and remove the bytecode array when we
-  // switch to baseline code. We might consider keeping around the bytecode so
-  // that it can be used as the "source of truth" eventually. Note that this
-  // only applies in case the --ignition-preserve-bytecode flag is not passed.
-  if (!FLAG_ignition_preserve_bytecode) shared->ClearBytecodeArray();
-
   // Update the shared function info with the scope info.
   InstallSharedScopeInfo(&info, shared);
 
@@ -1312,19 +1214,6 @@ bool Compiler::EnsureDeoptimizationSupport(CompilationInfo* info) {
     // TurboFan in this case.
     if (IsResumableFunction(shared->kind())) return false;
 
-    // TODO(4280): For now we disable switching to baseline code in the presence
-    // of interpreter activations of the given function. The reasons is that the
-    // underlying bytecode is cleared below. The expensive check for activations
-    // only needs to be done when the given function has bytecode, otherwise we
-    // can be sure there are no activations. Note that this only applies in case
-    // the --ignition-preserve-bytecode flag is not passed.
-    if (!FLAG_ignition_preserve_bytecode && shared->HasBytecodeArray()) {
-      InterpreterActivationsFinder activations_finder(*shared);
-      if (HasInterpreterActivations(info->isolate(), &activations_finder)) {
-        return false;
-      }
-    }
-
     // When we call PrepareForSerializing below, we will change the shared
     // ParseInfo. Make sure to reset it.
     bool old_will_serialize_value = info->parse_info()->will_serialize();
@@ -1341,14 +1230,6 @@ bool Compiler::EnsureDeoptimizationSupport(CompilationInfo* info) {
 
     info->parse_info()->set_will_serialize(old_will_serialize_value);
 
-    // TODO(4280): For now we play it safe and remove the bytecode array when we
-    // switch to baseline code. We might consider keeping around the bytecode so
-    // that it can be used as the "source of truth" eventually. Note that this
-    // only applies in case the --ignition-preserve-bytecode flag is not passed.
-    if (!FLAG_ignition_preserve_bytecode && shared->HasBytecodeArray()) {
-      shared->ClearBytecodeArray();
-    }
-
     // The scope info might not have been set if a lazily compiled
     // function is inlined before being called for the first time.
     if (shared->scope_info() == ScopeInfo::Empty(info->isolate())) {

src/flag-definitions.h

@@ -299,8 +299,6 @@ DEFINE_BOOL(ignition_peephole, true, "use ignition peephole optimizer")
 DEFINE_BOOL(ignition_reo, true, "use ignition register equivalence optimizer")
 DEFINE_BOOL(ignition_filter_expression_positions, true,
             "filter expression positions before the bytecode pipeline")
-DEFINE_BOOL(ignition_preserve_bytecode, true,
-            "preserve generated bytecode even when switching tiers")
 DEFINE_BOOL(print_bytecode, false,
             "print bytecode generated by ignition interpreter")
 DEFINE_BOOL(trace_ignition, false,

src/frames.cc

@@ -406,15 +406,11 @@ static bool IsInterpreterFramePc(Isolate* isolate, Address pc) {
       isolate->builtins()->builtin(Builtins::kInterpreterEntryTrampoline);
   Code* interpreter_bytecode_dispatch =
       isolate->builtins()->builtin(Builtins::kInterpreterEnterBytecodeDispatch);
-  Code* interpreter_baseline_on_return =
-      isolate->builtins()->builtin(Builtins::kInterpreterMarkBaselineOnReturn);
 
   return (pc >= interpreter_entry_trampoline->instruction_start() &&
           pc < interpreter_entry_trampoline->instruction_end()) ||
          (pc >= interpreter_bytecode_dispatch->instruction_start() &&
-          pc < interpreter_bytecode_dispatch->instruction_end()) ||
-         (pc >= interpreter_baseline_on_return->instruction_start() &&
-          pc < interpreter_baseline_on_return->instruction_end());
+          pc < interpreter_bytecode_dispatch->instruction_end());
 }
 
 StackFrame::Type StackFrame::ComputeType(const StackFrameIteratorBase* iterator,

src/objects-inl.h

@@ -5055,8 +5055,7 @@ inline bool Code::is_hydrogen_stub() {
 inline bool Code::is_interpreter_trampoline_builtin() {
   Builtins* builtins = GetIsolate()->builtins();
   return this == *builtins->InterpreterEntryTrampoline() ||
-         this == *builtins->InterpreterEnterBytecodeDispatch() ||
-         this == *builtins->InterpreterMarkBaselineOnReturn();
+         this == *builtins->InterpreterEnterBytecodeDispatch();
 }
 
 inline bool Code::has_unwinding_info() const {

test/cctest/test-compiler.cc

@@ -619,53 +619,6 @@ TEST(SplitConstantsInFullCompiler) {
 }
 #endif
 
-static void IsBaselineCompiled(
-    const v8::FunctionCallbackInfo<v8::Value>& args) {
-  Handle<Object> object = v8::Utils::OpenHandle(*args[0]);
-  Handle<JSFunction> function = Handle<JSFunction>::cast(object);
-  bool is_baseline = function->shared()->code()->kind() == Code::FUNCTION;
-  return args.GetReturnValue().Set(is_baseline);
-}
-
-static void InstallIsBaselineCompiledHelper(v8::Isolate* isolate) {
-  v8::Local<v8::Context> context = isolate->GetCurrentContext();
-  v8::Local<v8::FunctionTemplate> t =
-      v8::FunctionTemplate::New(isolate, IsBaselineCompiled);
-  CHECK(context->Global()
-            ->Set(context, v8_str("IsBaselineCompiled"),
-                  t->GetFunction(context).ToLocalChecked())
-            .FromJust());
-}
-
-TEST(IgnitionBaselineOnReturn) {
-  // TODO(4280): Remove this entire test once --ignition-preserve-bytecode is
-  // the default and the flag is removed. This test doesn't provide benefit any
-  // longer once {InterpreterActivationsFinder} is gone.
-  if (FLAG_ignition_preserve_bytecode) return;
-  FLAG_allow_natives_syntax = true;
-  FLAG_always_opt = false;
-  CcTest::InitializeVM();
-  FLAG_ignition = true;
-  Isolate* isolate = CcTest::i_isolate();
-  isolate->interpreter()->Initialize();
-  v8::HandleScope scope(CcTest::isolate());
-  InstallIsBaselineCompiledHelper(CcTest::isolate());
-
-  CompileRun(
-      "var is_baseline_in_function, is_baseline_after_return;\n"
-      "var return_val;\n"
-      "function f() {\n"
-      "  %CompileBaseline(f);\n"
-      "  is_baseline_in_function = IsBaselineCompiled(f);\n"
-      "  return 1234;\n"
-      "};\n"
-      "return_val = f();\n"
-      "is_baseline_after_return = IsBaselineCompiled(f);\n");
-  CHECK_EQ(false, GetGlobalProperty("is_baseline_in_function")->BooleanValue());
-  CHECK_EQ(true, GetGlobalProperty("is_baseline_after_return")->BooleanValue());
-  CHECK_EQ(1234.0, GetGlobalProperty("return_val")->Number());
-}
-
 TEST(IgnitionEntryTrampolineSelfHealing) {
   FLAG_allow_natives_syntax = true;
   FLAG_always_opt = false;