Inline native String.charCodeAt in optimized code.

Inline native String.charCodeAt in optimized code and fix a bounds-check bug.

Introduce a new IL instruction that does charCodeAt from
1- and 2-byte character strings. The existing CheckArrayBound
instructions is extended to be used for string bounds checking.

Also fix a bug with a missing compile-time bounds-check on constant
arrays.

TEST=tests/language/optimized_string_charcodeat.dart,
tests/language/optimized_constant_array_string_access.dart
Committed: https://code.google.com/p/dart/source/detail?r=14441

[srdjan, 2012-11-01 20:32:18]

LGTM with comments

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

https://codereview.chromium.org/11360033/diff/6003/runtime/vm/flow_graph_opti...
runtime/vm/flow_graph_optimizer.cc:483: }
I get a crash with following code. Can you please check if it works and add it
to your tests:

main() {
  for (int i = 0; i < 10000; i++) { foo(1); }
  foo(2);
}

foo(a) {
  if (a == 1) { return 2; }
  var aa = const [1, 2];
  return aa[2.3];
}

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

https://codereview.chromium.org/11360033/diff/6003/runtime/vm/intermediate_la...
runtime/vm/intermediate_language_ia32.cc:1110: __ SmiUntag(index);
Is it better to tag/untag or to require a temporary?

https://codereview.chromium.org/11360033/diff/6003/tests/language/optimized_c...
File tests/language/optimized_constant_array_string_access.dart (right):

https://codereview.chromium.org/11360033/diff/6003/tests/language/optimized_c...
tests/language/optimized_constant_array_string_access.dart:15: } catch(e) { }
You may want to use Expect.throws instead.

https://codereview.chromium.org/11360033/diff/6003/tests/language/optimized_c...
tests/language/optimized_constant_array_string_access.dart:18: return
test(true);
ditto and below

[Florian Schneider, 2012-11-01 22:58:55]

Landed. Thanks for the review.

http://codereview.chromium.org/11360033/diff/6003/runtime/vm/flow_graph_optim...
File runtime/vm/flow_graph_optimizer.cc (right):

http://codereview.chromium.org/11360033/diff/6003/runtime/vm/flow_graph_optim...
runtime/vm/flow_graph_optimizer.cc:483: }
On 2012/11/01 20:32:18, srdjan wrote:
> I get a crash with following code. Can you please check if it works and add it
> to your tests:
> 
> main() {
>   for (int i = 0; i < 10000; i++) { foo(1); }
>   foo(2);
> }
> 
> foo(a) {
>   if (a == 1) { return 2; }
>   var aa = const [1, 2];
>   return aa[2.3];
> }

Good point. I added the test and fixed the non-smi index case for strings and
arrays.

http://codereview.chromium.org/11360033/diff/6003/runtime/vm/intermediate_lan...
File runtime/vm/intermediate_language_ia32.cc (right):

http://codereview.chromium.org/11360033/diff/6003/runtime/vm/intermediate_lan...
runtime/vm/intermediate_language_ia32.cc:1110: __ SmiUntag(index);
On 2012/11/01 20:32:18, srdjan wrote:
> Is it better to tag/untag or to require a temporary? 

With a quick test, I found no noticable difference. On ia32 I always try to
minimize the number of temps. In many cases a temp would require an additional
mov-instruction which is why the difference will probably small if at all.

http://codereview.chromium.org/11360033/diff/6003/tests/language/optimized_co...
File tests/language/optimized_constant_array_string_access.dart (right):

http://codereview.chromium.org/11360033/diff/6003/tests/language/optimized_co...
tests/language/optimized_constant_array_string_access.dart:15: } catch(e) { }
On 2012/11/01 20:32:18, srdjan wrote:
> You may want to use Expect.throws instead.

Done.

http://codereview.chromium.org/11360033/diff/6003/tests/language/optimized_co...
tests/language/optimized_constant_array_string_access.dart:18: return
test(true);
On 2012/11/01 20:32:18, srdjan wrote:
> ditto and below

Done.