[ignition] Fix building lookup graph when search depth is 0

[ignition] Fix building lookup graph when search depth is 0

In some (rare) cases, the context depth passed to a dynamic variable lookup can
be zero. In these cases, the fast path for the lookup (i.e. load from context or
global) can always be taken, as there is no need to search the current context.

However, with no slow path checks, the bytecode graph builder had a null
environment for the slow path, causing segfaults when this graph was built.

This patch adds a null check for the slow path environment, and skips building
the slow path if the environment is null.

BUG=chromium:652186
Committed: https://crrev.com/4ad35791199a48e2e503b0f6193f93ab3141be00
Cr-Commit-Position: refs/heads/master@{#39949}

[Leszek Swirski, 2016-10-03 11:28:07]

The CQ bit was checked by leszeks@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-03 11:28:17]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Leszek Swirski, 2016-10-03 11:36:16]

leszeks@chromium.org changed reviewers:
+ mstarzinger@chromium.org

[commit-bot: I haz the power, 2016-10-03 11:51:51]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-10-03 11:51:51]

Dry run: This issue passed the CQ dry run.

[Michael Starzinger, 2016-10-04 09:43:10]

LGTM with nits.

https://codereview.chromium.org/2385123002/diff/1/src/compiler/bytecode-graph...
File src/compiler/bytecode-graph-builder.cc (right):

https://codereview.chromium.org/2385123002/diff/1/src/compiler/bytecode-graph...
src/compiler/bytecode-graph-builder.cc:952: NewMerge();
nit: Shouldn't we only create the singleton merge node in the case we have a
slow path (i.e. move it into the below conditional)?

https://codereview.chromium.org/2385123002/diff/1/src/compiler/bytecode-graph...
src/compiler/bytecode-graph-builder.cc:1001: NewMerge();
nit: Likewise.

[Leszek Swirski, 2016-10-04 10:45:21]

The CQ bit was checked by leszeks@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-04 10:45:24]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Leszek Swirski, 2016-10-04 10:46:55]

Thanks, I'll go ahead and commit

https://codereview.chromium.org/2385123002/diff/1/src/compiler/bytecode-graph...
File src/compiler/bytecode-graph-builder.cc (right):

https://codereview.chromium.org/2385123002/diff/1/src/compiler/bytecode-graph...
src/compiler/bytecode-graph-builder.cc:952: NewMerge();
On 2016/10/04 09:43:10, Michael Starzinger wrote:
> nit: Shouldn't we only create the singleton merge node in the case we have a
> slow path (i.e. move it into the below conditional)?

Done.

https://codereview.chromium.org/2385123002/diff/1/src/compiler/bytecode-graph...
src/compiler/bytecode-graph-builder.cc:1001: NewMerge();
On 2016/10/04 09:43:10, Michael Starzinger wrote:
> nit: Likewise.

Done.

[Leszek Swirski, 2016-10-04 10:47:10]

The CQ bit was unchecked by leszeks@chromium.org

[Leszek Swirski, 2016-10-04 10:47:10]

The CQ bit was checked by leszeks@chromium.org

[Leszek Swirski, 2016-10-04 10:47:11]

The patchset sent to the CQ was uploaded after l-g-t-m from
mstarzinger@chromium.org
Link to the patchset: https://codereview.chromium.org/2385123002/#ps20001
(title: "Remove singleton merge when there is no slow path")

[commit-bot: I haz the power, 2016-10-04 10:47:13]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-04 11:07:58]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-10-04 11:08:20]

Description was changed from

==========
[ignition] Fix building lookup graph when search depth is 0

In some (rare) cases, the context depth passed to a dynamic variable lookup can
be zero. In these cases, the fast path for the lookup (i.e. load from context or
global) can always be taken, as there is no need to search the current context.

However, with no slow path checks, the bytecode graph builder had a null
environment for the slow path, causing segfaults when this graph was built.

This patch adds a null check for the slow path environment, and skips building
the slow path if the environment is null.

BUG=chromium:652186
==========

to

==========
[ignition] Fix building lookup graph when search depth is 0

In some (rare) cases, the context depth passed to a dynamic variable lookup can
be zero. In these cases, the fast path for the lookup (i.e. load from context or
global) can always be taken, as there is no need to search the current context.

However, with no slow path checks, the bytecode graph builder had a null
environment for the slow path, causing segfaults when this graph was built.

This patch adds a null check for the slow path environment, and skips building
the slow path if the environment is null.

BUG=chromium:652186

Committed: https://crrev.com/4ad35791199a48e2e503b0f6193f93ab3141be00
Cr-Commit-Position: refs/heads/master@{#39949}
==========

[commit-bot: I haz the power, 2016-10-04 11:08:21]

Patchset 2 (id:??) landed as
https://crrev.com/4ad35791199a48e2e503b0f6193f93ab3141be00
Cr-Commit-Position: refs/heads/master@{#39949}