Issue 10546035: Intrinsify some ByteArray methods.

Issue 10546035: Intrinsify some ByteArray methods. (Closed)

Created:
8 years, 6 months ago by cshapiro

Modified:
8 years, 6 months ago

Reviewers:
srdjan

CC:
reviews_dartlang.org, vm-dev_dartlang.org

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

Visibility:
Public.

More Reviews

Description

Intrinsify some ByteArray methods. get:length is intrinsified for internal and external subtypes. [] is intrinsified for 8- and 16-bit internal subtypes on ia32. [] is also intrinsified for 32-bit internal subtypes on x64. Committed: https://code.google.com/p/dart/source/detail?r=8371

Patch Set 1 #

Total comments: 13

Patch Set 2 : address review comments #

Created: 8 years, 6 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+217 lines, -1 line)			Patch
M	runtime/vm/assembler_x64.cc	View	1	1 chunk	+4 lines, -1 line	0 comments	Download
M	runtime/vm/intrinsifier.h	View		1 chunk	+7 lines, -0 lines	0 comments	Download
M	runtime/vm/intrinsifier_ia32.cc	View	1	1 chunk	+93 lines, -0 lines	0 comments	Download
M	runtime/vm/intrinsifier_x64.cc	View	1	1 chunk	+113 lines, -0 lines	0 comments	Download

Messages

Total messages: 3 (0 generated)

Expand Messages | Collapse Messages

srdjan

LGTM with suggestions http://codereview.chromium.org/10546035/diff/1/runtime/vm/assembler_x64.cc File runtime/vm/assembler_x64.cc (right): http://codereview.chromium.org/10546035/diff/1/runtime/vm/assembler_x64.cc#newcode326 runtime/vm/assembler_x64.cc:326: // MOVSXD Weird comment, remove? http://codereview.chromium.org/10546035/diff/1/runtime/vm/intrinsifier_ia32.cc ...

8 years, 6 months ago (2012-06-06 23:30:00 UTC) #2

cshapiro

8 years, 6 months ago (2012-06-06 23:43:07 UTC) #3

http://codereview.chromium.org/10546035/diff/1/runtime/vm/assembler_x64.cc
File runtime/vm/assembler_x64.cc (right):

http://codereview.chromium.org/10546035/diff/1/runtime/vm/assembler_x64.cc#ne...
runtime/vm/assembler_x64.cc:326: // MOVSXD
I will remove it.

The curious thing about this instruction is that the manual calls it MOVSXD and
not MOVSX, probably because it has a completely different opcode from MOVSX.  I
think we may want to rename this instruction for the sake of sanity however,
using the GAS naming convention makes it hard to undo this entirely.

http://codereview.chromium.org/10546035/diff/1/runtime/vm/intrinsifier_ia32.cc
File runtime/vm/intrinsifier_ia32.cc (right):

http://codereview.chromium.org/10546035/diff/1/runtime/vm/intrinsifier_ia32.c...
runtime/vm/intrinsifier_ia32.cc:478: __ SmiUntag(EBX);
Thanks!  Done.

http://codereview.chromium.org/10546035/diff/1/runtime/vm/intrinsifier_ia32.c...
runtime/vm/intrinsifier_ia32.cc:493: __ SmiUntag(EBX);
Done.

http://codereview.chromium.org/10546035/diff/1/runtime/vm/intrinsifier_x64.cc
File runtime/vm/intrinsifier_x64.cc (right):

http://codereview.chromium.org/10546035/diff/1/runtime/vm/intrinsifier_x64.cc...
runtime/vm/intrinsifier_x64.cc:208: __ SmiUntag(RBX);
Done.

http://codereview.chromium.org/10546035/diff/1/runtime/vm/intrinsifier_x64.cc...
runtime/vm/intrinsifier_x64.cc:238: __ SmiUntag(RBX);
Done.

http://codereview.chromium.org/10546035/diff/1/runtime/vm/intrinsifier_x64.cc...
runtime/vm/intrinsifier_x64.cc:253: __ SmiUntag(RBX);
Done.

Expand Messages | Collapse Messages