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

Issue 2821014: Add movw and movt support for ARMv7. This includes some code from... (Closed)

Created:
10 years, 6 months ago by Erik Corry
Modified:
9 years, 7 months ago
CC:
v8-dev
Visibility:
Public.

Description

Add movw and movt support for ARMv7. This includes some code from Zhang Kun. For now we only emit movw and movt in places where no relocation is needed. Small performance boost (around 0.5%). Also adds support for turning ALU operations (eor etc.) with large immediates into mvn or movw followed by a register-based ALU operation. Committed: http://code.google.com/p/v8/source/detail?r=4913

Patch Set 1 #

Total comments: 4
Unified diffs Side-by-side diffs Delta from patch set Stats (+116 lines, -12 lines) Patch
M src/arm/assembler-arm.h View 2 chunks +9 lines, -0 lines 1 comment Download
M src/arm/assembler-arm.cc View 8 chunks +47 lines, -7 lines 0 comments Download
M src/arm/constants-arm.h View 1 chunk +3 lines, -0 lines 0 comments Download
M src/arm/disasm-arm.cc View 5 chunks +26 lines, -3 lines 1 comment Download
M src/arm/simulator-arm.cc View 2 chunks +7 lines, -2 lines 0 comments Download
M src/jsregexp.cc View 1 chunk +2 lines, -0 lines 1 comment Download
M test/cctest/test-disasm-arm.cc View 1 chunk +22 lines, -0 lines 1 comment Download

Messages

Total messages: 2 (0 generated)
Erik Corry
10 years, 6 months ago (2010-06-21 19:25:51 UTC) #1
Søren Thygesen Gjesse
10 years, 6 months ago (2010-06-21 21:39:26 UTC) #2
LGTM

http://codereview.chromium.org/2821014/diff/1/3
File src/arm/assembler-arm.h (right):

http://codereview.chromium.org/2821014/diff/1/3#newcode784
src/arm/assembler-arm.h:784: // Movw is just mov with the relevant immediate.
We don't need movw which just asserts a 16-bit immediate and calls mov, to
ensure only one instruction is generated?

http://codereview.chromium.org/2821014/diff/1/5
File src/arm/disasm-arm.cc (right):

http://codereview.chromium.org/2821014/diff/1/5#newcode386
src/arm/disasm-arm.cc:386: out_buffer_pos_ += v8i::OS::SNPrintF(out_buffer_ +
out_buffer_pos_,
According to the ARM reference manual the assembler syntax is

  MOVT<c><q> <Rd>, #<imm16>

(without the trailing 0's)

http://codereview.chromium.org/2821014/diff/1/7
File src/jsregexp.cc (right):

http://codereview.chromium.org/2821014/diff/1/7#newcode1750
src/jsregexp.cc:1750: // For 2-character preloads in ASCII mode we also use a 16
bit load with
This comment should maybe cover both the ASCII and two byte condition.

http://codereview.chromium.org/2821014/diff/1/8
File test/cctest/test-disasm-arm.cc (right):

http://codereview.chromium.org/2821014/diff/1/8#newcode293
test/cctest/test-disasm-arm.cc:293: 
Missing movt test?

Powered by Google App Engine
This is Rietveld 408576698