Generated code for substring slices in ia32.

src/ia32/code-stubs-ia32.cc

Index: src/ia32/code-stubs-ia32.cc
diff --git a/src/ia32/code-stubs-ia32.cc b/src/ia32/code-stubs-ia32.cc
index d76e4bf1f444a40f4021827d747738cd21bd7d53..813bc1caf30a924419e46118f33f83505744569d 100644
--- a/src/ia32/code-stubs-ia32.cc
+++ b/src/ia32/code-stubs-ia32.cc
@@ -5642,9 +5642,6 @@ void StringHelper::GenerateHashGetHash(MacroAssembler* masm,
 void SubStringStub::Generate(MacroAssembler* masm) {
   Label runtime;
 
-  if (FLAG_string_slices) {
-    __ jmp(&runtime);
-  }
   // Stack frame on entry.
   //  esp[0]: return address
   //  esp[4]: to
@@ -5706,7 +5703,81 @@ void SubStringStub::Generate(MacroAssembler* masm) {
   __ movzx_b(ebx, FieldOperand(ebx, Map::kInstanceTypeOffset));
   __ Set(ecx, Immediate(2));
 
-  __ bind(&result_longer_than_two);
+  if (FLAG_string_slices) {
+    __ SmiTag(edx);  // Make edx a smi again.

[antonm, 2011/08/26 16:10:55]

do you want to fallback here from make_two_character_string?

[Yang, 2011/08/29 12:55:56]

Yes.

[antonm, 2011/08/29 14:51:27]

Brevity is .... :)  I meant you know for sure you won't need a slice for two
char string, so you're doing redundant work.

[Yang, 2011/08/29 15:17:07]

Oups. I actually managed to overlook this simple fact. Thanks.

+    __ bind(&result_longer_than_two);
+
+    // eax: string
+    // ebx: instance type
+    // ecx: sub string length
+    // edx: from index (smi)
+    Label copy_routine, allocate_slice, sliced_string, seq_string;
+    __ cmp(ecx, SlicedString::kMinLength);
+    // Short slice.  Copy instead of slicing.
+    __ j(less, &copy_routine);
+    STATIC_ASSERT(kSeqStringTag == 0);
+    __ test(ebx, Immediate(kStringRepresentationMask));
+    __ j(zero, &seq_string, Label::kNear);
+    STATIC_ASSERT(kIsIndirectStringMask == (kSlicedStringTag & kConsStringTag));
+    STATIC_ASSERT(kIsIndirectStringMask != 0);
+    __ test(ebx, Immediate(kIsIndirectStringMask));
+    // External string.  Jump to runtime.
+    __ j(zero, &runtime);
+
+    Factory* factory = masm->isolate()->factory();
+    const uint32_t kSlicedNotConsMask = kSlicedStringTag & ~kConsStringTag;

[antonm, 2011/08/26 16:10:55]

lift those constants into objects.h?

[antonm, 2011/08/29 14:51:27]

any response?

[Yang, 2011/08/29 15:17:07]

I did move these two lines into objects.h
Maybe you were looking in the wrong diff.

[antonm, 2011/08/29 17:19:26]

Sorry, didn't notice that.  Usually people respond w/ something like Done.

+    ASSERT(IsPowerOf2(kSlicedNotConsMask) && kSlicedNotConsMask != 0);
+    __ test(ebx, Immediate(kSlicedNotConsMask));
+    __ j(not_zero, &sliced_string, Label::kNear);
+    // Cons string.  Check whether it is flat, then fetch first part.
+    __ cmp(FieldOperand(eax, ConsString::kSecondOffset),
+           factory->empty_string());
+    __ j(not_equal, &runtime);
+    __ mov(edi, FieldOperand(eax, ConsString::kFirstOffset));
+    __ jmp(&allocate_slice, Label::kNear);
+
+    __ bind(&sliced_string);
+    // Sliced string.  Fetch parent and correct start index by offset.
+    __ add(edx, FieldOperand(eax, SlicedString::kOffsetOffset));
+    __ mov(edi, FieldOperand(eax, SlicedString::kParentOffset));
+    __ jmp(&allocate_slice, Label::kNear);
+
+    __ bind(&seq_string);
+    // Sequential string.  Just move string to the right register.
+    __ mov(edi, eax);

[antonm, 2011/08/26 16:10:55]

maybe keep the parent string in eax and save this move?

[Yang, 2011/08/29 12:55:56]

eax has to be used as destination when allocation the new sliced string and as
return value.

[antonm, 2011/08/29 14:51:27]

I am not sure, apparently you can allocate into any register you like, at least
w/ some minimal amount of work (at least AllocateInNewSpace behaves this way)

[Yang, 2011/08/29 15:17:07]

Yes but the issue is that the return value of this code stub is fixed to be eax.
Now I could of course load the original string into edi to begin with, but then
I would have to move edi into eax in case the substring is the same as the
original string.
So the issue is that the return value has to be eax, the original string has to
be loaded into eax as well since we need to be able to return the original
string immediately if  necessary.

[antonm, 2011/08/29 17:19:26]

ok

+
+    __ bind(&allocate_slice);
+    // edi: parent string
+    // ebx: instance type of parent string
+    // edx: offset
+    // ecx: length
+    // Allocate new sliced string.  At this point we do not reload the instance
+    // type including the string encoding because we simply rely on the info
+    // provided by the parent string.  It does not matter if the parent's
+    // encoding is wrong because we have to recheck encoding of the slice's
+    // parent anyways due to externalized strings
+    Label two_byte_slice, set_slice_header;
+    STATIC_ASSERT(kAsciiStringTag != 0);
+    __ test(ebx, Immediate(kAsciiStringTag));
+    __ j(zero, &two_byte_slice, Label::kNear);
+    __ AllocateAsciiSlicedString(eax, ebx, no_reg, &runtime);
+    __ jmp(&set_slice_header, Label::kNear);
+    __ bind(&two_byte_slice);
+    __ AllocateSlicedString(eax, ebx, no_reg, &runtime);
+    __ bind(&set_slice_header);
+    __ mov(FieldOperand(eax, SlicedString::kOffsetOffset), edx);
+    __ SmiTag(ecx);

[antonm, 2011/08/26 16:10:55]

apparently there is too much of tagging/untagging, but it should have only
minimal perf impact.

[Yang, 2011/08/29 12:55:56]

The alternative saves this one tagging, but would make may other places ugly
(compare with shifted immediates etc.). The reason is that many macros take
un-smi-tagged register values as parameter.

[antonm, 2011/08/29 14:51:27]

Up to you.

+    __ mov(FieldOperand(eax, SlicedString::kLengthOffset), ecx);
+    __ mov(FieldOperand(eax, SlicedString::kParentOffset), edi);
+    __ mov(FieldOperand(eax, SlicedString::kHashFieldOffset),
+           Immediate(String::kEmptyHashField));
+    __ jmp(&return_eax);
+
+    __ bind(&copy_routine);
+  } else {
+    __ bind(&result_longer_than_two);
+  }
+
   // eax: string
   // ebx: instance type
   // ecx: result string length