[Compiler] Skip Ignition for asm.js code.

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 "src/ast/ast-numbering.h"
 #include "src/ast/prettyprinter.h"
@@ skipping 921 matching lines @@
 
   // Install compilation result on the shared function info
   InstallSharedCompilationResult(&info, shared);
 
   // Record the function compilation event.
   RecordFunctionCompilation(Logger::LAZY_COMPILE_TAG, &info);
 
   return info.code();
 }
 
+bool ShouldOptimizeForAsm(Isolate* isolate, Handle<JSFunction> function) {
+  // If the debugger is active, do not compile with turbofan unless we can
+  // deopt from turbofan code.
+  return FLAG_turbo_asm && function->shared()->asm_function() &&
+         (FLAG_turbo_asm_deoptimization || !isolate->debug()->is_active());

[Michael Starzinger, 2016/05/20 11:45:10]

What would you thing about guarding this by FLAG_ignition as well. Because for
the shipping configuration (with OSR support) we abandoned the eager
optimization of asm.js functions.

[rmcilroy, 2016/05/20 12:42:03]

Is there any particularly reason we abandoned it? Given asmJS is meant to be
ahead-of-time style code isn't it better just to optimize first without running
unoptimized code at all (and possibly causing jank mid-way through the app when
we recompile for OSR)?

As I mentioned on the chat, there is an issue with the sqlite tests which are
much slower after this change. Is there type feedback you are relying on in TF
to optmize asmJS code? If so, we should probably just skip Ignition and go
straight to FCG here rather than trying to optimize straight to TF.

[Michael Starzinger, 2016/05/23 14:45:38]

As discussed offline: I am afraid this will tank our shipping configuration. If
we want to change the shipping configuration then I would vote for doing so in a
separate CL (i.e. land this CL with the additional check for FLAG_ignition, then
land a second CL that removes the FLAG_ignition). This way we can at least
revert it easily without having the remove the Ignition path.

+Benedikt, +Ben: WDYT?

[rmcilroy, 2016/05/23 15:07:53]

Sounds good, I've updated this CL to only do this for FLAG_ignition and will
upload another CL for the shipping configuration.

[titzer, 2016/05/23 15:48:34]

It's not type feedback, it's just that TF is too heavyweight for the startup
phase of asm.js code. But OSR is required for the asm.js benchmarks to do well.
So OSR from ignition->TF for asm.js code is probably not in the cards, so maybe
best to just do as you say and do FCG.

+}
+
 MaybeHandle<Code> GetLazyCode(Handle<JSFunction> function) {
   Isolate* isolate = function->GetIsolate();
   DCHECK(!isolate->has_pending_exception());
   DCHECK(!function->is_compiled());
   TimerEventScope<TimerEventCompileCode> compile_timer(isolate);
   TRACE_EVENT0("v8", "V8.CompileCode");
   AggregatedHistogramTimerScope timer(isolate->counters()->compile_lazy());
 
   if (FLAG_turbo_cache_shared_code) {
     Handle<Code> cached_code;
@@ skipping 12 matching lines @@
   if (function->shared()->is_compiled()) {
     return Handle<Code>(function->shared()->code());
   }
 
   Zone zone(isolate->allocator());
   ParseInfo parse_info(&zone, function);
   CompilationInfo info(&parse_info, function);
   Handle<Code> result;
   ASSIGN_RETURN_ON_EXCEPTION(isolate, result, GetUnoptimizedCode(&info), Code);
 
-  if (FLAG_always_opt) {
+  if (FLAG_always_opt || ShouldOptimizeForAsm(isolate, function)) {

[Michael Starzinger, 2016/05/20 11:45:10]

Can we keep the optimization path for production above the unoptimized compile?
That way we don't needlessly compile unoptimized code without deoptimization
support. For the --always-opt case the trigger is down here to increase code
coverage.

[rmcilroy, 2016/05/20 12:42:03]

Done.

     Handle<Code> opt_code;
     if (GetOptimizedCode(function, Compiler::NOT_CONCURRENT)
             .ToHandle(&opt_code)) {
       result = opt_code;
     }
   }
 
   return result;
 }
 
@@ skipping 772 matching lines @@
   if (FLAG_trace_opt) {
     PrintF("[aborted optimizing ");
     info->closure()->ShortPrint();
     PrintF(" because: %s]\n", GetBailoutReason(info->bailout_reason()));
   }
   info->closure()->ReplaceCode(shared->code());
 }
 
 void Compiler::PostInstantiation(Handle<JSFunction> function,
                                  PretenureFlag pretenure) {
+  Isolate* isolate = function->GetIsolate();
   Handle<SharedFunctionInfo> shared(function->shared());
 
-  if (FLAG_always_opt && shared->allows_lazy_compilation()) {
+  if ((FLAG_always_opt || ShouldOptimizeForAsm(isolate, function)) &&

[Michael Starzinger, 2016/05/20 11:45:10]

I am not entirely sure why this is needed. For functions where SFI is not yet
compiled we should hit the GetLazy code path above. For functions where SFI is
already compiled, the optimized code map should hit. Could you elaborate in
this?

[rmcilroy, 2016/05/20 12:42:03]

This is for the eager compilation pipeline (--ignition-eager) without it we
don't optimize on first-run (there is nothing in the optimized code map the
first time we run) and so we run the unoptimized bytecode. 

[rmcilroy, 2016/05/23 09:28:16]

I just realized, this would mark all new functions for optimization even if
there was something already in the code cache (I mistakenly thought --always-opt
would do the right thing but forgot it was a purely testing flag). I've moved
this down to below the optimized code map check. I still have issues with sqlite
timing out though :/.

[Michael Starzinger, 2016/05/23 14:45:38]

Acknowledged. New version in patch set #5 works for me.

+      shared->allows_lazy_compilation()) {
     function->MarkForOptimization();
   }
 
   CodeAndLiterals cached = shared->SearchOptimizedCodeMap(
       function->context()->native_context(), BailoutId::None());
   if (cached.code != nullptr) {
     // Caching of optimized code enabled and optimized code found.
     DCHECK(!cached.code->marked_for_deoptimization());
     DCHECK(function->shared()->is_compiled());
     function->ReplaceCode(cached.code);
   }
 
   if (cached.literals != nullptr) {
     function->set_literals(cached.literals);
   } else {
-    Isolate* isolate = function->GetIsolate();
     int number_of_literals = shared->num_literals();
     Handle<LiteralsArray> literals =
         LiteralsArray::New(isolate, handle(shared->feedback_vector()),
                            number_of_literals, pretenure);
     function->set_literals(*literals);
 
     // Cache context-specific literals.
     MaybeHandle<Code> code;
     if (cached.code != nullptr) code = handle(cached.code);
     Handle<Context> native_context(function->context()->native_context());
     SharedFunctionInfo::AddToOptimizedCodeMap(shared, native_context, code,
                                               literals, BailoutId::None());
   }
 }
 
 }  // namespace internal
 }  // namespace v8