Issue 6312193: Fixed a number of issues on x64 crankshaft port:

Issue 6312193: Fixed a number of issues on x64 crankshaft port: (Closed)

Created:
9 years, 10 months ago by Mads Ager (chromium)

Modified:
9 years, 7 months ago

Reviewers:
Lasse Reichstein, Rico

CC:
v8-dev

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

Visibility:
Public.

Description

Fixed a number of issues on x64 crankshaft port: - Don't use SmiSub when overflow can occur. It asserts that overflow does not happen. - Actually use CompareICs and signal to crankshaft whether or not smi code was inlined. - Fix bug in CmpI where 64 bits were compared instead of 32 bits. - Implement Throw, DeferredStackCheck, StoreKeyedFastElement in lithium backend. BUG= TEST= Committed: http://code.google.com/p/v8/source/detail?r=6669

Patch Set 1 #

Total comments: 4

Patch Set 2 : Address comments. #

Created: 9 years, 10 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+185 lines, -43 lines)			Patch
M	src/ia32/full-codegen-ia32.cc	View		1 chunk	+1 line, -2 lines	0 comments	Download
M	src/ia32/lithium-codegen-ia32.cc	View	1	2 chunks	+13 lines, -3 lines	0 comments	Download
M	src/messages.js	View		1 chunk	+1 line, -0 lines	0 comments	Download
M	src/x64/assembler-x64.h	View		1 chunk	+2 lines, -0 lines	0 comments	Download
M	src/x64/code-stubs-x64.cc	View		1 chunk	+2 lines, -2 lines	0 comments	Download
M	src/x64/full-codegen-x64.cc	View		8 chunks	+96 lines, -26 lines	0 comments	Download
M	src/x64/ic-x64.cc	View		1 chunk	+13 lines, -1 line	0 comments	Download
M	src/x64/lithium-codegen-x64.cc	View		4 chunks	+41 lines, -5 lines	0 comments	Download
M	src/x64/lithium-x64.cc	View		2 chunks	+16 lines, -4 lines	0 comments	Download

Messages

Total messages: 3 (0 generated)

Expand Messages | Collapse Messages

Rico

LGTM http://codereview.chromium.org/6312193/diff/1/src/x64/lithium-codegen-x64.cc File src/x64/lithium-codegen-x64.cc (right): http://codereview.chromium.org/6312193/diff/1/src/x64/lithium-codegen-x64.cc#newcode1896 src/x64/lithium-codegen-x64.cc:1896: times_pointer_size, I think we should change this to ...

9 years, 10 months ago (2011-02-08 07:33:42 UTC) #2

Mads Ager (chromium)

9 years, 10 months ago (2011-02-08 07:43:00 UTC) #3

http://codereview.chromium.org/6312193/diff/1/src/x64/lithium-codegen-x64.cc
File src/x64/lithium-codegen-x64.cc (right):

http://codereview.chromium.org/6312193/diff/1/src/x64/lithium-codegen-x64.cc#...
src/x64/lithium-codegen-x64.cc:1896: times_pointer_size,
On 2011/02/08 07:33:42, Rico wrote:
> I think we should change this to times_pointer_size on ia32 as well

Done.

http://codereview.chromium.org/6312193/diff/1/src/x64/lithium-codegen-x64.cc#...
src/x64/lithium-codegen-x64.cc:1905: times_pointer_size,
On 2011/02/08 07:33:42, Rico wrote:
> Same here - we should change this to times_pointer_size on ia32 as well to be
> consistent (actually, we should also change it in DoLoadKeyedFastElement on
> IA32)

Done.

Expand Messages | Collapse Messages