[turbofan] Rename context stack as part of the environment for OSR.

[turbofan] Rename context stack as part of the environment for OSR.

R=mstarzinger@chromium.org
BUG=

Committed: https://crrev.com/71765afff92bd590227674058f7092397c62dcfa
Cr-Commit-Position: refs/heads/master@{#26705}

[Michael Starzinger, 2015-02-17 10:34:37]

Words cannot even describe how much I dislike that we have to make this change.

https://codereview.chromium.org/921083004/diff/1/src/compiler/ast-graph-build...
File src/compiler/ast-graph-builder.cc (right):

https://codereview.chromium.org/921083004/diff/1/src/compiler/ast-graph-build...
src/compiler/ast-graph-builder.cc:454: Node* closure = GetFunctionClosure();
nit: The closure is only needed in the case we build a function-context, can we
move it into the branch?

https://codereview.chromium.org/921083004/diff/1/src/compiler/ast-graph-build...
src/compiler/ast-graph-builder.cc:468: if (ok) {
It should be safe to also do this in the case of stack-overflow, because
environments are still intact. Let's try to avoid this "ok" dance here and just
always add the end.

https://codereview.chromium.org/921083004/diff/1/src/compiler/ast-graph-build...
File src/compiler/ast-graph-builder.h (right):

https://codereview.chromium.org/921083004/diff/1/src/compiler/ast-graph-build...
src/compiler/ast-graph-builder.h:80: bool CreateGraphBody();
nit: Please move this down to below all the fields (e.g. to line 135) for
readability.

https://codereview.chromium.org/921083004/diff/1/src/compiler/ast-graph-build...
src/compiler/ast-graph-builder.h:370: Node* Context() const { return
contexts_[contexts_.size() - 1]; }
Can use contexts()->back() here instead.

[titzer, 2015-02-17 10:44:17]

https://codereview.chromium.org/921083004/diff/1/src/compiler/ast-graph-build...
File src/compiler/ast-graph-builder.cc (right):

https://codereview.chromium.org/921083004/diff/1/src/compiler/ast-graph-build...
src/compiler/ast-graph-builder.cc:454: Node* closure = GetFunctionClosure();
On 2015/02/17 10:34:37, Michael Starzinger wrote:
> nit: The closure is only needed in the case we build a function-context, can
we
> move it into the branch?

Tried that, but nope. GetFunctionClosure() has a side effect. Rename?

https://codereview.chromium.org/921083004/diff/1/src/compiler/ast-graph-build...
src/compiler/ast-graph-builder.cc:468: if (ok) {
On 2015/02/17 10:34:37, Michael Starzinger wrote:
> It should be safe to also do this in the case of stack-overflow, because
> environments are still intact. Let's try to avoid this "ok" dance here and
just
> always add the end.

Tried that too, but nope, the environment's control dependency may be NULL in
some cases => crash.

https://codereview.chromium.org/921083004/diff/1/src/compiler/ast-graph-build...
File src/compiler/ast-graph-builder.h (right):

https://codereview.chromium.org/921083004/diff/1/src/compiler/ast-graph-build...
src/compiler/ast-graph-builder.h:370: Node* Context() const { return
contexts_[contexts_.size() - 1]; }
On 2015/02/17 10:34:37, Michael Starzinger wrote:
> Can use contexts()->back() here instead.

Done.

[titzer, 2015-02-17 11:24:36]

https://codereview.chromium.org/921083004/diff/1/src/compiler/ast-graph-build...
File src/compiler/ast-graph-builder.cc (right):

https://codereview.chromium.org/921083004/diff/1/src/compiler/ast-graph-build...
src/compiler/ast-graph-builder.cc:468: if (ok) {
On 2015/02/17 10:44:16, titzer wrote:
> On 2015/02/17 10:34:37, Michael Starzinger wrote:
> > It should be safe to also do this in the case of stack-overflow, because
> > environments are still intact. Let's try to avoid this "ok" dance here and
> just
> > always add the end.
> 
> Tried that too, but nope, the environment's control dependency may be NULL in
> some cases => crash.

Disregard previous comment.

[titzer, 2015-02-17 15:30:32]

On 2015/02/17 11:24:36, titzer wrote:
>
https://codereview.chromium.org/921083004/diff/1/src/compiler/ast-graph-build...
> File src/compiler/ast-graph-builder.cc (right):
> 
>
https://codereview.chromium.org/921083004/diff/1/src/compiler/ast-graph-build...
> src/compiler/ast-graph-builder.cc:468: if (ok) {
> On 2015/02/17 10:44:16, titzer wrote:
> > On 2015/02/17 10:34:37, Michael Starzinger wrote:
> > > It should be safe to also do this in the case of stack-overflow, because
> > > environments are still intact. Let's try to avoid this "ok" dance here and
> > just
> > > always add the end.
> > 
> > Tried that too, but nope, the environment's control dependency may be NULL
in
> > some cases => crash.
> 
> Disregard previous comment.

PTAL

[Michael Starzinger, 2015-02-17 15:51:41]

LGTM. Sigh.

[titzer, 2015-02-17 15:56:50]

The CQ bit was checked by titzer@chromium.org

[commit-bot: I haz the power, 2015-02-17 15:57:46]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/921083004/40001

[commit-bot: I haz the power, 2015-02-17 18:10:07]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2015-02-17 18:10:21]

Patchset 3 (id:??) landed as
https://crrev.com/71765afff92bd590227674058f7092397c62dcfa
Cr-Commit-Position: refs/heads/master@{#26705}