[Interpreter] Support compiling for baseline on return from interpreted function.

src/compiler.cc

Index: src/compiler.cc
diff --git a/src/compiler.cc b/src/compiler.cc
index 1fe623bbc9aefbd2765ba8b3657bef92d78ede1f..3e048ed81e00faf5b3382d6339bf3f2ea2f30525 100644
--- a/src/compiler.cc
+++ b/src/compiler.cc
@@ -785,18 +785,23 @@ MaybeHandle<Code> GetOptimizedCode(Handle<JSFunction> function,
 class InterpreterActivationsFinder : public ThreadVisitor,
                                      public OptimizedFunctionVisitor {
  public:
-  SharedFunctionInfo* shared_;
-  bool has_activations_;
-
   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() && !has_activations_; it.Advance()) {
+    for (; !it.done(); it.Advance()) {
       JavaScriptFrame* frame = it.frame();
       if (!frame->is_interpreted()) continue;
-      if (frame->function()->shared() == shared_) has_activations_ = true;
+      if (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);
     }
   }
 
@@ -806,19 +811,39 @@ class InterpreterActivationsFinder : public ThreadVisitor,
 
   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, SharedFunctionInfo* shared) {
-  InterpreterActivationsFinder activations_finder(shared);
-  activations_finder.VisitThread(isolate, isolate->thread_local_top());
-  isolate->thread_manager()->IterateArchivedThreads(&activations_finder);
+bool HasInterpreterActivations(
+    Isolate* isolate, InterpreterActivationsFinder* activations_finder) {
+  activations_finder->VisitThread(isolate, isolate->thread_local_top());
+  isolate->thread_manager()->IterateArchivedThreads(activations_finder);
   if (FLAG_turbo_from_bytecode) {
     // If we are able to optimize functions directly from bytecode, then there
     // might be optimized functions that rely on bytecode being around. We need
     // to prevent switching the given function to baseline code in those cases.
-    Deoptimizer::VisitAllOptimizedFunctions(isolate, &activations_finder);
+    Deoptimizer::VisitAllOptimizedFunctions(isolate, activations_finder);
   }
-  return activations_finder.has_activations_;
+  return activations_finder->has_activations();
 }
 
 MaybeHandle<Code> GetBaselineCode(Handle<JSFunction> function) {
@@ -856,12 +881,22 @@ MaybeHandle<Code> GetBaselineCode(Handle<JSFunction> function) {
   // of interpreter activations of the given function. The reasons are:
   //  1) The debugger assumes each function is either full-code or bytecode.
   //  2) The underlying bytecode is cleared below, breaking stack unwinding.
-  if (HasInterpreterActivations(isolate, function->shared())) {
+  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) << " for baseline recompilation "

[Michael Starzinger, 2016/05/11 13:04:27]

nit: Let's print s/*function/function->shared()/ here instead, because we are
not marking this particular closures, but all of them essentially.

[rmcilroy, 2016/05/12 13:11:47]

Done. This brings up the point of whether we should be somehow marking the
shared function instead of the closure for Baseline in the normal case, since we
don't depend on the closure like CS does?

[Michael Starzinger, 2016/05/13 09:01:39]

Hmm. Interesting idea. I suppose it would be possible to install the
CompileBaseline builtin on the SharedFunctionInfo itself (as opposed to only the
specific closure). I think the self-healing mechanism we added to the
interpreter entry trampoline would require adapting, because there might still
be many closures on the heap that have the interpreter entry trampoline
installed and just switching SharedFunctionInfo::code won't have an effect on
them. If we tackle this, then lets do it in a separate CL though.

[rmcilroy, 2016/05/17 11:28:30]

Yes absolutely, only in a separate CL and if it's useful.

+           << "on return]" << std::endl;
+      }
+    }
+
     return MaybeHandle<Code>();
   }
 
@@ -1251,15 +1286,18 @@ bool Compiler::EnsureDeoptimizationSupport(CompilationInfo* info) {
     Zone zone(info->isolate()->allocator());
     CompilationInfo unoptimized(info->parse_info(), info->closure());
     unoptimized.EnableDeoptimizationSupport();
+
     // TODO(4280): For now we disable switching to baseline code in the presence
     // of interpreter activations of the given function. The reasons are:
     //  1) The debugger assumes each function is either full-code or bytecode.
     //  2) The underlying bytecode is cleared below, breaking stack unwinding.
     // 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.
-    if (shared->HasBytecodeArray() &&
-        HasInterpreterActivations(info->isolate(), *shared)) {
-      return false;
+    if (shared->HasBytecodeArray()) {
+      InterpreterActivationsFinder activations_finder(*shared);
+      if (HasInterpreterActivations(info->isolate(), &activations_finder)) {
+        return false;
+      }
     }
     // If the current code has reloc info for serialization, also include
     // reloc info for serialization for the new code, so that deopt support