[turbofan] Optimize stores to global properties.

[turbofan] Optimize stores to global properties.

This adds support to also lower stores to global property cells in state
kConstant or kConstantType, where we need to deoptimize eagerly in case
we have a value/type mismatch.

Also fixes bugs in the construction of the frame states in the
AstGraphBuilder.

R=jarin@chromium.org
BUG=v8:4470
LOG=n

Committed: https://crrev.com/9d2e21775cd4f883f43d77c674fde5e91bb784f8
Cr-Commit-Position: refs/heads/master@{#31178}

[Benedikt Meurer, 2015-10-08 11:18:13]

Hey Jaro,

Patch to consistently optimize all global stores (I'll look into unifying the
deoptimize control stuff w/ JSTypeFeedbackLowering later).
Please take a look.

Thanks,
Benedikt

[Jarin, 2015-10-08 12:41:32]

lgtm. Sort of.

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

https://codereview.chromium.org/1398723002/diff/1/src/compiler/ast-graph-buil...
src/compiler/ast-graph-builder.cc:2689: if (!is_postfix)
environment()->Push(old_value);
How about making this an 'else' branch of the "if (postfix) ..." statement
above?

https://codereview.chromium.org/1398723002/diff/1/src/compiler/ast-graph-buil...
src/compiler/ast-graph-builder.cc:2690: FrameStateBeforeAndAfter
store_states(this, expr->ToNumberId());
Looking at the full-code, I am not convinced this is correct, but if it passes
the test suite, we can live with that (as any eager deopt through here should be
under a flag). Maybe we want to add a TODO here.

https://codereview.chromium.org/1398723002/diff/1/src/compiler/js-global-spec...
File src/compiler/js-global-specialization.cc (right):

https://codereview.chromium.org/1398723002/diff/1/src/compiler/js-global-spec...
src/compiler/js-global-specialization.cc:227: check, control);
Hmm, interesting code reuse...

[Benedikt Meurer, 2015-10-08 12:52:28]

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

https://codereview.chromium.org/1398723002/diff/1/src/compiler/ast-graph-buil...
src/compiler/ast-graph-builder.cc:2689: if (!is_postfix)
environment()->Push(old_value);
On 2015/10/08 12:41:32, Jarin wrote:
> How about making this an 'else' branch of the "if (postfix) ..." statement
> above?

Done.

https://codereview.chromium.org/1398723002/diff/1/src/compiler/ast-graph-buil...
src/compiler/ast-graph-builder.cc:2690: FrameStateBeforeAndAfter
store_states(this, expr->ToNumberId());
On 2015/10/08 12:41:32, Jarin wrote:
> Looking at the full-code, I am not convinced this is correct, but if it passes
> the test suite, we can live with that (as any eager deopt through here should
be
> under a flag). Maybe we want to add a TODO here.

Done.

https://codereview.chromium.org/1398723002/diff/1/src/compiler/js-global-spec...
File src/compiler/js-global-specialization.cc (right):

https://codereview.chromium.org/1398723002/diff/1/src/compiler/js-global-spec...
src/compiler/js-global-specialization.cc:227: check, control);
On 2015/10/08 12:41:32, Jarin wrote:
> Hmm, interesting code reuse...

Acknowledged.

[Benedikt Meurer, 2015-10-08 12:52:34]

The CQ bit was checked by bmeurer@chromium.org

[Benedikt Meurer, 2015-10-08 12:52:34]

The patchset sent to the CQ was uploaded after l-g-t-m from jarin@chromium.org
Link to the patchset: https://codereview.chromium.org/1398723002/#ps20001
(title: "Address Jaro's comment.")

[commit-bot: I haz the power, 2015-10-08 12:52:44]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1398723002/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1398723002/20001

[commit-bot: I haz the power, 2015-10-08 13:29:53]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2015-10-08 13:30:04]

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