[compiler] Make SFI "optimize" flag a "tier up" flag

src/compiler.cc

 // Copyright 2012 the V8 project authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "src/compiler.h"
 
 #include <algorithm>
 #include <memory>
 
 #include "src/asmjs/asm-js.h"
@@ skipping 616 matching lines @@
                                    Compiler::ConcurrencyMode mode,
                                    BailoutId osr_ast_id = BailoutId::None(),
                                    JavaScriptFrame* osr_frame = nullptr) {
   Isolate* isolate = function->GetIsolate();
   Handle<SharedFunctionInfo> shared(function->shared(), isolate);
 
   bool ignition_osr = osr_frame && osr_frame->is_interpreted();
   DCHECK_IMPLIES(ignition_osr, !osr_ast_id.IsNone());
   DCHECK_IMPLIES(ignition_osr, FLAG_ignition_osr);
 
+  // Shared function no longer needs to be tiered up
+  shared->set_was_marked_for_tier_up(false);
+
   // Flag combination --ignition-osr --no-turbo-from-bytecode is unsupported.
   if (ignition_osr && !FLAG_turbo_from_bytecode) return MaybeHandle<Code>();
 
   Handle<Code> cached_code;
   // TODO(4764): When compiling for OSR from bytecode, BailoutId might derive
   // from bytecode offset and overlap with actual BailoutId. No lookup!
   if (!ignition_osr &&
       GetCodeFromOptimizedCodeMap(function, osr_ast_id)
           .ToHandle(&cached_code)) {
     if (FLAG_trace_opt) {
@@ skipping 216 matching lines @@
 }
 
 MaybeHandle<Code> GetBaselineCode(Handle<JSFunction> function) {
   Isolate* isolate = function->GetIsolate();
   VMState<COMPILER> state(isolate);
   PostponeInterruptsScope postpone(isolate);
   Zone zone(isolate->allocator(), ZONE_NAME);
   ParseInfo parse_info(&zone, handle(function->shared()));
   CompilationInfo info(&parse_info, function);
 
+  // Function no longer needs to be tiered up
+  function->shared()->set_was_marked_for_tier_up(false);
+
   // Reset profiler ticks, function is no longer considered hot.
   if (function->shared()->HasBytecodeArray()) {
     function->shared()->set_profiler_ticks(0);
   }
 
   // Nothing left to do if the function already has baseline code.
   if (function->shared()->code()->kind() == Code::FUNCTION) {
     return Handle<Code>(function->shared()->code());
   }
 
@@ skipping 87 matching lines @@
           .ToHandle(&cached_code)) {
     if (FLAG_trace_opt) {
       PrintF("[found optimized code for ");
       function->ShortPrint();
       PrintF(" during unoptimized compile]\n");
     }
     DCHECK(function->shared()->is_compiled());
     return cached_code;
   }
 
-  if (function->shared()->was_marked_for_optimization()) {
-    DCHECK(FLAG_optimize_shared_functions);
+  if (function->shared()->was_marked_for_tier_up()) {
+    DCHECK(FLAG_tier_up_shared_functions);
+    DCHECK(function->shared()->is_compiled());

[Michael Starzinger, 2016/10/27 09:29:59]

Can you elaborate why this DCHECK was added? I can see how it currently holds,
but I am not sure why it is actually needed or what it is intended to be
guarding. Both {GetBaselineCode} as well as {GetOptimizedCode} should be
resilient towards the SFI no being compiled yet. Unless I am missing something I
would vote for dropping the DCHECK.

[Michael Starzinger, 2016/10/27 09:32:21]

Actually, I think with code flushing this is not guaranteed to hold. There might
very well be an SFI that was makred for eager tier-up but the GC flushed the
shared code in the meantime. Pretty sure I can write a repro for this. Let me
know if you want/need a repro for this.

[Leszek Swirski, 2016/10/27 09:40:20]

It's here because I put the default case as unreachable, my reasoning being that
"tiering up" to interpreter is a logic error somewhere. But, I see your point
about resilience, and we may have some eager marking in the future (e.g. for
always-opt or some parse-time heuristics), so I'll remove it.

Oh, and good point with the code flushing. No repro needed, but thanks!

 
-    function->shared()->set_was_marked_for_optimization(false);
+    function->shared()->set_was_marked_for_tier_up(false);
 
-    if (FLAG_trace_opt) {
-      PrintF("[optimizing function ");
-      function->PrintName();
-      PrintF(" eagerly because shared function was previously marked]\n");
-    }
+    switch (Compiler::NextCompilationTier(*function)) {
+      case Compiler::BASELINE: {
+        if (FLAG_trace_opt) {
+          PrintF("[recompiling function ");
+          function->ShortPrint();
+          PrintF(
+              " to baseline eagerly (shared function marked for tier up)]\n");
+        }
 
-    Handle<Code> opt_code;
-    if (GetOptimizedCode(function, Compiler::NOT_CONCURRENT)
-            .ToHandle(&opt_code)) {
-      return opt_code;
+        Handle<Code> code;
+        if (!GetBaselineCode(function).ToHandle(&code)) {
+          return code;
+        }
+        break;
+      }
+      case Compiler::OPTIMIZED: {
+        if (FLAG_trace_opt) {
+          PrintF("[optimizing method ");
+          function->ShortPrint();
+          PrintF(" eagerly (shared function marked for tier up)]\n");
+        }
+
+        Handle<Code> code;
+        // TODO(leszeks): Look into performing this compilation concurrently.
+        if (!GetOptimizedCode(function, Compiler::NOT_CONCURRENT)
+                 .ToHandle(&code)) {
+          return code;
+        }
+        break;
+      }
+      default:
+        UNREACHABLE();
     }
   }
 
   if (function->shared()->is_compiled()) {
     return Handle<Code>(function->shared()->code());
   }
 
   if (function->shared()->HasBytecodeArray()) {
     Handle<Code> entry = isolate->builtins()->InterpreterEntryTrampoline();
     function->shared()->ReplaceCode(*entry);
@@ skipping 825 matching lines @@
     DCHECK(shared->is_compiled());
     function->set_literals(cached.literals);
   } else if (shared->is_compiled()) {
     // TODO(mvstanton): pass pretenure flag to EnsureLiterals.
     JSFunction::EnsureLiterals(function);
   }
 }
 
 }  // namespace internal
 }  // namespace v8