Inline getters of byte array view in the optimized flow graph.

Inline getters of byte array view in the optimized flow graph.

This CL provides inline IL code for the getters _getInt8, _getInt16, etc.
to speed up [] and byte array views.

The code uses the existing LoadIndexed instructions by passing a index
scale factor explicitly: For normal arrays loads, the scale factor is equal
to the element size. For byte array access, the scale factor is always 1.

I'm adding inlined setters in a separate CL.
Committed: https://code.google.com/p/dart/source/detail?r=18173

[Florian Schneider, 2013-02-06 14:09:10]

On 2013/02/06 14:07:42, Florian Schneider wrote:

The test I added is a modified version of the existing byte_array_test.dart
which hits all cases I added (the existing test results in polymorphic calls, so
getters can't be inlined)

[Vyacheslav Egorov (Google), 2013-02-06 14:49:47]

lgtm

[Vyacheslav Egorov (Google), 2013-02-06 15:16:52]

still lgtm

[srdjan, 2013-02-06 17:11:16]

DBC

https://codereview.chromium.org/12218008/diff/4004/runtime/vm/flow_graph_comp...
File runtime/vm/flow_graph_compiler_ia32.cc (right):

https://codereview.chromium.org/12218008/diff/4004/runtime/vm/flow_graph_comp...
runtime/vm/flow_graph_compiler_ia32.cc:1517: static ScaleFactor
ToScaleFactor(intptr_t index_scale) {
I understand this function but find it confusing (integer index scale is
actually index_scale * 2, except when it is not (index_scale == 1)).

https://codereview.chromium.org/12218008/diff/4004/runtime/vm/flow_graph_opti...
File runtime/vm/flow_graph_optimizer.cc (right):

https://codereview.chromium.org/12218008/diff/4004/runtime/vm/flow_graph_opti...
runtime/vm/flow_graph_optimizer.cc:1370: case kFloat64ArrayCid:
They may be ordered, i.e., you could check for ranges, maybe...

https://codereview.chromium.org/12218008/diff/4004/runtime/vm/flow_graph_opti...
runtime/vm/flow_graph_optimizer.cc:1469: if
(IsSupportedByteArrayCid(class_ids[0]) && ic_data.NumberOfChecks() == 1) {
Add Parenthesis

https://codereview.chromium.org/12218008/diff/4004/runtime/vm/intermediate_la...
File runtime/vm/intermediate_language_ia32.cc (right):

https://codereview.chromium.org/12218008/diff/4004/runtime/vm/intermediate_la...
runtime/vm/intermediate_language_ia32.cc:1212: if (index_scale() == 1 &&
index.IsRegister()) {
Use parenthesis, here and below.

[Florian Schneider, 2013-02-14 12:20:51]

Thanks. Addressed DBCs in a following CL.

https://codereview.chromium.org/12218008/diff/4004/runtime/vm/flow_graph_comp...
File runtime/vm/flow_graph_compiler_ia32.cc (right):

https://codereview.chromium.org/12218008/diff/4004/runtime/vm/flow_graph_comp...
runtime/vm/flow_graph_compiler_ia32.cc:1517: static ScaleFactor
ToScaleFactor(intptr_t index_scale) {
On 2013/02/06 17:11:16, srdjan wrote:
> I understand this function but find it confusing (integer index scale is
> actually index_scale * 2, except when it is not (index_scale == 1)). 

It is indeed a little subtle. The code before had the same underlying issue
though( that element size 1 is different)

Any suggestions to improve it?

https://codereview.chromium.org/12218008/diff/4004/runtime/vm/flow_graph_opti...
File runtime/vm/flow_graph_optimizer.cc (right):

https://codereview.chromium.org/12218008/diff/4004/runtime/vm/flow_graph_opti...
runtime/vm/flow_graph_optimizer.cc:1370: case kFloat64ArrayCid:
On 2013/02/06 17:11:16, srdjan wrote:
> They may be ordered, i.e., you could check for ranges, maybe...

How can I make sure that they are ordered? It would be nice to have
kFirstByteArrayCid and kLastByteArrayCid for a concise cid check.

Right now, I'm hoping that gcc figures out an efficient check if they're are
indeed ordered.

https://codereview.chromium.org/12218008/diff/4004/runtime/vm/flow_graph_opti...
runtime/vm/flow_graph_optimizer.cc:1469: if
(IsSupportedByteArrayCid(class_ids[0]) && ic_data.NumberOfChecks() == 1) {
On 2013/02/06 17:11:16, srdjan wrote:
> Add Parenthesis

Done.

https://codereview.chromium.org/12218008/diff/4004/runtime/vm/intermediate_la...
File runtime/vm/intermediate_language_ia32.cc (right):

https://codereview.chromium.org/12218008/diff/4004/runtime/vm/intermediate_la...
runtime/vm/intermediate_language_ia32.cc:1212: if (index_scale() == 1 &&
index.IsRegister()) {
On 2013/02/06 17:11:16, srdjan wrote:
> Use parenthesis, here and below.

Done.

[srdjan, 2013-02-14 16:36:35]

https://codereview.chromium.org/12218008/diff/4004/runtime/vm/flow_graph_comp...
File runtime/vm/flow_graph_compiler_ia32.cc (right):

https://codereview.chromium.org/12218008/diff/4004/runtime/vm/flow_graph_comp...
runtime/vm/flow_graph_compiler_ia32.cc:1517: static ScaleFactor
ToScaleFactor(intptr_t index_scale) {
On 2013/02/14 12:20:51, Florian Schneider wrote:
> On 2013/02/06 17:11:16, srdjan wrote:
> > I understand this function but find it confusing (integer index scale is
> > actually index_scale * 2, except when it is not (index_scale == 1)). 
> 
> It is indeed a little subtle. The code before had the same underlying issue
> though( that element size 1 is different)
> 
> Any suggestions to improve it?

No :-(.

https://codereview.chromium.org/12218008/diff/4004/runtime/vm/flow_graph_opti...
File runtime/vm/flow_graph_optimizer.cc (right):

https://codereview.chromium.org/12218008/diff/4004/runtime/vm/flow_graph_opti...
runtime/vm/flow_graph_optimizer.cc:1370: case kFloat64ArrayCid:
On 2013/02/14 12:20:51, Florian Schneider wrote:
> On 2013/02/06 17:11:16, srdjan wrote:
> > They may be ordered, i.e., you could check for ranges, maybe...
> 
> How can I make sure that they are ordered? It would be nice to have
> kFirstByteArrayCid and kLastByteArrayCid for a concise cid check.
> 
> Right now, I'm hoping that gcc figures out an efficient check if they're are
> indeed ordered.


They are ordered but not as we need it, see RawObject::IsByteArrayClassId.