Issue 2826001: ARM: Be more smart about switching instructions when immediates...

Issue 2826001: ARM: Be more smart about switching instructions when immediates... (Closed)

Created:
10 years, 6 months ago by Erik Corry

Modified:
9 years, 7 months ago

Reviewers:
Søren Thygesen Gjesse

CC:
v8-dev

Base URL:
http://v8.googlecode.com/svn/branches/bleeding_edge/

Visibility:
Public.

Description

ARM: Be more smart about switching instructions when immediates don't fit in the instruction. Use ubfx and sbfx more. Committed: http://code.google.com/p/v8/source/detail?r=4855

Patch Set 1 #

Total comments: 10

Created: 10 years, 6 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Stats (+235 lines, -83 lines)			Patch
M	src/arm/assembler-arm.h	View	2 chunks	+24 lines, -0 lines	0 comments	Download
M	src/arm/assembler-arm.cc	View	4 chunks	+53 lines, -5 lines	2 comments	Download
M	src/arm/builtins-arm.cc	View	3 chunks	+10 lines, -10 lines	0 comments	Download
M	src/arm/codegen-arm.cc	View	11 chunks	+33 lines, -40 lines	6 comments	Download
M	src/arm/ic-arm.cc	View	3 chunks	+18 lines, -23 lines	0 comments	Download
M	src/arm/macro-assembler-arm.h	View	1 chunk	+8 lines, -0 lines	0 comments	Download
M	src/arm/macro-assembler-arm.cc	View	1 chunk	+54 lines, -0 lines	0 comments	Download
M	src/objects.h	View	2 chunks	+5 lines, -5 lines	0 comments	Download
M	src/utils.h	View	1 chunk	+30 lines, -0 lines	2 comments	Download

Messages

Total messages: 3 (0 generated)

Expand Messages | Collapse Messages

Søren Thygesen Gjesse

LGTM, with comments. http://codereview.chromium.org/2826001/diff/1/2 File src/arm/assembler-arm.cc (right): http://codereview.chromium.org/2826001/diff/1/2#newcode660 src/arm/assembler-arm.cc:660: // If the opcode is one ...

10 years, 6 months ago (2010-06-14 07:56:47 UTC) #2

Erik Corry

10 years, 6 months ago (2010-06-14 21:05:46 UTC) #3

http://codereview.chromium.org/2826001/diff/1/2
File src/arm/assembler-arm.cc (right):

http://codereview.chromium.org/2826001/diff/1/2#newcode660
src/arm/assembler-arm.cc:660: // If the opcode is one with a complementary
version and the complementary
On 2010/06/14 07:56:47, Søren Gjesse wrote:
> Shouldn't we have some test cases for this instruction transforming? I think
> extending test-disasm-arm.cc would do the trick.

Done.

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

http://codereview.chromium.org/2826001/diff/1/5#newcode6899
src/arm/codegen-arm.cc:6899: __ Sbfx(r4, lhs_exponent,
HeapNumber::kExponentShift, HeapNumber::kExponentBits);
On 2010/06/14 07:56:47, Søren Gjesse wrote:
> Long line.

Done.

http://codereview.chromium.org/2826001/diff/1/5#newcode6911
src/arm/codegen-arm.cc:6911: __ Sbfx(r4, rhs_exponent,
HeapNumber::kExponentShift, HeapNumber::kExponentBits);
On 2010/06/14 07:56:47, Søren Gjesse wrote:
> Long line.

Done.

http://codereview.chromium.org/2826001/diff/1/5#newcode7634
src/arm/codegen-arm.cc:7634: __ Ubfx(scratch2, scratch,
HeapNumber::kExponentShift, HeapNumber::kExponentBits);
On 2010/06/14 07:56:47, Søren Gjesse wrote:
> Long line.

Done.

http://codereview.chromium.org/2826001/diff/1/10
File src/utils.h (right):

http://codereview.chromium.org/2826001/diff/1/10#newcode52
src/utils.h:52: static inline int WhichPowerOf2(T x) {
On 2010/06/14 07:56:47, Søren Gjesse wrote:
> Shouldn't there be a test case for this?

Assert added.

Expand Messages | Collapse Messages