VM: Optimized code for all of [External]{One|Two}ByteString::codeUnitAt.

VM: Optimized code for all of [External]{One|Two}ByteString::codeUnitAt.

Added support for external string using flow graph based intrinsics
which helps with precompiled code, but also polymorphic calls in jitted code.
I also added support for the missing cases in the flow graph optimizer.

BUG=
R=vegorov@google.com

Committed: https://github.com/dart-lang/sdk/commit/c1e6aef400fd131ee8b260ef5bd90ef2aa9c9de1

[Florian Schneider, 2016-05-10 13:12:59]

fschneider@google.com changed reviewers:
+ vegorov@google.com

[Vyacheslav Egorov (Google), 2016-05-10 15:21:57]

lgtm

https://codereview.chromium.org/1961393002/diff/1/runtime/vm/flow_graph_inlin...
File runtime/vm/flow_graph_inliner.cc (right):

https://codereview.chromium.org/1961393002/diff/1/runtime/vm/flow_graph_inlin...
runtime/vm/flow_graph_inliner.cc:2874: if
(RawObject::IsExternalStringClassId(cid)) {
This would just return false for external strings so the code below does not
actually do much.

https://codereview.chromium.org/1961393002/diff/1/runtime/vm/intrinsifier.cc
File runtime/vm/intrinsifier.cc (right):

https://codereview.chromium.org/1961393002/diff/1/runtime/vm/intrinsifier.cc#...
runtime/vm/intrinsifier.cc:662: static bool BuildCodeUnitAt(FlowGraph*
flow_graph, intptr_t cid) {
We should refactor the code to avoid this duplication between the graph
intrinsifier and optimizer.

[Florian Schneider, 2016-05-10 15:51:52]

https://codereview.chromium.org/1961393002/diff/1/runtime/vm/flow_graph_inlin...
File runtime/vm/flow_graph_inliner.cc (right):

https://codereview.chromium.org/1961393002/diff/1/runtime/vm/flow_graph_inlin...
runtime/vm/flow_graph_inliner.cc:2874: if
(RawObject::IsExternalStringClassId(cid)) {
On 2016/05/10 15:21:57, Vyacheslav Egorov (Google) wrote:
> This would just return false for external strings so the code below does not
> actually do much.

Done.

https://codereview.chromium.org/1961393002/diff/1/runtime/vm/intrinsifier.cc
File runtime/vm/intrinsifier.cc (right):

https://codereview.chromium.org/1961393002/diff/1/runtime/vm/intrinsifier.cc#...
runtime/vm/intrinsifier.cc:662: static bool BuildCodeUnitAt(FlowGraph*
flow_graph, intptr_t cid) {
On 2016/05/10 15:21:57, Vyacheslav Egorov (Google) wrote:
> We should refactor the code to avoid this duplication between the graph
> intrinsifier and optimizer.

Agree. This refactoring would be for a new CL though.

[Florian Schneider, 2016-05-11 07:29:33]

Description was changed from

==========
VM: Optimized code for all of [External]{One|Two}ByteString::codeUnitAt.

Added support for external string using flow graph based intrinsics
which helps with precompiled code, but also polymorphic calls in jitted code.
I also added support for the missing cases in the flow graph optimizer.

BUG=
==========

to

==========
VM: Optimized code for all of [External]{One|Two}ByteString::codeUnitAt.

Added support for external string using flow graph based intrinsics
which helps with precompiled code, but also polymorphic calls in jitted code.
I also added support for the missing cases in the flow graph optimizer.

BUG=
R=vegorov@google.com

Committed:
https://github.com/dart-lang/sdk/commit/c1e6aef400fd131ee8b260ef5bd90ef2aa9c9de1
==========

[Florian Schneider, 2016-05-11 07:29:35]

Committed patchset #2 (id:20001) manually as
c1e6aef400fd131ee8b260ef5bd90ef2aa9c9de1 (presubmit successful).