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

src/compiler.cc

Index: src/compiler.cc
diff --git a/src/compiler.cc b/src/compiler.cc
index 1ccf5c2a394cb125b09e9c20549dab38d2b662b1..5b3603d83da880ae7b9ccd5e82e60168bc0e433c 100644
--- a/src/compiler.cc
+++ b/src/compiler.cc
@@ -634,6 +634,9 @@ MaybeHandle<Code> GetOptimizedCode(Handle<JSFunction> function,
   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>();
 
@@ -870,6 +873,9 @@ MaybeHandle<Code> GetBaselineCode(Handle<JSFunction> function) {
   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);
@@ -977,21 +983,44 @@ MaybeHandle<Code> GetLazyCode(Handle<JSFunction> function) {
     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();
     }
   }