Issue 7210057: ARM: Reduce amount of code generated for LoadIC_Megamorphic....

Issue 7210057: ARM: Reduce amount of code generated for LoadIC_Megamorphic.... (Closed)

Created:
9 years, 5 months ago by m.m.capewell

Modified:
8 years, 8 months ago

Reviewers:
Søren Thygesen Gjesse, Erik Corry

CC:
v8-dev

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

Visibility:
Public.

Description

ARM: Reduce amount of code generated for LoadIC_Megamorphic. BUG=none TEST=none

Patch Set 2 : '' #

Created: 9 years, 5 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+33 lines, -35 lines)			Patch
M	src/arm/ic-arm.cc	View	1	4 chunks	+4 lines, -4 lines	0 comments	Download
M	src/arm/stub-cache-arm.cc	View	1	5 chunks	+17 lines, -26 lines	0 comments	Download
M	src/ia32/stub-cache-ia32.cc	View	1	2 chunks	+5 lines, -2 lines	0 comments	Download
M	src/stub-cache.h	View	1	1 chunk	+2 lines, -1 line	0 comments	Download
M	src/x64/stub-cache-x64.cc	View	1	2 chunks	+5 lines, -2 lines	0 comments	Download

Messages

Total messages: 5 (0 generated)

Expand Messages | Collapse Messages

Erik Corry

lgtm http://codereview.chromium.org/7210057/diff/1/src/arm/assembler-arm.h File src/arm/assembler-arm.h (right): http://codereview.chromium.org/7210057/diff/1/src/arm/assembler-arm.h#newcode1229 src/arm/assembler-arm.h:1229: static int NumRegistersInRegList(RegList list); frames.h already has int ...

9 years, 5 months ago (2011-07-04 06:54:34 UTC) #3

Søren Thygesen Gjesse

Drive by comments. http://codereview.chromium.org/7210057/diff/1/src/ia32/stub-cache-ia32.cc File src/ia32/stub-cache-ia32.cc (right): http://codereview.chromium.org/7210057/diff/1/src/ia32/stub-cache-ia32.cc#newcode171 src/ia32/stub-cache-ia32.cc:171: USE(extra2); // The register extra2 is ...

9 years, 5 months ago (2011-07-04 07:21:19 UTC) #4

m.m.capewell

9 years, 5 months ago (2011-07-14 15:00:18 UTC) #5

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

http://codereview.chromium.org/7210057/diff/1/src/arm/assembler-arm.h#newcode...
src/arm/assembler-arm.h:1229: static int NumRegistersInRegList(RegList list);
On 2011/07/04 06:54:34, Erik Corry wrote:
> frames.h already has
> int NumRegs(RegList list)

Done.

http://codereview.chromium.org/7210057/diff/1/src/arm/stub-cache-arm.cc
File src/arm/stub-cache-arm.cc (right):

http://codereview.chromium.org/7210057/diff/1/src/arm/stub-cache-arm.cc#newco...
src/arm/stub-cache-arm.cc:72: value_off_addr - key_off_addr));
On 2011/07/04 06:54:34, Erik Corry wrote:
> Strange indentation here.  The value_off_addr should line up with the
> offsets_base_addr.  Alternatively the MemOperand should line up with the
> scratch2.

Done.

http://codereview.chromium.org/7210057/diff/1/src/ia32/stub-cache-ia32.cc
File src/ia32/stub-cache-ia32.cc (right):

http://codereview.chromium.org/7210057/diff/1/src/ia32/stub-cache-ia32.cc#new...
src/ia32/stub-cache-ia32.cc:171: USE(extra2);  // The register extra2 is not
used on the ia32 platform.
On 2011/07/04 07:21:19, Søren Gjesse wrote:
> Will a USE(extra3) be needed to compile in release mode?

Done.

http://codereview.chromium.org/7210057/diff/1/src/ia32/stub-cache-ia32.cc#new...
src/ia32/stub-cache-ia32.cc:188: ASSERT(!scratch.is(no_reg));
On 2011/07/04 07:21:19, Søren Gjesse wrote:
> Also assert that extra is not no_reg?

extra can be no_reg here, eg. ic-ia32.cc:877.

http://codereview.chromium.org/7210057/diff/1/src/x64/stub-cache-x64.cc
File src/x64/stub-cache-x64.cc (right):

http://codereview.chromium.org/7210057/diff/1/src/x64/stub-cache-x64.cc#newco...
src/x64/stub-cache-x64.cc:162: ASSERT(!scratch.is(no_reg));
On 2011/07/04 07:21:19, Søren Gjesse wrote:
> Also assert that extra is no_reg?

extra can be no_reg here, eg. ic-x64.cc:765.

Expand Messages | Collapse Messages