Issue 9017024: X64: check all cases with "__ movq(FieldAddress" and converted them to StoreIntoObject if appropr...

Issue 9017024: X64: check all cases with "__ movq(FieldAddress" and converted them to StoreIntoObject if appropr... (Closed)

Created:
9 years ago by srdjan

Modified:
9 years ago

Reviewers:
cshapiro

CC:
reviews_dartlang.org, vm-dev_dartlang.org

Base URL:
http://dart.googlecode.com/svn/branches/bleeding_edge/dart/runtime/

Visibility:
Public.

More Reviews

Description

X64: check all cases with "__ movq(FieldAddress" and converted them to StoreIntoObject if appropriate. Committed: https://code.google.com/p/dart/source/detail?r=2725

Patch Set 2 : '' #

Total comments: 4

Created: 9 years ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+17 lines, -7 lines)			Patch
M	vm/assembler_macros_x64.cc	View		1 chunk	+3 lines, -1 line	0 comments	Download
M	vm/code_generator_x64.cc	View	1	4 chunks	+9 lines, -3 lines	2 comments	Download
M	vm/stub_code_x64.cc	View		1 chunk	+5 lines, -3 lines	2 comments	Download

Messages

Total messages: 3 (0 generated)

Expand Messages | Collapse Messages

cshapiro

lgtm C http://codereview.chromium.org/9017024/diff/4001/vm/code_generator_x64.cc File vm/code_generator_x64.cc (right): http://codereview.chromium.org/9017024/diff/4001/vm/code_generator_x64.cc#newcode305 vm/code_generator_x64.cc:305: // R8 contains an integer value, not ...

9 years ago (2011-12-21 19:45:05 UTC) #2

srdjan

9 years ago (2011-12-21 21:29:12 UTC) #3

http://codereview.chromium.org/9017024/diff/4001/vm/code_generator_x64.cc
File vm/code_generator_x64.cc (right):

http://codereview.chromium.org/9017024/diff/4001/vm/code_generator_x64.cc#new...
vm/code_generator_x64.cc:305: // R8 contains an integer value, not an object.
On 2011/12/21 19:45:06, cshapiro wrote:
> Presumably, "integer value" means raw- or unboxed-integer value and not an
> integer object, right?  The incq might be enough to give that away.

That is correct. The increment operation could also mean that it is handled
incorrectly, and the comment confirms the fact that we are handling an unboxed
value.

http://codereview.chromium.org/9017024/diff/4001/vm/stub_code_x64.cc
File vm/stub_code_x64.cc (right):

http://codereview.chromium.org/9017024/diff/4001/vm/stub_code_x64.cc#newcode622
vm/stub_code_x64.cc:622: __ StoreIntoObject(RAX, FieldAddress(RAX,
Array::length_offset()), R10);
On 2011/12/21 19:45:06, cshapiro wrote:
> This should always be a SMI so a store check would seem redundant.

I am concerned to add here an implicit knwoledge that length is a Smi.

Expand Messages | Collapse Messages