Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(439)

Issue 1400753003: [Interpreter] Add array literal support. (Closed)

Created:
5 years, 2 months ago by rmcilroy
Modified:
5 years, 2 months ago
CC:
v8-reviews_googlegroups.com
Base URL:
https://chromium.googlesource.com/v8/v8.git@int_literal
Target Ref:
refs/pending/heads/master
Project:
v8
Visibility:
Public.

Description

[Interpreter] Add array literal support. Adds array literal support to the interpreter. Currently constructed array elements don't have type feedback slots, so also adds support for generic keyed store operations. Adds the following bytecodes: - CreateArrayLiteral - KeyedStoreICGeneric BUG=v8:4280 LOG=N Committed: https://crrev.com/6a10a9af3b954e2e530bfd9fd1604a63f1f70735 Cr-Commit-Position: refs/heads/master@{#31240}

Patch Set 1 #

Patch Set 2 : #

Total comments: 2

Patch Set 3 : Rebase #

Patch Set 4 : Add empty array test case #

Patch Set 5 : Rebase #

Unified diffs Side-by-side diffs Delta from patch set Stats (+324 lines, -4 lines) Patch
M src/compiler/bytecode-graph-builder.cc View 1 2 3 4 1 chunk +12 lines, -0 lines 0 comments Download
M src/compiler/interpreter-assembler.h View 3 chunks +5 lines, -0 lines 0 comments Download
M src/compiler/interpreter-assembler.cc View 1 2 3 4 3 chunks +27 lines, -0 lines 0 comments Download
M src/compiler/raw-machine-assembler.h View 1 2 3 4 1 chunk +3 lines, -0 lines 0 comments Download
M src/compiler/raw-machine-assembler.cc View 1 2 3 4 1 chunk +16 lines, -0 lines 0 comments Download
M src/interpreter/bytecode-array-builder.h View 1 2 3 4 3 chunks +8 lines, -2 lines 0 comments Download
M src/interpreter/bytecode-array-builder.cc View 1 2 3 4 2 chunks +20 lines, -0 lines 0 comments Download
M src/interpreter/bytecode-generator.cc View 1 2 3 4 2 chunks +40 lines, -1 line 0 comments Download
M src/interpreter/bytecodes.h View 1 2 3 4 2 chunks +5 lines, -0 lines 0 comments Download
M src/interpreter/interpreter.cc View 1 2 3 4 2 chunks +41 lines, -0 lines 0 comments Download
M test/cctest/interpreter/test-bytecode-generator.cc View 1 2 3 4 1 chunk +107 lines, -0 lines 0 comments Download
M test/cctest/interpreter/test-interpreter.cc View 1 2 3 4 2 chunks +35 lines, -0 lines 0 comments Download
M test/unittests/interpreter/bytecode-array-builder-unittest.cc View 1 2 3 4 1 chunk +5 lines, -1 line 0 comments Download

Messages

Total messages: 23 (9 generated)
commit-bot: I haz the power
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1400753003/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1400753003/20001
5 years, 2 months ago (2015-10-09 10:56:13 UTC) #2
rmcilroy
Benedikt: Please review compiler/ changes Orion: Please review interpreter/ changes Michael Stanton: Please review generic ...
5 years, 2 months ago (2015-10-09 10:57:34 UTC) #4
commit-bot: I haz the power
Dry run: Try jobs failed on following builders: v8_mac_rel on tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_mac_rel/builds/10528)
5 years, 2 months ago (2015-10-09 11:06:06 UTC) #6
oth
Cool! test262 trembles... lgtm modulo additional test request. Thanks. https://codereview.chromium.org/1400753003/diff/20001/test/cctest/interpreter/test-interpreter.cc File test/cctest/interpreter/test-interpreter.cc (right): https://codereview.chromium.org/1400753003/diff/20001/test/cctest/interpreter/test-interpreter.cc#newcode1585 test/cctest/interpreter/test-interpreter.cc:1585: ...
5 years, 2 months ago (2015-10-09 13:07:35 UTC) #7
oth
Cool! test262 trembles... lgtm on the interpreter front modulo additional test request. Thanks.
5 years, 2 months ago (2015-10-09 13:07:54 UTC) #8
commit-bot: I haz the power
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1400753003/60001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1400753003/60001
5 years, 2 months ago (2015-10-09 13:12:28 UTC) #10
rmcilroy
https://codereview.chromium.org/1400753003/diff/20001/test/cctest/interpreter/test-interpreter.cc File test/cctest/interpreter/test-interpreter.cc (right): https://codereview.chromium.org/1400753003/diff/20001/test/cctest/interpreter/test-interpreter.cc#newcode1585 test/cctest/interpreter/test-interpreter.cc:1585: std::make_pair("return [1, 3, 2][1];\n", On 2015/10/09 13:07:34, oth wrote: ...
5 years, 2 months ago (2015-10-09 13:14:02 UTC) #13
commit-bot: I haz the power
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1400753003/80001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1400753003/80001
5 years, 2 months ago (2015-10-09 13:14:04 UTC) #14
commit-bot: I haz the power
Dry run: This issue passed the CQ dry run.
5 years, 2 months ago (2015-10-09 13:35:14 UTC) #16
Benedikt Meurer
compiler is LGTM.
5 years, 2 months ago (2015-10-12 17:53:46 UTC) #17
mvstanton
LGTM. Per our discussion earlier, the array literal non-constant element initialization case will soon have ...
5 years, 2 months ago (2015-10-13 09:17:23 UTC) #18
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1400753003/100001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1400753003/100001
5 years, 2 months ago (2015-10-13 13:38:58 UTC) #21
commit-bot: I haz the power
Committed patchset #5 (id:100001)
5 years, 2 months ago (2015-10-13 14:00:46 UTC) #22
commit-bot: I haz the power
5 years, 2 months ago (2015-10-13 14:01:04 UTC) #23
Message was sent while issue was closed.
Patchset 5 (id:??) landed as
https://crrev.com/6a10a9af3b954e2e530bfd9fd1604a63f1f70735
Cr-Commit-Position: refs/heads/master@{#31240}

Powered by Google App Engine
This is Rietveld 408576698