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

Issue 9015020: Make sure transitioned arrays efficiently call builtin Array functions (Closed)

Created:
8 years, 11 months ago by danno
Modified:
8 years, 11 months ago
Reviewers:
fschneider
CC:
v8-dev
Visibility:
Public.

Description

Make sure transitioned arrays efficiently call builtin Array functions Loosen the requirement for Map equivalency on several map checks, including checks up the prototype chain, that are not sensitive to ElementsKinds. These selected map checks should also match against FAST_DOUBLE_ELEMENT and FAST_ELEMENT transitions of the original map. This specifically helps all variants of transitioned JSArrays to still efficiently call builtins like push, pop and sort. BUG=none TEST=none Committed: http://code.google.com/p/v8/source/detail?r=10331 Committed: http://code.google.com/p/v8/source/detail?r=10356

Patch Set 1 #

Patch Set 2 : disable flag #

Patch Set 3 : Add crankshaft support #

Patch Set 4 : Remove debugging output #

Patch Set 5 : Remove debug code #

Patch Set 6 : Implement all platforms #

Total comments: 4

Patch Set 7 : Tweaks #

Total comments: 10

Patch Set 8 : Review feedback #

Patch Set 9 : Rename enum #

Total comments: 16

Patch Set 10 : final version #

Patch Set 11 : Merge with latest #

Patch Set 12 : Stop crashing #

Patch Set 13 : check diff #

Patch Set 14 : fix perf regression #

Total comments: 1

Patch Set 15 : merge with latest #

Unified diffs Side-by-side diffs Delta from patch set Stats (+305 lines, -156 lines) Patch
M src/arm/lithium-codegen-arm.h View 1 2 3 4 5 6 7 8 10 2 chunks +4 lines, -1 line 0 comments Download
M src/arm/lithium-codegen-arm.cc View 1 2 3 4 5 6 7 8 10 4 chunks +22 lines, -9 lines 0 comments Download
M src/arm/macro-assembler-arm.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 2 chunks +17 lines, -6 lines 0 comments Download
M src/arm/macro-assembler-arm.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 2 chunks +36 lines, -5 lines 0 comments Download
src/arm/stub-cache-arm.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 6 chunks +13 lines, -27 lines 0 comments Download
M src/hydrogen.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 6 chunks +13 lines, -7 lines 0 comments Download
M src/hydrogen-instructions.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 4 chunks +17 lines, -4 lines 0 comments Download
M src/ia32/lithium-codegen-ia32.h View 1 2 3 4 5 6 7 8 10 2 chunks +4 lines, -1 line 0 comments Download
M src/ia32/lithium-codegen-ia32.cc View 1 2 3 4 5 6 7 8 10 4 chunks +19 lines, -10 lines 0 comments Download
M src/ia32/macro-assembler-ia32.h View 1 2 3 4 5 6 7 8 10 2 chunks +14 lines, -3 lines 0 comments Download
M src/ia32/macro-assembler-ia32.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 2 chunks +36 lines, -3 lines 0 comments Download
M src/ia32/stub-cache-ia32.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 6 chunks +12 lines, -28 lines 0 comments Download
M src/lithium.h View 1 2 3 4 5 2 chunks +1 line, -3 lines 0 comments Download
M src/objects.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 2 chunks +6 lines, -1 line 0 comments Download
M src/objects.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 2 chunks +4 lines, -2 lines 0 comments Download
M src/x64/lithium-codegen-x64.h View 1 2 3 4 5 6 7 8 10 2 chunks +5 lines, -2 lines 0 comments Download
M src/x64/lithium-codegen-x64.cc View 1 2 3 4 5 6 7 8 10 4 chunks +18 lines, -10 lines 0 comments Download
M src/x64/macro-assembler-x64.h View 1 2 3 4 5 6 7 8 10 2 chunks +16 lines, -5 lines 0 comments Download
M src/x64/macro-assembler-x64.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 2 chunks +36 lines, -3 lines 0 comments Download
M src/x64/stub-cache-x64.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 6 chunks +12 lines, -26 lines 0 comments Download

Messages

Total messages: 6 (0 generated)
danno
PTAL
8 years, 11 months ago (2012-01-03 16:01:08 UTC) #1
fschneider
Most plaform-specific feedback applies to the other platforms as well. http://codereview.chromium.org/9015020/diff/7001/src/hydrogen-instructions.cc File src/hydrogen-instructions.cc (right): http://codereview.chromium.org/9015020/diff/7001/src/hydrogen-instructions.cc#newcode1 ...
8 years, 11 months ago (2012-01-04 09:20:18 UTC) #2
danno
Feedback addressed, please take another look. http://codereview.chromium.org/9015020/diff/7001/src/hydrogen-instructions.cc File src/hydrogen-instructions.cc (right): http://codereview.chromium.org/9015020/diff/7001/src/hydrogen-instructions.cc#newcode1 src/hydrogen-instructions.cc:1: // Copyright 2012 ...
8 years, 11 months ago (2012-01-04 10:42:15 UTC) #3
fschneider
lgtm http://codereview.chromium.org/9015020/diff/14002/src/arm/macro-assembler-arm.cc File src/arm/macro-assembler-arm.cc (right): http://codereview.chromium.org/9015020/diff/14002/src/arm/macro-assembler-arm.cc#newcode2004 src/arm/macro-assembler-arm.cc:2004: Map* transitioned_fast_element_map( Handle<Map> http://codereview.chromium.org/9015020/diff/14002/src/arm/macro-assembler-arm.cc#newcode2013 src/arm/macro-assembler-arm.cc:2013: Map* transitioned_double_map( Handle<Map> ...
8 years, 11 months ago (2012-01-04 12:17:51 UTC) #4
danno
Please take another look http://codereview.chromium.org/9015020/diff/14002/src/arm/macro-assembler-arm.cc File src/arm/macro-assembler-arm.cc (right): http://codereview.chromium.org/9015020/diff/14002/src/arm/macro-assembler-arm.cc#newcode2004 src/arm/macro-assembler-arm.cc:2004: Map* transitioned_fast_element_map( On 2012/01/04 12:17:51, ...
8 years, 11 months ago (2012-01-05 13:41:57 UTC) #5
fschneider
8 years, 11 months ago (2012-01-05 17:15:10 UTC) #6
LGTM.

http://codereview.chromium.org/9015020/diff/27001/src/hydrogen-instructions.h
File src/hydrogen-instructions.h (right):

http://codereview.chromium.org/9015020/diff/27001/src/hydrogen-instructions.h...
src/hydrogen-instructions.h:1964: bool has_element_transitions_;
This will make each HCheckMap one word larger. Consider packing mode_ and
has_element_transitions_ one bitfield, since they are both only 1-bit values.

Powered by Google App Engine
This is Rietveld 408576698