I have a couple of small comments about where it's used in the compiler.
Other than that, LGTM. Land it before Florian changes Lithium even more. http://codereview.chromium.org/5753005/diff/40001/src/compiler.cc File src/compiler.cc (right): http://codereview.chromium.org/5753005/diff/40001/src/compiler.cc#newcode665 src/compiler.cc:665: if (!MakeCrankshaftCode(&info)) { 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 .... http://codereview.chromium.org/5753005/diff/40001/src/compiler.cc#newcode669 src/compiler.cc:669: && !AlwaysFullCompiler() 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?). http://codereview.chromium.org/5753005/diff/40001/src/compiler.cc#newcode674 src/compiler.cc:674: info.code()->set_optimizable(true); 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. } http://codereview.chromium.org/5753005/ -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
