Improve internal compiler API so that OSR code is never installed on function.

runtime/vm/compiler.cc

Index: runtime/vm/compiler.cc
diff --git a/runtime/vm/compiler.cc b/runtime/vm/compiler.cc
index 0a452b337b1a412b47d1b82ca76ea63fc7d60420..874a2aebfb307a4a2f6c5524021a00a02921e51a 100644
--- a/runtime/vm/compiler.cc
+++ b/runtime/vm/compiler.cc
@@ -204,14 +204,14 @@ CompilationPipeline* CompilationPipeline::New(Zone* zone,
 DEFINE_RUNTIME_ENTRY(CompileFunction, 1) {
   const Function& function = Function::CheckedHandle(arguments.ArgAt(0));
   ASSERT(!function.HasCode());
-  const Error& error =
-      Error::Handle(Compiler::CompileFunction(thread, function));
-  if (!error.IsNull()) {
-    if (error.IsLanguageError()) {
-      Exceptions::ThrowCompileTimeError(LanguageError::Cast(error));
+  const Object& result =
+      Object::Handle(Compiler::CompileFunction(thread, function));
+  if (result.IsError()) {
+    if (result.IsLanguageError()) {
+      Exceptions::ThrowCompileTimeError(LanguageError::Cast(result));
       UNREACHABLE();
     }
-    Exceptions::PropagateError(error);
+    Exceptions::PropagateError(Error::Cast(result));
   }
 }
 
@@ -496,7 +496,7 @@ class CompileParsedFunctionHelper : public ValueObject {
         loading_invalidation_gen_at_start_(
             isolate()->loading_invalidation_gen()) {}
 
-  bool Compile(CompilationPipeline* pipeline);
+  RawCode* Compile(CompilationPipeline* pipeline);
 
  private:
   ParsedFunction* parsed_function() const { return parsed_function_; }
@@ -507,9 +507,9 @@ class CompileParsedFunctionHelper : public ValueObject {
   intptr_t loading_invalidation_gen_at_start() const {
     return loading_invalidation_gen_at_start_;
   }
-  void FinalizeCompilation(Assembler* assembler,
-                           FlowGraphCompiler* graph_compiler,
-                           FlowGraph* flow_graph);
+  RawCode* FinalizeCompilation(Assembler* assembler,
+                               FlowGraphCompiler* graph_compiler,
+                               FlowGraph* flow_graph);
   void CheckIfBackgroundCompilerIsBeingStopped();
 
   ParsedFunction* parsed_function_;
@@ -522,7 +522,7 @@ class CompileParsedFunctionHelper : public ValueObject {
 };
 
 
-void CompileParsedFunctionHelper::FinalizeCompilation(
+RawCode* CompileParsedFunctionHelper::FinalizeCompilation(
     Assembler* assembler,
     FlowGraphCompiler* graph_compiler,
     FlowGraph* flow_graph) {
@@ -539,7 +539,7 @@ void CompileParsedFunctionHelper::FinalizeCompilation(
            deopt_info_array.Length() * sizeof(uword));
   // Allocates instruction object. Since this occurs only at safepoint,
   // there can be no concurrent access to the instruction page.
-  const Code& code =
+  Code& code =
       Code::Handle(Code::FinalizeCode(function, assembler, optimized()));
   code.set_is_optimized(optimized());
   code.set_owner(function);
@@ -588,12 +588,13 @@ void CompileParsedFunctionHelper::FinalizeCompilation(
   graph_compiler->FinalizeCodeSourceMap(code);
 
   if (optimized()) {
-    bool code_was_installed = false;
     // Installs code while at safepoint.
     if (thread()->IsMutatorThread()) {
       const bool is_osr = osr_id() != Compiler::kNoOSRDeoptId;
-      function.InstallOptimizedCode(code, is_osr);
-      code_was_installed = true;
+      if (!is_osr) {
+        function.InstallOptimizedCode(code);
+      }
+      ASSERT(code.owner() == function.raw());
     } else {
       // Background compilation.
       // Before installing code check generation counts if the code may
@@ -637,8 +638,9 @@ void CompileParsedFunctionHelper::FinalizeCompilation(
       if (code_is_valid && Compiler::CanOptimizeFunction(thread(), function)) {
         const bool is_osr = osr_id() != Compiler::kNoOSRDeoptId;
         ASSERT(!is_osr);  // OSR is not compiled in background.
-        function.InstallOptimizedCode(code, is_osr);
-        code_was_installed = true;
+        function.InstallOptimizedCode(code);
+      } else {
+        code = Code::null();
       }
       if (function.usage_counter() < 0) {
         // Reset to 0 so that it can be recompiled if needed.
@@ -651,7 +653,7 @@ void CompileParsedFunctionHelper::FinalizeCompilation(
       }
     }
 
-    if (code_was_installed) {
+    if (!code.IsNull()) {
       // The generated code was compiled under certain assumptions about
       // class hierarchy and field types. Register these dependencies
       // to ensure that the code will be deoptimized if they are violated.
@@ -682,6 +684,7 @@ void CompileParsedFunctionHelper::FinalizeCompilation(
       (*prefixes)[i]->RegisterDependentCode(code);
     }
   }
+  return code.raw();
 }
 
 
@@ -695,16 +698,15 @@ void CompileParsedFunctionHelper::CheckIfBackgroundCompilerIsBeingStopped() {
 }
 
 
-// Return false if bailed out.
+// Return null if bailed out.
 // If optimized_result_code is not NULL then it is caller's responsibility
 // to install code.
-bool CompileParsedFunctionHelper::Compile(CompilationPipeline* pipeline) {
+RawCode* CompileParsedFunctionHelper::Compile(CompilationPipeline* pipeline) {
   ASSERT(!FLAG_precompiled_mode);
   const Function& function = parsed_function()->function();
   if (optimized() && !function.IsOptimizable()) {
-    return false;
+    return Code::null();
   }
-  bool is_compiled = false;
   Zone* const zone = thread()->zone();
   NOT_IN_PRODUCT(TimelineStream* compiler_timeline =
                      Timeline::GetCompilerStream());
@@ -722,12 +724,13 @@ bool CompileParsedFunctionHelper::Compile(CompilationPipeline* pipeline) {
   volatile bool use_far_branches = false;
   const bool use_speculative_inlining = false;
 
+  Code* volatile result = &Code::ZoneHandle(zone);
   while (!done) {
+    *result = Code::null();
     const intptr_t prev_deopt_id = thread()->deopt_id();
     thread()->set_deopt_id(0);
     LongJumpScope jump;
-    const intptr_t val = setjmp(*jump.Set());
-    if (val == 0) {
+    if (setjmp(*jump.Set()) == 0) {
       FlowGraph* flow_graph = NULL;
 
       // Class hierarchy analysis is registered with the thread in the
@@ -1135,7 +1138,8 @@ bool CompileParsedFunctionHelper::Compile(CompilationPipeline* pipeline) {
         NOT_IN_PRODUCT(TimelineDurationScope tds(thread(), compiler_timeline,
                                                  "FinalizeCompilation"));
         if (thread()->IsMutatorThread()) {
-          FinalizeCompilation(&assembler, &graph_compiler, flow_graph);
+          *result =
+              FinalizeCompilation(&assembler, &graph_compiler, flow_graph);
         } else {
           // This part of compilation must be at a safepoint.
           // Stop mutator thread before creating the instruction object and
@@ -1151,7 +1155,8 @@ bool CompileParsedFunctionHelper::Compile(CompilationPipeline* pipeline) {
             // heap to grow.
             NoHeapGrowthControlScope no_growth_control;
             CheckIfBackgroundCompilerIsBeingStopped();
-            FinalizeCompilation(&assembler, &graph_compiler, flow_graph);
+            *result =
+                FinalizeCompilation(&assembler, &graph_compiler, flow_graph);
           }
           // TODO(srdjan): Enable this and remove the one from
           // 'BackgroundCompiler::CompileOptimized' once cause of time-outs
@@ -1162,7 +1167,6 @@ bool CompileParsedFunctionHelper::Compile(CompilationPipeline* pipeline) {
         }
       }
       // Exit the loop and the function with the correct result value.
-      is_compiled = true;
       done = true;
     } else {
       // We bailed out or we encountered an error.
@@ -1179,8 +1183,7 @@ bool CompileParsedFunctionHelper::Compile(CompilationPipeline* pipeline) {
         UNREACHABLE();
       } else {
         // If the error isn't due to an out of range branch offset, we don't
-        // try again (done = true), and indicate that we did not finish
-        // compiling (is_compiled = false).
+        // try again (done = true).
         if (FLAG_trace_bailout) {
           THR_Print("%s\n", error.ToErrorCString());
         }
@@ -1194,19 +1197,18 @@ bool CompileParsedFunctionHelper::Compile(CompilationPipeline* pipeline) {
           (LanguageError::Cast(error).kind() == Report::kBailout)) {
         thread()->clear_sticky_error();
       }
-      is_compiled = false;
     }
     // Reset global isolate state.
     thread()->set_deopt_id(prev_deopt_id);
   }
-  return is_compiled;
+  return result->raw();
 }
 
 
-static RawError* CompileFunctionHelper(CompilationPipeline* pipeline,
-                                       const Function& function,
-                                       bool optimized,
-                                       intptr_t osr_id) {
+static RawObject* CompileFunctionHelper(CompilationPipeline* pipeline,
+                                        const Function& function,
+                                        bool optimized,
+                                        intptr_t osr_id) {
   ASSERT(!FLAG_precompiled_mode);
   ASSERT(!optimized || function.was_compiled());
   LongJumpScope jump;
@@ -1262,8 +1264,8 @@ static RawError* CompileFunctionHelper(CompilationPipeline* pipeline,
       }
     }
 
-    const bool success = helper.Compile(pipeline);
-    if (success) {
+    const Code& result = Code::Handle(helper.Compile(pipeline));
+    if (!result.IsNull()) {
       if (!optimized) {
         function.set_was_compiled(true);
       }
@@ -1305,6 +1307,7 @@ static RawError* CompileFunctionHelper(CompilationPipeline* pipeline,
         function.SetIsOptimizable(false);
         return Error::null();
       } else {
+        ASSERT(!optimized);
         // Encountered error.
         Error& error = Error::Handle();
         // We got an error during compilation.
@@ -1318,11 +1321,12 @@ static RawError* CompileFunctionHelper(CompilationPipeline* pipeline,
                 LanguageError::Cast(error).kind() != Report::kBailout));
         return error.raw();
       }
+      UNREACHABLE();
     }
 
     per_compile_timer.Stop();
 
-    if (trace_compiler && success) {
+    if (trace_compiler) {
       THR_Print("--> '%s' entry: %#" Px " size: %" Pd " time: %" Pd64 " us\n",
                 function.ToFullyQualifiedCString(),
                 Code::Handle(function.CurrentCode()).PayloadStart(),
@@ -1341,7 +1345,7 @@ static RawError* CompileFunctionHelper(CompilationPipeline* pipeline,
       Disassembler::DisassembleCode(function, true);
     }
 
-    return Error::null();
+    return result.raw();
   } else {
     Thread* const thread = Thread::Current();
     StackZone stack_zone(thread);
@@ -1356,14 +1360,14 @@ static RawError* CompileFunctionHelper(CompilationPipeline* pipeline,
         THR_Print("Aborted background compilation: %s\n",
                   function.ToFullyQualifiedCString());
       }
-      return Error::null();
+      return Object::null();
     }
     // Do not attempt to optimize functions that can cause errors.
     function.set_is_optimizable(false);
     return error.raw();
   }
   UNREACHABLE();
-  return Error::null();
+  return Object::null();
 }
 
 
@@ -1424,7 +1428,7 @@ static RawError* ParseFunctionHelper(CompilationPipeline* pipeline,
 }
 
 
-RawError* Compiler::CompileFunction(Thread* thread, const Function& function) {
+RawObject* Compiler::CompileFunction(Thread* thread, const Function& function) {
 #ifdef DART_PRECOMPILER
   if (FLAG_precompiled_mode) {
     return Precompiler::CompileFunction(
@@ -1487,19 +1491,20 @@ RawError* Compiler::EnsureUnoptimizedCode(Thread* thread,
   }
   CompilationPipeline* pipeline =
       CompilationPipeline::New(thread->zone(), function);
-  const Error& error = Error::Handle(
+  const Object& result = Object::Handle(
       CompileFunctionHelper(pipeline, function, false, /* not optimized */
                             kNoOSRDeoptId));
-  if (!error.IsNull()) {
-    return error.raw();
+  if (result.IsError()) {
+    return Error::Cast(result).raw();
   }
   // Since CompileFunctionHelper replaces the current code, re-attach the
   // the original code if the function was already compiled.
-  if (!original_code.IsNull() &&
-      (original_code.raw() != function.CurrentCode())) {
+  if (!original_code.IsNull() && result.raw() == function.CurrentCode() &&
+      !original_code.IsDisabled()) {
     function.AttachCode(original_code);
   }
   ASSERT(function.unoptimized_code() != Object::null());
+  ASSERT(function.unoptimized_code() == result.raw());
   if (FLAG_trace_compiler) {
     THR_Print("Ensure unoptimized code for %s\n", function.ToCString());
   }
@@ -1507,9 +1512,9 @@ RawError* Compiler::EnsureUnoptimizedCode(Thread* thread,
 }
 
 
-RawError* Compiler::CompileOptimizedFunction(Thread* thread,
-                                             const Function& function,
-                                             intptr_t osr_id) {
+RawObject* Compiler::CompileOptimizedFunction(Thread* thread,
+                                              const Function& function,
+                                              intptr_t osr_id) {
 #if !defined(PRODUCT)
   VMTagScope tagScope(thread, VMTag::kCompileOptimizedTagId);
   const char* event_name;
@@ -1592,14 +1597,14 @@ void Compiler::ComputeLocalVarDescriptors(const Code& code) {
 RawError* Compiler::CompileAllFunctions(const Class& cls) {
   Thread* thread = Thread::Current();
   Zone* zone = thread->zone();
-  Error& error = Error::Handle(zone);
+  Object& result = Object::Handle(zone);
   Array& functions = Array::Handle(zone, cls.functions());
   Function& func = Function::Handle(zone);
   // Class dynamic lives in the vm isolate. Its array fields cannot be set to
   // an empty array.
   if (functions.IsNull()) {
     ASSERT(cls.IsDynamicClass());
-    return error.raw();
+    return Error::null();
   }
   // Compile all the regular functions.
   for (int i = 0; i < functions.Length(); i++) {
@@ -1612,15 +1617,16 @@ RawError* Compiler::CompileAllFunctions(const Class& cls) {
         // Skipping optional parameters in mixin application.
         continue;
       }
-      error = CompileFunction(thread, func);
-      if (!error.IsNull()) {
-        return error.raw();
+      result = CompileFunction(thread, func);
+      if (result.IsError()) {
+        return Error::Cast(result).raw();
       }
+      ASSERT(!result.IsNull());
       func.ClearICDataArray();
       func.ClearCode();
     }
   }
-  return error.raw();
+  return Error::null();
 }
 
 
@@ -2197,7 +2203,7 @@ RawError* Compiler::CompileClass(const Class& cls) {
 }
 
 
-RawError* Compiler::CompileFunction(Thread* thread, const Function& function) {
+RawObject* Compiler::CompileFunction(Thread* thread, const Function& function) {
   UNREACHABLE();
   return Error::null();
 }
@@ -2216,9 +2222,9 @@ RawError* Compiler::EnsureUnoptimizedCode(Thread* thread,
 }
 
 
-RawError* Compiler::CompileOptimizedFunction(Thread* thread,
-                                             const Function& function,
-                                             intptr_t osr_id) {
+RawObject* Compiler::CompileOptimizedFunction(Thread* thread,
+                                              const Function& function,
+                                              intptr_t osr_id) {
   UNREACHABLE();
   return Error::null();
 }