Make closures optimizable by Crankshaft compiler.

src/compiler.cc

 // Copyright 2010 the V8 project authors. All rights reserved.
 // Redistribution and use in source and binary forms, with or without
 // modification, are permitted provided that the following conditions are
 // met:
 //
 //     * Redistributions of source code must retain the above copyright
 //       notice, this list of conditions and the following disclaimer.
 //     * Redistributions in binary form must reproduce the above
 //       copyright notice, this list of conditions and the following
 //       disclaimer in the documentation and/or other materials provided
@@ skipping 644 matching lines @@
       !LiveEditFunctionTracker::IsActive();
 
   Handle<SerializedScopeInfo> scope_info(SerializedScopeInfo::Empty());
 
   // Generate code
   if (FLAG_lazy && allow_lazy) {
     Handle<Code> code(Builtins::builtin(Builtins::LazyCompile));
     info.SetCode(code);
   } else {
     if (V8::UseCrankshaft()) {
       if (!MakeCrankshaftCode(&info)) {

[Kevin Millikin (Chromium), 2011/01/14 12:48:59]

The code here is confusing.  From my reading of the code:

MakeCrankshaftCode will check if the CompilationInfo allows optimizing, which
requires a closure.  Since we don't have one for BuildFunctionInfo, it's not
allowed from this call site.  Then it will mark the CompilationInfo as not to be
optimized (which it already is anyway) and then always compile with the baseline
compiler.

I think this does the same and is clearer:

if (V8::UseCrankshaft()) {
  if (!FullCodeGenerator::MakeCode(&info)) {
    return Handle...;
  }
  if (FLAG_optimize_closures ....

         return Handle<SharedFunctionInfo>::null();
       }
+      if (FLAG_optimize_closures
+          && !AlwaysFullCompiler()

[Kevin Millikin (Chromium), 2011/01/14 12:48:59]

I'm not sure all these checks are necessary.  I would expect (and hope) that you
don't have to check AlwaysFullCompiler() here because we should never get into
the crankshaft backend for AlwaysFullCompiler().  I don't think you need to
require a non-trivial outer context (do you?).

+          && !info.scope()->HasTrivialOuterContext()
+          && !info.scope()->outer_scope_calls_eval()
+          && !info.scope()->inside_with()) {
+        ASSERT(info.code()->kind() == Code::FUNCTION);
+        info.code()->set_optimizable(true);

[Kevin Millikin (Chromium), 2011/01/14 12:48:59]

Then, I'm not a fan of compiling with non-optimizable and then flipping a bit
after compilation to say that it actually is.  That is error-prone if we do
anything different for optimizable code during baseline compilation.  It would
be better to have these checks before compiling:

if (FLAG_optimize_closures && ...) {
  info->SetOptimizable()  // You'll have to implement this.
}

+      }
     } else {
       // The bodies of function literals have not yet been visited by the
       // AST optimizer/analyzer.
       if (!Rewriter::Analyze(&info)) return Handle<SharedFunctionInfo>::null();
 
       bool is_run_once = literal->try_full_codegen();
       bool can_use_full = FLAG_full_compiler && !literal->contains_loops();
 
       if (AlwaysFullCompiler() || (is_run_once && can_use_full)) {
         if (!FullCodeGenerator::MakeCode(&info)) {
@@ skipping 90 matching lines @@
                               *code,
                               *name));
       OPROFILE(CreateNativeCodeRegion(*name,
                                       code->instruction_start(),
                                       code->instruction_size()));
     }
   }
 }
 
 } }  // namespace v8::internal