Fix %OptimizeFunctionOnNextCall to actually work when the function has not yet been compiled.

Fix %OptimizeFunctionOnNextCall to actually work when the function has not yet been compiled.

R=yangguo@chromium.org
BUG=

Committed: https://code.google.com/p/v8/source/detail?r=23687

[Yang, 2014-09-04 09:07:18]

https://codereview.chromium.org/543643002/diff/1/src/compiler.cc
File src/compiler.cc (right):

https://codereview.chromium.org/543643002/diff/1/src/compiler.cc#newcode1251
src/compiler.cc:1251: CompilationInfoWithZone nested(function);
at this point, if deopt support is not enabled, we will end up recompiling the
unoptimized code again in OptimizedCompileJob::CreateGraph. I think we can
simply remove this DCHECK and ensure that we have unoptimized code in
OptimizedCompileJob::CreateGraph.

It seems to me that OptimizedCompileJob::CreateGraph requires scope info before
it ensure we have deopt support in the unoptimized code. But I think we could
change the order: first ensure that we have unoptimized code with deopt
support;then do whatever we need that requires scope info.

Alternatively, when we compile unoptimized here, enable deopt support so that we
don't have to recompile unoptimized later.

[titzer, 2014-09-04 10:25:05]

On 2014/09/04 09:07:18, Yang wrote:
> https://codereview.chromium.org/543643002/diff/1/src/compiler.cc
> File src/compiler.cc (right):
> 
> https://codereview.chromium.org/543643002/diff/1/src/compiler.cc#newcode1251
> src/compiler.cc:1251: CompilationInfoWithZone nested(function);
> at this point, if deopt support is not enabled, we will end up recompiling the
> unoptimized code again in OptimizedCompileJob::CreateGraph. I think we can
> simply remove this DCHECK and ensure that we have unoptimized code in
> OptimizedCompileJob::CreateGraph.
> 
> It seems to me that OptimizedCompileJob::CreateGraph requires scope info
before
> it ensure we have deopt support in the unoptimized code. But I think we could
> change the order: first ensure that we have unoptimized code with deopt
> support;then do whatever we need that requires scope info.
> 
> Alternatively, when we compile unoptimized here, enable deopt support so that
we
> don't have to recompile unoptimized later.

Nice catch. I've enabled deoptimization support when doing the unoptimized
compile, which should avoid the first problem you mentioned.

In general I think we need a deeper refactoring here that cleans up the
evolution of SharedFunctions, JSFunctions, and the CompilationInfo through the
pipeline.

Given that, I don't want to hatchetize this code more than necessary at the
moment.

[Yang, 2014-09-04 10:38:17]

On 2014/09/04 10:25:05, titzer wrote:
> On 2014/09/04 09:07:18, Yang wrote:
> > https://codereview.chromium.org/543643002/diff/1/src/compiler.cc
> > File src/compiler.cc (right):
> > 
> > https://codereview.chromium.org/543643002/diff/1/src/compiler.cc#newcode1251
> > src/compiler.cc:1251: CompilationInfoWithZone nested(function);
> > at this point, if deopt support is not enabled, we will end up recompiling
the
> > unoptimized code again in OptimizedCompileJob::CreateGraph. I think we can
> > simply remove this DCHECK and ensure that we have unoptimized code in
> > OptimizedCompileJob::CreateGraph.
> > 
> > It seems to me that OptimizedCompileJob::CreateGraph requires scope info
> before
> > it ensure we have deopt support in the unoptimized code. But I think we
could
> > change the order: first ensure that we have unoptimized code with deopt
> > support;then do whatever we need that requires scope info.
> > 
> > Alternatively, when we compile unoptimized here, enable deopt support so
that
> we
> > don't have to recompile unoptimized later.
> 
> Nice catch. I've enabled deoptimization support when doing the unoptimized
> compile, which should avoid the first problem you mentioned.
> 
> In general I think we need a deeper refactoring here that cleans up the
> evolution of SharedFunctions, JSFunctions, and the CompilationInfo through the
> pipeline.
> 
> Given that, I don't want to hatchetize this code more than necessary at the
> moment.

lgtm.

[titzer, 2014-09-04 11:27:32]

Committed patchset #3 manually as 23687 (presubmit successful).