Issue 12041005: Optimize loads and stores to Int32Array and Uint32Array.

Issue 12041005: Optimize loads and stores to Int32Array and Uint32Array. (Closed)

Created:
7 years, 11 months ago by Florian Schneider

Modified:
7 years, 11 months ago

Reviewers:
Kevin Millikin (Google)

CC:
reviews_dartlang.org

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

Visibility:
Public.

More Reviews

Description

Optimize loads and stores to Int32Array and Uint32Array. Committed: https://code.google.com/p/dart/source/detail?r=17362

Patch Set 1 #

Total comments: 7

Created: 7 years, 11 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Stats (+297 lines, -94 lines)			Patch
M	runtime/vm/flow_graph_compiler.cc	View	2 chunks	+8 lines, -0 lines	2 comments	Download
M	runtime/vm/flow_graph_compiler_ia32.cc	View	1 chunk	+4 lines, -0 lines	0 comments	Download
M	runtime/vm/flow_graph_compiler_x64.cc	View	1 chunk	+4 lines, -0 lines	0 comments	Download
M	runtime/vm/flow_graph_optimizer.cc	View	4 chunks	+35 lines, -7 lines	3 comments	Download
M	runtime/vm/intermediate_language.cc	View	2 chunks	+2 lines, -69 lines	0 comments	Download
M	runtime/vm/intermediate_language_arm.cc	View	2 chunks	+19 lines, -0 lines	0 comments	Download
M	runtime/vm/intermediate_language_ia32.cc	View	5 chunks	+109 lines, -14 lines	2 comments	Download
M	runtime/vm/intermediate_language_mips.cc	View	2 chunks	+19 lines, -0 lines	0 comments	Download
M	runtime/vm/intermediate_language_x64.cc	View	5 chunks	+97 lines, -4 lines	0 comments	Download

Messages

Total messages: 3 (0 generated)

Expand Messages | Collapse Messages

Kevin Millikin (Google)

LGTM. https://codereview.chromium.org/12041005/diff/1/runtime/vm/flow_graph_compiler.cc File runtime/vm/flow_graph_compiler.cc (right): https://codereview.chromium.org/12041005/diff/1/runtime/vm/flow_graph_compiler.cc#newcode893 runtime/vm/flow_graph_compiler.cc:893: case kInt32ArrayCid: There are several places with boilerplate. ...

7 years, 11 months ago (2013-01-21 15:16:01 UTC) #2

Florian Schneider

7 years, 11 months ago (2013-01-21 16:03:25 UTC) #3

https://codereview.chromium.org/12041005/diff/1/runtime/vm/flow_graph_compile...
File runtime/vm/flow_graph_compiler.cc (right):

https://codereview.chromium.org/12041005/diff/1/runtime/vm/flow_graph_compile...
runtime/vm/flow_graph_compiler.cc:893: case kInt32ArrayCid:
On 2013/01/21 15:16:01, kmillikin wrote:
> There are several places with boilerplate.  Keep in mind a way to generate
them
> with macros or templates.
> 
> Another alternative is to have a separate enum for the Array subclasses, a
> function to map cid to array class ID, and switch over that everywhere ---
that
> would give a compiler warning for nonexhaustiveness.

I'll keep that in mind for my next change where I want to add all loads/stores
with External* array variants. It would become even more boilerplate.

https://codereview.chromium.org/12041005/diff/1/runtime/vm/flow_graph_optimiz...
File runtime/vm/flow_graph_optimizer.cc (right):

https://codereview.chromium.org/12041005/diff/1/runtime/vm/flow_graph_optimiz...
runtime/vm/flow_graph_optimizer.cc:608: // mints (ia32 with at least SSE 4.1)
On 2013/01/21 15:16:01, kmillikin wrote:
> The lack of period (and the unbalanced parens

Done.

https://codereview.chromium.org/12041005/diff/1/runtime/vm/intermediate_langu...
File runtime/vm/intermediate_language_ia32.cc (right):

https://codereview.chromium.org/12041005/diff/1/runtime/vm/intermediate_langu...
runtime/vm/intermediate_language_ia32.cc:1201: if (representation() ==
kUnboxedDouble ||
On 2013/01/21 15:16:01, kmillikin wrote:
> I guess you need more parens.

Done.

Expand Messages | Collapse Messages