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

Issue 1750017: Port string keyed load IC improvements (r4444) to x64. (Closed)

Created:
10 years, 8 months ago by podivilov
Modified:
9 years, 3 months ago
Reviewers:
Vitaly Repeshko
CC:
v8-dev
Visibility:
Public.

Description

Patch Set 1 #

Patch Set 2 : '' #

Patch Set 3 : '' #

Total comments: 5

Patch Set 4 : '' #

Unified diffs Side-by-side diffs Delta from patch set Stats (+368 lines, -226 lines) Patch
M src/ia32/codegen-ia32.h View 1 2 3 2 chunks +6 lines, -4 lines 0 comments Download
M src/ia32/codegen-ia32.cc View 1 2 3 6 chunks +10 lines, -9 lines 0 comments Download
M src/ia32/ic-ia32.cc View 1 2 3 4 chunks +8 lines, -10 lines 0 comments Download
M src/x64/codegen-x64.h View 1 2 3 6 chunks +43 lines, -10 lines 0 comments Download
M src/x64/codegen-x64.cc View 1 2 3 18 chunks +211 lines, -174 lines 0 comments Download
M src/x64/ic-x64.cc View 1 2 3 1 chunk +58 lines, -19 lines 0 comments Download
M test/mjsunit/string-index.js View 1 2 3 1 chunk +32 lines, -0 lines 0 comments Download

Messages

Total messages: 2 (0 generated)
podivilov
10 years, 8 months ago (2010-04-27 16:14:04 UTC) #1
Vitaly Repeshko
10 years, 8 months ago (2010-04-28 12:38:27 UTC) #2
LGTM with comments addressed.


Thanks,
Vitaly

http://codereview.chromium.org/1750017/diff/15001/16005
File src/x64/ic-x64.cc (right):

http://codereview.chromium.org/1750017/diff/15001/16005#newcode579
src/x64/ic-x64.cc:579: __ Cmp(scratch, Factory::heap_number_map());
CompareRoot?

http://codereview.chromium.org/1750017/diff/15001/16005#newcode582
src/x64/ic-x64.cc:582: __ Cmp(code, Factory::nan_value());
CompareRoot?

http://codereview.chromium.org/1750017/diff/15001/16005#newcode586
src/x64/ic-x64.cc:586: __ Move(rax, Factory::undefined_value());
LoadRoot?

http://codereview.chromium.org/1750017/diff/15001/16001
File test/mjsunit/string-index.js (right):

http://codereview.chromium.org/1750017/diff/15001/16001#newcode199
test/mjsunit/string-index.js:199: var keys = [0, 1.0];
I don't think "1.0" tests what you want.

http://codereview.chromium.org/1750017/diff/15001/16001#newcode227
test/mjsunit/string-index.js:227: }
It'd be cool to have two-byte strings covered as well.

Powered by Google App Engine
This is Rietveld 408576698