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

Issue 993002: Port of optimized ICs for external and pixel arrays from ia32 to ARM. (Closed)

Created:
10 years, 9 months ago by Vyacheslav Egorov (Chromium)
Modified:
9 years, 7 months ago
CC:
v8-dev
Visibility:
Public.

Description

Port of optimized ICs for external and pixel arrays from ia32 to ARM. Committed: http://code.google.com/p/v8/source/detail?r=4228

Patch Set 1 #

Total comments: 37

Patch Set 2 : '' #

Patch Set 3 : '' #

Patch Set 4 : '' #

Patch Set 5 : '' #

Total comments: 7
Unified diffs Side-by-side diffs Delta from patch set Stats (+1488 lines, -327 lines) Patch
M src/arm/assembler-arm.h View 2 chunks +27 lines, -6 lines 0 comments Download
M src/arm/assembler-arm.cc View 1 2 3 4 3 chunks +179 lines, -20 lines 0 comments Download
M src/arm/codegen-arm.h View 1 chunk +39 lines, -0 lines 2 comments Download
M src/arm/codegen-arm.cc View 1 18 chunks +19 lines, -118 lines 1 comment Download
M src/arm/constants-arm.h View 1 2 3 4 3 chunks +18 lines, -2 lines 0 comments Download
M src/arm/constants-arm.cc View 1 2 3 4 2 chunks +21 lines, -3 lines 0 comments Download
M src/arm/disasm-arm.cc View 1 2 3 4 5 chunks +164 lines, -76 lines 0 comments Download
M src/arm/ic-arm.cc View 1 2 3 4 8 chunks +689 lines, -8 lines 4 comments Download
M src/arm/macro-assembler-arm.h View 2 chunks +12 lines, -0 lines 0 comments Download
M src/arm/macro-assembler-arm.cc View 2 chunks +53 lines, -1 line 0 comments Download
M src/arm/simulator-arm.h View 1 chunk +5 lines, -0 lines 0 comments Download
M src/arm/simulator-arm.cc View 1 2 3 4 8 chunks +253 lines, -93 lines 0 comments Download
M src/globals.h View 1 chunk +9 lines, -0 lines 0 comments Download

Messages

Total messages: 5 (0 generated)
Vyacheslav Egorov (Chromium)
10 years, 9 months ago (2010-03-15 18:19:46 UTC) #1
Mads Ager (chromium)
Erik, ARM stuff inside. :) Could you review this one? Thanks, -- Mads
10 years, 9 months ago (2010-03-16 07:15:58 UTC) #2
Erik Corry
http://codereview.chromium.org/993002/diff/1/7 File src/arm/assembler-arm.cc (right): http://codereview.chromium.org/993002/diff/1/7#newcode1330 src/arm/assembler-arm.cc:1330: ASSERT(offset % 4 == 0); Here and in the ...
10 years, 9 months ago (2010-03-16 11:04:56 UTC) #3
Vyacheslav Egorov (Chromium)
http://codereview.chromium.org/993002/diff/1/7 File src/arm/assembler-arm.cc (right): http://codereview.chromium.org/993002/diff/1/7#newcode1330 src/arm/assembler-arm.cc:1330: ASSERT(offset % 4 == 0); On 2010/03/16 11:04:56, Erik ...
10 years, 9 months ago (2010-03-18 10:27:52 UTC) #4
Mads Ager (chromium)
10 years, 9 months ago (2010-03-23 11:46:54 UTC) #5
LGTM!

http://codereview.chromium.org/993002/diff/49001/50003
File src/arm/codegen-arm.cc (right):

http://codereview.chromium.org/993002/diff/49001/50003#newcode4753
src/arm/codegen-arm.cc:4753: // We have -1, 0 or 1, which we treat specially.
Could we clearify the comment to state that source contains a positive number
(and therefore the removal of the compare to zero is correct).

http://codereview.chromium.org/993002/diff/49001/50004
File src/arm/codegen-arm.h (right):

http://codereview.chromium.org/993002/diff/49001/50004#newcode680
src/arm/codegen-arm.h:680: // Minor key encoding in 16 bits.
This is not new code, but we do not seem to use these bit fields.  Could you
change this to follow the pattern used for the other stubs.  Define BitFields
here and create the MinorKey using bitwise or of the bitfield encodings?

http://codereview.chromium.org/993002/diff/49001/50004#newcode687
src/arm/codegen-arm.h:687: return  the_int_.code() +
Please remove extra space.

http://codereview.chromium.org/993002/diff/49001/50010
File src/arm/ic-arm.cc (right):

http://codereview.chromium.org/993002/diff/49001/50010#newcode650
src/arm/ic-arm.cc:650: const int meaningfull_bits = kBitsPerInt - leading_zeroes
- 1;
meaningfull -> meaningful

http://codereview.chromium.org/993002/diff/49001/50010#newcode697
src/arm/ic-arm.cc:697: // Check that the object is a JS object. Load map into r2
End comment with period.

http://codereview.chromium.org/993002/diff/49001/50010#newcode719
src/arm/ic-arm.cc:719: 
Remove empty line?

http://codereview.chromium.org/993002/diff/49001/50010#newcode797
src/arm/ic-arm.cc:797: __ Jump(stub.GetCode(), RelocInfo::CODE_TARGET);
Please use "__ TailCallStub(&stub);"

Powered by Google App Engine
This is Rietveld 408576698