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

This is forked off and a prerequisite for https://codereview.chromium.org/2771013002/ which is itself a prerequisite for a change to make fewer old-space allocations in the optimizing compiler, which is in turn a fix for the performance regression in https://codereview.chromium.org/2737303003 Fixes dart-lang/sdk#29160 R=vegorov@google.com BUG=http://dartbug.com/29160 Review-Url: https://codereview.chromium.org/2781483005 .
2017-03-30 15:39:52 +02:00
parent 6a614e17ea
commit 48607be33d
9 changed files with 134 additions and 152 deletions
@@ -95,14 +95,7 @@ static void EnsureConstructorsAreCompiled(const Function& func) {
    Exceptions::PropagateError(error);
    UNREACHABLE();
  }
-  if (!func.HasCode()) {
-    const Error& error =
-        Error::Handle(zone, Compiler::CompileFunction(thread, func));
-    if (!error.IsNull()) {
-      Exceptions::PropagateError(error);
-      UNREACHABLE();
-    }
-  }
+  func.EnsureHasCode();
 }

 static RawInstance* CreateParameterMirrorList(const Function& func,
@@ -657,14 +657,7 @@ DEFINE_RUNTIME_ENTRY(PatchStaticCall, 0) {
  ASSERT(caller_code.is_optimized());
  const Function& target_function = Function::Handle(
      zone, caller_code.GetStaticCallTargetFunctionAt(caller_frame->pc()));
-  if (!target_function.HasCode()) {
-    const Error& error =
-        Error::Handle(zone, Compiler::CompileFunction(thread, target_function));
-    if (!error.IsNull()) {
-      Exceptions::PropagateError(error);
-    }
-  }
-  const Code& target_code = Code::Handle(zone, target_function.CurrentCode());
+  const Code& target_code = Code::Handle(zone, target_function.EnsureHasCode());
  // Before patching verify that we are not repeatedly patching to the same
  // target.
  ASSERT(target_code.raw() !=
@@ -940,13 +933,7 @@ DEFINE_RUNTIME_ENTRY(StaticCallMissHandlerOneArg, 2) {
  // IC data for static call is prepopulated with the statically known target.
  ASSERT(ic_data.NumberOfChecksIs(1));
  const Function& target = Function::Handle(ic_data.GetTargetAt(0));
-  if (!target.HasCode()) {
-    const Error& error =
-        Error::Handle(Compiler::CompileFunction(thread, target));
-    if (!error.IsNull()) {
-      Exceptions::PropagateError(error);
-    }
-  }
+  target.EnsureHasCode();
  ASSERT(!target.IsNull() && target.HasCode());
  ic_data.AddReceiverCheck(arg.GetClassId(), target, 1);
  if (FLAG_trace_ic) {
@@ -972,14 +959,7 @@ DEFINE_RUNTIME_ENTRY(StaticCallMissHandlerTwoArgs, 3) {
  // IC data for static call is prepopulated with the statically known target.
  ASSERT(!ic_data.NumberOfChecksIs(0));
  const Function& target = Function::Handle(ic_data.GetTargetAt(0));
-  if (!target.HasCode()) {
-    const Error& error =
-        Error::Handle(Compiler::CompileFunction(thread, target));
-    if (!error.IsNull()) {
-      Exceptions::PropagateError(error);
-    }
-  }
-  ASSERT(!target.IsNull() && target.HasCode());
+  target.EnsureHasCode();
  GrowableArray<intptr_t> cids(2);
  cids.Add(arg0.GetClassId());
  cids.Add(arg1.GetClassId());
@@ -1321,13 +1301,8 @@ DEFINE_RUNTIME_ENTRY(MegamorphicCacheMissHandler, 3) {
      // the monomorphic case hides an live instance selector from the
      // treeshaker.

-      if (!target_function.HasCode()) {
-        const Error& error =
-            Error::Handle(Compiler::CompileFunction(thread, target_function));
-        if (!error.IsNull()) {
-          Exceptions::PropagateError(error);
-        }
-      }
+      const Code& target_code =
+          Code::Handle(zone, target_function.EnsureHasCode());

      DartFrameIterator iterator;
      StackFrame* miss_function_frame = iterator.NextFrame();
@@ -1336,8 +1311,6 @@ DEFINE_RUNTIME_ENTRY(MegamorphicCacheMissHandler, 3) {
      ASSERT(caller_frame->IsDartFrame());
      const Code& caller_code =
          Code::Handle(zone, caller_frame->LookupDartCode());
-      const Code& target_code =
-          Code::Handle(zone, target_function.CurrentCode());
      const Smi& expected_cid =
          Smi::Handle(zone, Smi::New(receiver.GetClassId()));

@@ -1706,28 +1679,20 @@ DEFINE_RUNTIME_ENTRY(StackOverflow, 0) {
                function.usage_counter());
    }

-    const Code& original_code = Code::Handle(function.CurrentCode());
    // Since the code is referenced from the frame and the ZoneHandle,
    // it cannot have been removed from the function.
-    ASSERT(!original_code.IsNull());
-    const Error& error = Error::Handle(
+    const Object& result = Object::Handle(
        Compiler::CompileOptimizedFunction(thread, function, osr_id));
-    if (!error.IsNull()) {
-      Exceptions::PropagateError(error);
+    if (result.IsError()) {
+      Exceptions::PropagateError(Error::Cast(result));
    }

-    const Code& optimized_code = Code::Handle(function.CurrentCode());
-    // The current code will not be changed in the case that the compiler
-    // bailed out during OSR compilation.
-    if (optimized_code.raw() != original_code.raw()) {
-      // The OSR code does not work for calling the function, so restore the
-      // unoptimized code.  Patch the stack frame to return into the OSR
-      // code.
+    if (!result.IsNull()) {
+      const Code& code = Code::Cast(result);
      uword optimized_entry =
-          Instructions::UncheckedEntryPoint(optimized_code.instructions());
-      function.AttachCode(original_code);
+          Instructions::UncheckedEntryPoint(code.instructions());
      frame->set_pc(optimized_entry);
-      frame->set_pc_marker(optimized_code.raw());
+      frame->set_pc_marker(code.raw());
    }
  }
 }
@@ -1800,13 +1765,11 @@ DEFINE_RUNTIME_ENTRY(OptimizeInvokedFunction, 1) {
                  function.ToFullyQualifiedCString());
      }
    }
-    const Error& error = Error::Handle(
+    const Object& result = Object::Handle(
        zone, Compiler::CompileOptimizedFunction(thread, function));
-    if (!error.IsNull()) {
-      Exceptions::PropagateError(error);
+    if (result.IsError()) {
+      Exceptions::PropagateError(Error::Cast(result));
    }
-    const Code& optimized_code = Code::Handle(zone, function.CurrentCode());
-    ASSERT(!optimized_code.IsNull());
  }
  arguments.SetReturn(function);
 #else
@@ -1835,17 +1798,9 @@ DEFINE_RUNTIME_ENTRY(FixCallersTarget, 0) {
  ASSERT(caller_code.is_optimized());
  const Function& target_function = Function::Handle(
      zone, caller_code.GetStaticCallTargetFunctionAt(frame->pc()));
-  if (!target_function.HasCode()) {
-    const Error& error =
-        Error::Handle(zone, Compiler::CompileFunction(thread, target_function));
-    if (!error.IsNull()) {
-      Exceptions::PropagateError(error);
-    }
-  }
-  ASSERT(target_function.HasCode());

  const Code& current_target_code =
-      Code::Handle(zone, target_function.CurrentCode());
+      Code::Handle(zone, target_function.EnsureHasCode());
  CodePatcher::PatchStaticCallAt(frame->pc(), caller_code, current_target_code);
  caller_code.SetStaticCallTargetCodeAt(frame->pc(), current_target_code);
  if (FLAG_trace_patching) {
@@ -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();
 }
@@ -97,10 +97,13 @@ class Compiler : public AllStatic {
  // Returns Error::null() if there is no compilation error.
  static RawError* CompileClass(const Class& cls);

-  // Generates code for given function and sets its code field.
+  // Generates code for given function without optimization and sets its code
+  // field.
  //
+  // Returns the raw code object if compilation succeeds.  Otherwise returns a
+  // RawError.  Also installs the generated code on the function.
+  static RawObject* CompileFunction(Thread* thread, const Function& function);
  // Returns Error::null() if there is no compilation error.
-  static RawError* CompileFunction(Thread* thread, const Function& function);
  static RawError* ParseFunction(Thread* thread, const Function& function);

  // Generates unoptimized code if not present, current code is unchanged.
@@ -109,12 +112,13 @@ class Compiler : public AllStatic {

  // Generates optimized code for function.
  //
-  // Returns Error::null() if there is no compilation error.
-  // If 'result_code' is not NULL, then the generated code is returned but
-  // not installed.
-  static RawError* CompileOptimizedFunction(Thread* thread,
-                                            const Function& function,
-                                            intptr_t osr_id = kNoOSRDeoptId);
+  // Returns the code object if compilation succeeds.  Returns an Error if
+  // there is a compilation error.  If optimization fails, but there is no
+  // error, returns null.  Any generated code is installed unless we are in
+  // OSR mode.
+  static RawObject* CompileOptimizedFunction(Thread* thread,
+                                             const Function& function,
+                                             intptr_t osr_id = kNoOSRDeoptId);

  // Generates code for given parsed function (without parsing it again) and
  // sets its code field.
@@ -96,10 +96,10 @@ RawObject* DartEntry::InvokeFunction(const Function& function,
  ASSERT(thread->IsMutatorThread());
  ScopedIsolateStackLimits stack_limit(thread, current_sp);
  if (!function.HasCode()) {
-    const Error& error =
-        Error::Handle(zone, Compiler::CompileFunction(thread, function));
-    if (!error.IsNull()) {
-      return error.raw();
+    const Object& result =
+        Object::Handle(zone, Compiler::CompileFunction(thread, function));
+    if (result.IsError()) {
+      return Error::Cast(result).raw();
    }
  }
 // Now Call the invoke stub which will invoke the dart function.
@@ -5288,11 +5288,11 @@ bool Function::HasBreakpoint() const {
 }


-void Function::InstallOptimizedCode(const Code& code, bool is_osr) const {
+void Function::InstallOptimizedCode(const Code& code) const {
  DEBUG_ASSERT(IsMutatorOrAtSafepoint());
  // We may not have previous code if FLAG_precompile is set.
  // Hot-reload may have already disabled the current code.
-  if (!is_osr && HasCode() && !Code::Handle(CurrentCode()).IsDisabled()) {
+  if (HasCode() && !Code::Handle(CurrentCode()).IsDisabled()) {
    Code::Handle(CurrentCode()).DisableDartCode();
  }
  AttachCode(code);
@@ -7328,6 +7328,23 @@ bool Function::CheckSourceFingerprint(const char* prefix, int32_t fp) const {
 }


+RawCode* Function::EnsureHasCode() const {
+  if (HasCode()) return CurrentCode();
+  Thread* thread = Thread::Current();
+  Zone* zone = thread->zone();
+  const Object& result =
+      Object::Handle(zone, Compiler::CompileFunction(thread, *this));
+  if (result.IsError()) {
+    Exceptions::PropagateError(Error::Cast(result));
+    UNREACHABLE();
+  }
+  // Compiling in unoptimized mode should never fail if there are no errors.
+  ASSERT(HasCode());
+  ASSERT(unoptimized_code() == result.raw());
+  return CurrentCode();
+}
+
+
 const char* Function::ToCString() const {
  if (IsNull()) {
    return "Function: null";
@@ -11776,21 +11793,22 @@ RawError* Library::CompileAll() {
  // Inner functions get added to the closures array. As part of compilation
  // more closures can be added to the end of the array. Compile all the
  // closures until we have reached the end of the "worklist".
+  Object& result = Object::Handle(zone);
  const GrowableObjectArray& closures = GrowableObjectArray::Handle(
      zone, Isolate::Current()->object_store()->closure_functions());
  Function& func = Function::Handle(zone);
  for (int i = 0; i < closures.Length(); i++) {
    func ^= closures.At(i);
    if (!func.HasCode()) {
-      error = Compiler::CompileFunction(thread, func);
-      if (!error.IsNull()) {
-        return error.raw();
+      result = Compiler::CompileFunction(thread, func);
+      if (result.IsError()) {
+        return Error::Cast(result).raw();
      }
      func.ClearICDataArray();
      func.ClearCode();
    }
  }
-  return error.raw();
+  return Error::null();
 }


@@ -2293,7 +2293,7 @@ class Function : public Object {

  // Not thread-safe; must be called in the main thread.
  // Sets function's code and code's function.
-  void InstallOptimizedCode(const Code& code, bool is_osr) const;
+  void InstallOptimizedCode(const Code& code) const;
  void AttachCode(const Code& value) const;
  void SetInstructions(const Code& value) const;
  void ClearCode() const;
@@ -2301,6 +2301,12 @@ class Function : public Object {
  // Disables optimized code and switches to unoptimized code.
  void SwitchToUnoptimizedCode() const;

+  // Ensures that the function has code. If there is no code it compiles the
+  // unoptimized version of the code.  If the code contains errors, it calls
+  // Exceptions::PropagateError and does not return.  Normally returns the
+  // current code, whether it is optimized or unoptimized.
+  RawCode* EnsureHasCode() const;
+
  // Disables optimized code and switches to unoptimized code (or the lazy
  // compilation stub).
  void SwitchToLazyCompiledUnoptimizedCode() const;
@@ -2946,7 +2946,7 @@ void PrecompileParsedFunctionHelper::FinalizeCompilation(
  if (optimized()) {
    // Installs code while at safepoint.
    ASSERT(thread()->IsMutatorThread());
-    function.InstallOptimizedCode(code, /* is_osr = */ false);
+    function.InstallOptimizedCode(code);
  } else {  // not optimized.
    function.set_unoptimized_code(code);
    function.AttachCode(code);
@@ -472,9 +472,9 @@ bool CompilerTest::TestCompileFunction(const Function& function) {
  Thread* thread = Thread::Current();
  ASSERT(thread != NULL);
  ASSERT(ClassFinalizer::AllClassesFinalized());
-  const Error& error =
-      Error::Handle(Compiler::CompileFunction(thread, function));
-  return error.IsNull();
+  const Object& result =
+      Object::Handle(Compiler::CompileFunction(thread, function));
+  return result.IsCode();
 }