Native construction of RegExp result objects, with in-object index and input.

Native construction of RegExp result objects, with in-object index and input.
Avoid cloning using CloneRegExpResult for results that are just arrays.
Made a more direct path for string.match with string argument.

[Lasse Reichstein, 2010-04-12 13:21:18]

RegExp result in-object properties for review.

[Søren Thygesen Gjesse, 2010-04-13 07:22:29]

LGTM

http://codereview.chromium.org/1645001/diff/1/2
File src/arm/codegen-arm.cc (right):

http://codereview.chromium.org/1645001/diff/1/2#newcode4038
src/arm/codegen-arm.cc:4038: __ tst(r1, Operand(0x01));
kHeapObjectTag?

http://codereview.chromium.org/1645001/diff/1/2#newcode4049
src/arm/codegen-arm.cc:4049: (JSArray::kRegExpResultSize +
FixedArray::kHeaderSize) / kPointerSize;
JSArray::kRegExpResultSize -> JSRegExpResult::kSize

http://codereview.chromium.org/1645001/diff/1/2#newcode4079
src/arm/codegen-arm.cc:4079: __ str(r4, FieldMemOperand(r0, JSArray::kSize));
Please add constants JSRegExpResult::kIndexOffset and
JSRegExpResult::kInputOffset.

http://codereview.chromium.org/1645001/diff/1/2#newcode4085
src/arm/codegen-arm.cc:4085: // r1: Number of elements in array, as smi.
r1 -> r5 but untagged.

http://codereview.chromium.org/1645001/diff/1/2#newcode4090
src/arm/codegen-arm.cc:4090: // Untag r1 and set FixedArray length.
Remove "Untag r1 and ".

http://codereview.chromium.org/1645001/diff/1/2#newcode4099
src/arm/codegen-arm.cc:4099: // r2: the hole.
Add r5 to this comment.

http://codereview.chromium.org/1645001/diff/1/4
File src/bootstrapper.cc (right):

http://codereview.chromium.org/1645001/diff/1/4#newcode1272
src/bootstrapper.cc:1272: FieldDescriptor index_field(Heap::index_symbol(),
Move the constant to JSRegExpResult (see comment in objects.h).

http://codereview.chromium.org/1645001/diff/1/4#newcode1280
src/bootstrapper.cc:1280: FieldDescriptor input_field(Heap::input_symbol(),
Ditto.

http://codereview.chromium.org/1645001/diff/1/8
File src/ia32/codegen-ia32.cc (right):

http://codereview.chromium.org/1645001/diff/1/8#newcode6552
src/ia32/codegen-ia32.cc:6552: __ test(ebx, Immediate(0x01));
kHeapObjectTag?

http://codereview.chromium.org/1645001/diff/1/8#newcode6562
src/ia32/codegen-ia32.cc:6562: __ AllocateInNewSpace(JSArray::kRegExpResultSize
+ FixedArray::kHeaderSize,
JSArray::kRegExpResultSize -> JSRegExpResult::kSize

http://codereview.chromium.org/1645001/diff/1/8#newcode6586
src/ia32/codegen-ia32.cc:6586: __ pop(FieldOperand(eax, JSArray::kSize +
kPointerSize));
Please add constants JSRegExpResult::kIndexOffset and
JSRegExpResult::kInputOffset.

http://codereview.chromium.org/1645001/diff/1/10
File src/objects.h (right):

http://codereview.chromium.org/1645001/diff/1/10#newcode4626
src/objects.h:4626: // RegExp results are arrays with index and input as
internal fields.
How about creating the class JSRegExpResult inheriting from JSArray which adds
these two constants and Please add constantskIndexOffset and kInputOffset as
well?

http://codereview.chromium.org/1645001/diff/1/11
File src/regexp.js (right):

http://codereview.chromium.org/1645001/diff/1/11#newcode141
src/regexp.js:141: var answer = %_RegExpConstructResult(length, array.index,
array.input);
Will it be possible to pass in the array to have the generated code/runtime copy
the elements as well? This saves from first filling in holes. Maybe it does not
matter.

http://codereview.chromium.org/1645001/diff/1/12
File src/runtime.cc (right):

http://codereview.chromium.org/1645001/diff/1/12#newcode1254
src/runtime.cc:1254: // Write in-object properties after the length of the
array.
Move the constants to JSRegExpResult (see comment in objects.h).

http://codereview.chromium.org/1645001/diff/1/15
File src/x64/codegen-x64.cc (right):

http://codereview.chromium.org/1645001/diff/1/15#newcode4183
src/x64/codegen-x64.cc:4183: __ pop(FieldOperand(rax, JSArray::kSize +
kPointerSize));
Please add constants JSRegExpResult::kIndexOffset and
JSRegExpResult::kInputOffset.

http://codereview.chromium.org/1645001/diff/1/15#newcode4191
src/x64/codegen-x64.cc:4191: // rbx: Number of elements in array.
Maybe keep the same dsctiption for rbx "Number of array elements as int32." as
it is the same value.

http://codereview.chromium.org/1645001/diff/1/15#newcode4203
src/x64/codegen-x64.cc:4203: // rbx: Number of elements to fill (as int32).
Ditto.

[Lasse Reichstein, 2010-04-13 09:50:55]

http://codereview.chromium.org/1645001/diff/1/2
File src/arm/codegen-arm.cc (right):

http://codereview.chromium.org/1645001/diff/1/2#newcode4038
src/arm/codegen-arm.cc:4038: __ tst(r1, Operand(0x01));
That, or kSmiTagMask.
Fixed.

http://codereview.chromium.org/1645001/diff/1/2#newcode4049
src/arm/codegen-arm.cc:4049: (JSArray::kRegExpResultSize +
FixedArray::kHeaderSize) / kPointerSize;
Done.

http://codereview.chromium.org/1645001/diff/1/2#newcode4079
src/arm/codegen-arm.cc:4079: __ str(r4, FieldMemOperand(r0, JSArray::kSize));
Done.

http://codereview.chromium.org/1645001/diff/1/2#newcode4085
src/arm/codegen-arm.cc:4085: // r1: Number of elements in array, as smi.
Whoops, code and comments out of sync. Well spotted.

http://codereview.chromium.org/1645001/diff/1/2#newcode4090
src/arm/codegen-arm.cc:4090: // Untag r1 and set FixedArray length.
Done.

http://codereview.chromium.org/1645001/diff/1/2#newcode4099
src/arm/codegen-arm.cc:4099: // r2: the hole.
Fixed, r1->r5.

http://codereview.chromium.org/1645001/diff/1/4
File src/bootstrapper.cc (right):

http://codereview.chromium.org/1645001/diff/1/4#newcode1272
src/bootstrapper.cc:1272: FieldDescriptor index_field(Heap::index_symbol(),
Done.

http://codereview.chromium.org/1645001/diff/1/4#newcode1280
src/bootstrapper.cc:1280: FieldDescriptor input_field(Heap::input_symbol(),
Done too.

http://codereview.chromium.org/1645001/diff/1/8
File src/ia32/codegen-ia32.cc (right):

http://codereview.chromium.org/1645001/diff/1/8#newcode6552
src/ia32/codegen-ia32.cc:6552: __ test(ebx, Immediate(0x01));
Changed to kSmiTagMask.

http://codereview.chromium.org/1645001/diff/1/8#newcode6563
src/ia32/codegen-ia32.cc:6563: times_half_pointer_size,
Done.

http://codereview.chromium.org/1645001/diff/1/8#newcode6586
src/ia32/codegen-ia32.cc:6586: __ pop(FieldOperand(eax, JSArray::kSize +
kPointerSize));
Done.

http://codereview.chromium.org/1645001/diff/1/10
File src/objects.h (right):

http://codereview.chromium.org/1645001/diff/1/10#newcode4626
src/objects.h:4626: // RegExp results are arrays with index and input as
internal fields.
Done.

http://codereview.chromium.org/1645001/diff/1/11
File src/regexp.js (right):

http://codereview.chromium.org/1645001/diff/1/11#newcode141
src/regexp.js:141: var answer = %_RegExpConstructResult(length, array.index,
array.input);
It's a good idea for an optional argument. The function is also called below,
where we haven't created an array of strings yet.
However, I would want a fast CloneJSArray function too for, e.g., the
String.split cache. That might just be a "copy contents" code that can be reused
here.

http://codereview.chromium.org/1645001/diff/1/12
File src/runtime.cc (right):

http://codereview.chromium.org/1645001/diff/1/12#newcode1254
src/runtime.cc:1254: // Write in-object properties after the length of the
array.
Done.

http://codereview.chromium.org/1645001/diff/1/15
File src/x64/codegen-x64.cc (right):

http://codereview.chromium.org/1645001/diff/1/15#newcode4179
src/x64/codegen-x64.cc:4179: __ lea(rcx, Operand(rax,
JSArray::kRegExpResultSize));
Size moved to JSRegExpResult.

http://codereview.chromium.org/1645001/diff/1/15#newcode4183
src/x64/codegen-x64.cc:4183: __ pop(FieldOperand(rax, JSArray::kSize +
kPointerSize));
Done.

http://codereview.chromium.org/1645001/diff/1/15#newcode4191
src/x64/codegen-x64.cc:4191: // rbx: Number of elements in array.
Done.

http://codereview.chromium.org/1645001/diff/1/15#newcode4203
src/x64/codegen-x64.cc:4203: // rbx: Number of elements to fill (as int32).
Done, slightly reworded (since rbx is modified in the loop).