[ignition] Add global lookup fast path to generated turbofan graph

[ignition] Add global lookup fast path to generated turbofan graph

Adds a fast-path test and branch for the turbofan graph generated by
BytecodeGraphBuilder for dynamic global lookups.

BUG=v8:5263
Committed: https://crrev.com/5352e7a54467537cb93b0a6eafd500b57508670b
Cr-Commit-Position: refs/heads/master@{#39882}

[Leszek Swirski, 2016-09-28 15:30:45]

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

[commit-bot: I haz the power, 2016-09-28 15:30:51]

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-09-28 15:36:19]

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

[commit-bot: I haz the power, 2016-09-28 15:36:26]

Dry run: 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-09-28 16:02:04]

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

[commit-bot: I haz the power, 2016-09-28 16:02:05]

Dry run: This issue passed the CQ dry run.

[Leszek Swirski, 2016-09-29 14:35:08]

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

[commit-bot: I haz the power, 2016-09-29 14:35:12]

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-09-29 14:35:58]

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

[Michael Starzinger, 2016-09-29 14:52:25]

Looking good.

https://codereview.chromium.org/2374983003/diff/40001/src/compiler/bytecode-g...
File src/compiler/bytecode-graph-builder.cc (right):

https://codereview.chromium.org/2374983003/diff/40001/src/compiler/bytecode-g...
src/compiler/bytecode-graph-builder.cc:789: Node*
BytecodeGraphBuilder::BuildLoadGlobal(int feedback_slot_operand,
nit: Could we instead of passing the operand index just pass the operand value
(i.e. {int slod_id} in this case) directly? IMHO this would be easier to read. I
think the duplicated calls to {GetIndexOperand} are acceptable and they would be
closer to the actual visitation method.

https://codereview.chromium.org/2374983003/diff/40001/src/compiler/bytecode-g...
src/compiler/bytecode-graph-builder.cc:932: DCHECK(slow_environment != nullptr);
nit: DCHECK_NOT_NULL

[commit-bot: I haz the power, 2016-09-29 15:17:00]

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

[commit-bot: I haz the power, 2016-09-29 15:17:01]

Dry run: This issue passed the CQ dry run.

[Leszek Swirski, 2016-09-29 15:40:05]

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

[commit-bot: I haz the power, 2016-09-29 15:40:07]

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-09-29 15:40:18]

Thanks for the review, new one is up.

https://codereview.chromium.org/2374983003/diff/40001/src/compiler/bytecode-g...
File src/compiler/bytecode-graph-builder.cc (right):

https://codereview.chromium.org/2374983003/diff/40001/src/compiler/bytecode-g...
src/compiler/bytecode-graph-builder.cc:789: Node*
BytecodeGraphBuilder::BuildLoadGlobal(int feedback_slot_operand,
On 2016/09/29 14:52:25, Michael Starzinger wrote:
> nit: Could we instead of passing the operand index just pass the operand value
> (i.e. {int slod_id} in this case) directly? IMHO this would be easier to read.
I
> think the duplicated calls to {GetIndexOperand} are acceptable and they would
be
> closer to the actual visitation method.

Done. I was on the fence about this too, thanks!

https://codereview.chromium.org/2374983003/diff/40001/src/compiler/bytecode-g...
src/compiler/bytecode-graph-builder.cc:932: DCHECK(slow_environment != nullptr);
On 2016/09/29 14:52:25, Michael Starzinger wrote:
> nit: DCHECK_NOT_NULL

Done.

[Michael Starzinger, 2016-09-29 15:45:07]

LGTM.

[Leszek Swirski, 2016-09-29 15:47:00]

The CQ bit was unchecked by leszeks@chromium.org

[Leszek Swirski, 2016-09-29 15:47:00]

The CQ bit was checked by leszeks@chromium.org

[commit-bot: I haz the power, 2016-09-29 15:47:07]

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-09-29 16:08:33]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-09-29 16:08:57]

Description was changed from

==========
[ignition] Add global lookup fast path to generated turbofan graph

Adds a fast-path test and branch for the turbofan graph generated by
BytecodeGraphBuilder for dynamic global lookups.

BUG=v8:5263
==========

to

==========
[ignition] Add global lookup fast path to generated turbofan graph

Adds a fast-path test and branch for the turbofan graph generated by
BytecodeGraphBuilder for dynamic global lookups.

BUG=v8:5263

Committed: https://crrev.com/5352e7a54467537cb93b0a6eafd500b57508670b
Cr-Commit-Position: refs/heads/master@{#39882}
==========

[commit-bot: I haz the power, 2016-09-29 16:08:58]

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