Reapply: Inline fast cases in string keyed load IC.

Reapply: Inline fast cases in string keyed load IC.

(Fixed handling of out-of-bounds keys.)

String keyed load used to call STRING_CHAR_AT builtin that performs
two steps (get a char code, construct a one-char string from the
code), both of which have fast cases implemented as inline runtime
functions. In this chage most of the code from these functions is
extracted to a set of common generator functions in StringStubBase and
the fast cases are grouped together in the IC code.

Committed: http://code.google.com/p/v8/source/detail?r=4450

[Vitaly Repeshko, 2010-04-20 10:29:51]

Diff from the first attempt:

diff --git a/src/ia32/ic-ia32.cc b/src/ia32/ic-ia32.cc
index f07018a..34b32be 100644
--- a/src/ia32/ic-ia32.cc
+++ b/src/ia32/ic-ia32.cc
@@ -536,11 +536,25 @@ void KeyedLoadIC::GenerateString(MacroAssembler* masm) {
   __ EnterInternalFrame();
   __ push(receiver);
   __ push(index);
-  __ CallRuntime(Runtime::kStringCharCodeAt, 1);
+  __ CallRuntime(Runtime::kStringCharCodeAt, 2);
   ASSERT(!code.is(eax));
   __ mov(code, eax);
   __ LeaveInternalFrame();
-  __ jmp(&got_char_code);
+
+  // Check if the runtime call returned NaN char code. If yes, return
+  // undefined. Otherwise, we can continue.
+  if (FLAG_debug_code) {
+    ASSERT(kSmiTag == 0);
+    __ test(code, Immediate(kSmiTagMask));
+    __ j(zero, &got_char_code);
+    __ mov(scratch, FieldOperand(code, HeapObject::kMapOffset));
+    __ cmp(scratch, Factory::heap_number_map());
+    __ Assert(equal, "StringCharCodeAt must return smi or heap number");
+  }
+  __ cmp(code, Factory::nan_value());
+  __ j(not_equal, &got_char_code);
+  __ Set(eax, Immediate(Factory::undefined_value()));
+  __ ret(0);
 
   __ bind(&miss);
   GenerateMiss(masm);
diff --git a/test/mjsunit/string-index.js b/test/mjsunit/string-index.js
index 3ad71e0..6f11a5b 100644
--- a/test/mjsunit/string-index.js
+++ b/test/mjsunit/string-index.js
@@ -35,6 +35,14 @@ assertEquals("F", foo[0]);
 assertEquals("o", foo[1]);
 assertEquals("o", foo[2]);
 
+// Test string keyed load IC.
+for (var i = 0; i < 10; i++) {
+  assertEquals("F", foo[0]);
+  assertEquals("o", foo[1]);
+  assertEquals("o", foo[2]);
+  assertEquals("F", (foo[0] + "BarBazQuuxFooBarQuux")[0]);
+}
+
 assertEquals("F", foo["0" + ""], "string index");
 assertEquals("o", foo["1"], "string index");
 assertEquals("o", foo["2"], "string index");
@@ -178,9 +186,9 @@ for (var i = 0; i < 200; ++i) {
   assertEquals(expected, actual);
...skipping...
+  // undefined. Otherwise, we can continue.
+  if (FLAG_debug_code) {
+    ASSERT(kSmiTag == 0);
+    __ test(code, Immediate(kSmiTagMask));
+    __ j(zero, &got_char_code);
+    __ mov(scratch, FieldOperand(code, HeapObject::kMapOffset));
+    __ cmp(scratch, Factory::heap_number_map());
+    __ Assert(equal, "StringCharCodeAt must return smi or heap number");
+  }
+  __ cmp(code, Factory::nan_value());
+  __ j(not_equal, &got_char_code);
+  __ Set(eax, Immediate(Factory::undefined_value()));
+  __ ret(0);
 
   __ bind(&miss);
   GenerateMiss(masm);
diff --git a/test/mjsunit/string-index.js b/test/mjsunit/string-index.js
index 3ad71e0..6f11a5b 100644
--- a/test/mjsunit/string-index.js
+++ b/test/mjsunit/string-index.js
@@ -35,6 +35,14 @@ assertEquals("F", foo[0]);
 assertEquals("o", foo[1]);
 assertEquals("o", foo[2]);
 
+// Test string keyed load IC.
+for (var i = 0; i < 10; i++) {
+  assertEquals("F", foo[0]);
+  assertEquals("o", foo[1]);
+  assertEquals("o", foo[2]);
+  assertEquals("F", (foo[0] + "BarBazQuuxFooBarQuux")[0]);
+}
+
 assertEquals("F", foo["0" + ""], "string index");
 assertEquals("o", foo["1"], "string index");
 assertEquals("o", foo["2"], "string index");
@@ -178,9 +186,9 @@ for (var i = 0; i < 200; ++i) {
   assertEquals(expected, actual);
 }
 
-var keys = [0, '1', 2, 3.0];
-var str = 'abcd', arr = ['a', 'b', 'c', 'd'];
-for (var i = 0; i < 200; ++i) {
+var keys = [0, '1', 2, 3.0, -1, 10];
+var str = 'abcd', arr = ['a', 'b', 'c', 'd', undefined, undefined];
+for (var i = 0; i < 300; ++i) {
   var index = Math.floor(i / 50);
   var key = keys[index];
   var expected = arr[index];

[Mads Ager (chromium), 2010-04-20 10:36:50]

LGTM

http://codereview.chromium.org/1582041/diff/2001/3004
File src/ia32/ic-ia32.cc (right):

http://codereview.chromium.org/1582041/diff/2001/3004#newcode546
src/ia32/ic-ia32.cc:546: if (FLAG_debug_code) {
I'm not sure this debug_code code adds much?