Issue 8747009: Optimize Crankshaft array literal initialization from boilerplate.

Issue 8747009: Optimize Crankshaft array literal initialization from boilerplate. (Closed)

Created:
9 years ago by danno

Modified:
9 years ago

Reviewers:
Jakob Kummerow

CC:
v8-dev

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

Visibility:
Public.

More Reviews

Description

Optimize Crankshaft array literal initialization from boilerplate. BUG=none TEST=test/mjsunit/array-literal-transitions.js Committed: http://code.google.com/p/v8/source/detail?r=10138

Patch Set 5 : review feedback #

Patch Set 6 : merge with latest #

Created: 9 years ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+193 lines, -74 lines)			Patch
M	src/arm/lithium-codegen-arm.cc	View	1 2 3 4 5	2 chunks	+24 lines, -8 lines	0 comments	Download
M	src/hydrogen.cc	View	1 2 3 4 5	2 chunks	+37 lines, -40 lines	0 comments	Download
M	src/hydrogen-instructions.h	View	1 2 3 4 5	2 chunks	+7 lines, -4 lines	0 comments	Download
M	src/hydrogen-instructions.cc	View		1 chunk	+1 line, -4 lines	0 comments	Download
M	src/ia32/lithium-codegen-ia32.cc	View	1 2 3 4 5	2 chunks	+24 lines, -9 lines	0 comments	Download
M	src/runtime.cc	View		3 chunks	+3 lines, -0 lines	0 comments	Download
M	src/x64/lithium-codegen-x64.cc	View	1 2 3 4 5	2 chunks	+24 lines, -8 lines	0 comments	Download
M	test/cctest/test-debug.cc	View	1 2 3	1 chunk	+1 line, -1 line	0 comments	Download
M	test/mjsunit/array-literal-transitions.js	View	1 2 3 4	1 chunk	+72 lines, -0 lines	0 comments	Download

Messages

Total messages: 3 (0 generated)

Expand Messages | Collapse Messages

Jakob Kummerow

LGTM http://codereview.chromium.org/8747009/diff/4001/src/hydrogen.cc File src/hydrogen.cc (right): http://codereview.chromium.org/8747009/diff/4001/src/hydrogen.cc#newcode3467 src/hydrogen.cc:3467: } else { Since the if-branch ends with ...

9 years ago (2011-11-30 14:04:50 UTC) #2

danno

9 years ago (2011-12-01 14:41:10 UTC) #3

Comments addressed, landing.

http://codereview.chromium.org/8747009/diff/4001/src/hydrogen.cc
File src/hydrogen.cc (right):

http://codereview.chromium.org/8747009/diff/4001/src/hydrogen.cc#newcode3467
src/hydrogen.cc:3467: } else {
On 2011/11/30 14:04:51, Jakob wrote:
> Since the if-branch ends with a return statement, you can skip the explicit
> "else {" to avoid the "arrow" anti-pattern.

Done.

http://codereview.chromium.org/8747009/diff/4001/src/hydrogen.cc#newcode3504
src/hydrogen.cc:3504: AddInstruction(new(zone()) HStoreKeyedFastElement(
On 2011/11/30 14:04:51, Jakob wrote:
> Hm... this will deopt if |value| doesn't match |boilerplate_elements_kind|,
but
> IIUC your changes to runtime.cc should make sure that doesn't happen too
often.
> OK.

Done.

http://codereview.chromium.org/8747009/diff/4001/test/mjsunit/array-literal-t...
File test/mjsunit/array-literal-transitions.js (right):

http://codereview.chromium.org/8747009/diff/4001/test/mjsunit/array-literal-t...
test/mjsunit/array-literal-transitions.js:192: array =
deopt_array_literal_all_doubles(foo);;
On 2011/11/30 14:04:51, Jakob wrote:
> nit: duplicate semicolon

Done.

Expand Messages | Collapse Messages