[turbofan] Optimize loads from the global object in JSTypeFeedbackSpecializer.

[turbofan] Optimize loads from the global object in JSTypeFeedbackSpecializer.

Uses lazy deoptimization and code dependencies to introduce loads
from property cells and also to promote globals to constants.

R=mstarzinger@chromium.org
BUG=

Committed: https://crrev.com/aae4a62d07e839455b1d0ad4fa512cc5d48a1a68
Cr-Commit-Position: refs/heads/master@{#28057}

[Michael Starzinger, 2015-04-21 12:34:02]

LGTM with comments.

https://codereview.chromium.org/1063513003/diff/20001/src/compiler/js-type-fe...
File src/compiler/js-type-feedback.cc (right):

https://codereview.chromium.org/1063513003/diff/20001/src/compiler/js-type-fe...
src/compiler/js-type-feedback.cc:232:
String::Flatten(Handle<String>::cast(constant_value));
rant: We need to flatten strings here? SRSLY? OMG!

https://codereview.chromium.org/1063513003/diff/20001/src/compiler/pipeline.cc
File src/compiler/pipeline.cc (right):

https://codereview.chromium.org/1063513003/diff/20001/src/compiler/pipeline.c...
src/compiler/pipeline.cc:523: if (data->info()->has_global_object()) {
This is only valid if we don't share TurboFan code accross native contexts. We
are starting to add more and more assumptions like this. I think it's time to
introduce some sort of "sharing mode" that let's the pipeline (and the different
reducers) know what level of sharing is expected. WDYT?

Can we leave TODO in that regard here and introduce the "sharing mode" in a
follow-up CL?

https://codereview.chromium.org/1063513003/diff/20001/test/mjsunit/global-del...
File test/mjsunit/global-delete.js (right):

https://codereview.chromium.org/1063513003/diff/20001/test/mjsunit/global-del...
test/mjsunit/global-delete.js:3: // found in the LICENSE file.
suggestion: Should we move this file into the "compiler" supdirectory of
mjsunit? Because it seems to be targeting optimized code specifically.

https://codereview.chromium.org/1063513003/diff/20001/test/mjsunit/global-var...
File test/mjsunit/global-var-delete.js (right):

https://codereview.chromium.org/1063513003/diff/20001/test/mjsunit/global-var...
test/mjsunit/global-var-delete.js:3: // found in the LICENSE file.
suggestion: Should we move this file into the "compiler" supdirectory of
mjsunit? Because it seems to be targeting optimized code specifically.

https://codereview.chromium.org/1063513003/diff/20001/test/unittests/compiler...
File test/unittests/compiler/js-type-feedback-unittest.cc (right):

https://codereview.chromium.org/1063513003/diff/20001/test/unittests/compiler...
test/unittests/compiler/js-type-feedback-unittest.cc:71: USE(result);
nit: result.Check() or at least result.Assert() here.

https://codereview.chromium.org/1063513003/diff/20001/test/unittests/compiler...
test/unittests/compiler/js-type-feedback-unittest.cc:102: for (int i =
FLAG_turbo_deoptimization = 0; i < 2; \
nit: Assigning integer to boolean looks fishy.

https://codereview.chromium.org/1063513003/diff/20001/test/unittests/compiler...
test/unittests/compiler/js-type-feedback-unittest.cc:103:
FLAG_turbo_deoptimization = ++i)
nit: Likewise.

[titzer, 2015-04-21 12:56:52]

https://codereview.chromium.org/1063513003/diff/20001/src/compiler/js-type-fe...
File src/compiler/js-type-feedback.cc (right):

https://codereview.chromium.org/1063513003/diff/20001/src/compiler/js-type-fe...
src/compiler/js-type-feedback.cc:232:
String::Flatten(Handle<String>::cast(constant_value));
On 2015/04/21 12:34:01, Michael Starzinger wrote:
> rant: We need to flatten strings here? SRSLY? OMG!

Agreed, cribbed this one from Crankyshaft.

https://codereview.chromium.org/1063513003/diff/20001/src/compiler/pipeline.cc
File src/compiler/pipeline.cc (right):

https://codereview.chromium.org/1063513003/diff/20001/src/compiler/pipeline.c...
src/compiler/pipeline.cc:523: if (data->info()->has_global_object()) {
On 2015/04/21 12:34:01, Michael Starzinger wrote:
> This is only valid if we don't share TurboFan code accross native contexts. We
> are starting to add more and more assumptions like this. I think it's time to
> introduce some sort of "sharing mode" that let's the pipeline (and the
different
> reducers) know what level of sharing is expected. WDYT?
> 
> Can we leave TODO in that regard here and introduce the "sharing mode" in a
> follow-up CL?

Acknowledge, agreed. I have sketched out a specialization mode struct in another
CL and dropped in a TODO.

https://codereview.chromium.org/1063513003/diff/20001/test/mjsunit/global-del...
File test/mjsunit/global-delete.js (right):

https://codereview.chromium.org/1063513003/diff/20001/test/mjsunit/global-del...
test/mjsunit/global-delete.js:3: // found in the LICENSE file.
On 2015/04/21 12:34:01, Michael Starzinger wrote:
> suggestion: Should we move this file into the "compiler" supdirectory of
> mjsunit? Because it seems to be targeting optimized code specifically.

Done.

https://codereview.chromium.org/1063513003/diff/20001/test/unittests/compiler...
File test/unittests/compiler/js-type-feedback-unittest.cc (right):

https://codereview.chromium.org/1063513003/diff/20001/test/unittests/compiler...
test/unittests/compiler/js-type-feedback-unittest.cc:71: USE(result);
On 2015/04/21 12:34:01, Michael Starzinger wrote:
> nit: result.Check() or at least result.Assert() here.

Done.

https://codereview.chromium.org/1063513003/diff/20001/test/unittests/compiler...
test/unittests/compiler/js-type-feedback-unittest.cc:102: for (int i =
FLAG_turbo_deoptimization = 0; i < 2; \
On 2015/04/21 12:34:01, Michael Starzinger wrote:
> nit: Assigning integer to boolean looks fishy.

Not sure if a static cast really would make any difference, since I think the
C++ standards defines bools as integers with values 0 or 1.

[titzer, 2015-04-22 07:05:40]

The CQ bit was checked by titzer@chromium.org

[titzer, 2015-04-22 07:05:40]

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/1063513003/#ps40001
(title: " ")

[commit-bot: I haz the power, 2015-04-22 07:05:53]

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

[commit-bot: I haz the power, 2015-04-22 07:18:35]

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

[commit-bot: I haz the power, 2015-04-22 07:18:36]

Try jobs failed on following builders:
  v8_linux_nodcheck_rel on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_nodcheck_rel/build...)

[titzer, 2015-04-22 12:27:12]

On 2015/04/22 07:18:36, I haz the power (commit-bot) wrote:
> Try jobs failed on following builders:
>   v8_linux_nodcheck_rel on tryserver.v8 (JOB_FAILED,
>
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_nodcheck_rel/build...)

Added ScriptContext check. PTAL.

[Michael Starzinger, 2015-04-22 12:42:18]

Still LGTM. Just one nit.

https://codereview.chromium.org/1063513003/diff/80001/test/cctest/test-decls.cc
File test/cctest/test-decls.cc (right):

https://codereview.chromium.org/1063513003/diff/80001/test/cctest/test-decls....
test/cctest/test-decls.cc:926: TEST(CrossScriptGlobal0) {
nit: Do we need to add this test, since it's just apiece of the
CrossScriptGlobal below?

[titzer, 2015-04-22 12:51:33]

On 2015/04/22 12:42:18, Michael Starzinger wrote:
> Still LGTM. Just one nit.
> 
>
https://codereview.chromium.org/1063513003/diff/80001/test/cctest/test-decls.cc
> File test/cctest/test-decls.cc (right):
> 
>
https://codereview.chromium.org/1063513003/diff/80001/test/cctest/test-decls....
> test/cctest/test-decls.cc:926: TEST(CrossScriptGlobal0) {
> nit: Do we need to add this test, since it's just apiece of the
> CrossScriptGlobal below?

Done.

[titzer, 2015-04-22 12:51:43]

The CQ bit was checked by titzer@chromium.org

[titzer, 2015-04-22 12:51:43]

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/1063513003/#ps100001
(title: " ")

[commit-bot: I haz the power, 2015-04-22 12:51:54]

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

[commit-bot: I haz the power, 2015-04-22 13:14:11]

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

[commit-bot: I haz the power, 2015-04-22 13:14:12]

Try jobs failed on following builders:
  v8_linux_arm64_rel on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_arm64_rel/builds/5056)

[titzer, 2015-04-23 13:22:33]

The CQ bit was checked by titzer@chromium.org

[commit-bot: I haz the power, 2015-04-23 13:22:44]

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

[commit-bot: I haz the power, 2015-04-23 13:24:06]

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

[commit-bot: I haz the power, 2015-04-23 13:24:07]

Try jobs failed on following builders:
  v8_linux_arm64_rel on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_arm64_rel/builds/5102)

[titzer, 2015-04-27 09:05:25]

The CQ bit was checked by titzer@chromium.org

[titzer, 2015-04-27 09:05:25]

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/1063513003/#ps180001
(title: " ")

[commit-bot: I haz the power, 2015-04-27 09:05:36]

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

[commit-bot: I haz the power, 2015-04-27 09:07:14]

Committed patchset #10 (id:180001)

[commit-bot: I haz the power, 2015-04-27 09:07:24]

Patchset 10 (id:??) landed as
https://crrev.com/aae4a62d07e839455b1d0ad4fa512cc5d48a1a68
Cr-Commit-Position: refs/heads/master@{#28057}

[Michael Achenbach, 2015-04-27 10:07:15]

A revert of this CL (patchset #10 id:180001) has been created in
https://codereview.chromium.org/1084533003/ by machenbach@chromium.org.

The reason for reverting is: [sheriff] Breaks nosnap debug:
http://build.chromium.org/p/client.v8/builders/V8%20Linux%20-%20nosnap%20-%20....

[titzer, 2015-04-27 10:27:56]

On 2015/04/27 10:07:15, Michael Achenbach wrote:
> A revert of this CL (patchset #10 id:180001) has been created in
> https://codereview.chromium.org/1084533003/ by mailto:machenbach@chromium.org.
> 
> The reason for reverting is: [sheriff] Breaks nosnap debug:
>
http://build.chromium.org/p/client.v8/builders/V8%20Linux%20-%20nosnap%20-%20....

That failure was due to turbo exceptions being inherently broken with nosnap
builds; we need to disable these tests temporarily.