Serendipitously arrange the tags so that String.length() becomes a branch-fre...

src/codegen-ia32.cc

Index: src/codegen-ia32.cc
===================================================================
--- src/codegen-ia32.cc	(revision 595)
+++ src/codegen-ia32.cc	(working copy)
@@ -2849,38 +2849,23 @@
   __ sar(ebx, kSmiTagSize);
 
   __ bind(&try_again_with_new_string);
-  // Get the type of the heap object into ecx.
+  // Get the type of the heap object into edi.
   __ mov(edx, FieldOperand(eax, HeapObject::kMapOffset));
-  __ movzx_b(ecx, FieldOperand(edx, Map::kInstanceTypeOffset));
+  __ movzx_b(edi, FieldOperand(edx, Map::kInstanceTypeOffset));
   // We don't handle non-strings.
-  __ test(ecx, Immediate(kIsNotStringMask));
+  __ test(edi, Immediate(kIsNotStringMask));
   __ j(not_zero, &slow_case, not_taken);
 
+  // The code assumes the tags are serendpitous.

[Kasper Lund, 2008/10/27 08:27:31]

Please choose a better word than serendpitous. Preferably one you can spell :-)

+  ASSERT(kLongStringTag == 0);
+  ASSERT(kMediumStringTag + String::kLongLengthShift == String::kMediumLengthShift);

[Kasper Lund, 2008/10/27 08:27:31]

Is this line too long?

+  ASSERT(kShortStringTag + String::kLongLengthShift == String::kShortLengthShift);

[Kasper Lund, 2008/10/27 08:27:31]

Ditto?

+  __ mov(ecx, Operand(edi));
+  __ and_(ecx, kStringSizeMask);
+  __ add(Operand(ecx), Immediate(String::kLongLengthShift));
   // Get the length field.
   __ mov(edx, FieldOperand(eax, String::kLengthOffset));
-  Label long_string;
-  Label medium_string;
-  Label string_length_shifted;
-  // The code assumes the tags are disjoint.
-  ASSERT((kLongStringTag & kMediumStringTag) == 0);
-  ASSERT(kShortStringTag == 0);
-  __ test(ecx, Immediate(kLongStringTag));
-  __ j(not_zero, &long_string, not_taken);
-  __ test(ecx, Immediate(kMediumStringTag));
-  __ j(not_zero, &medium_string, taken);
-  // Short string.
-  __ shr(edx, String::kShortLengthShift);
-  __ jmp(&string_length_shifted);
-
-  // Medium string.
-  __ bind(&medium_string);
-  __ shr(edx, String::kMediumLengthShift - String::kLongLengthShift);
-  // Fall through to long string.
-  __ bind(&long_string);
-  __ shr(edx, String::kLongLengthShift);
-
-  __ bind(&string_length_shifted);
-  ASSERT(kSmiTag == 0);
+  __ shr(edx);  // Ecx is implicit operand.

[Mads Ager (chromium), 2008/10/27 09:37:00]

Since Ecx is a register name, I probably would not capitalize it.

   // edx is now the length of the string.
 
   // Check for index out of range.
@@ -2889,11 +2874,11 @@
 
   // We need special handling for non-flat strings.
   ASSERT(kSeqStringTag == 0);
-  __ test(ecx, Immediate(kStringRepresentationMask));
+  __ test(edi, Immediate(kStringRepresentationMask));
   __ j(not_zero, &not_a_flat_string, not_taken);
 
   // Check for 1-byte or 2-byte string.
-  __ test(ecx, Immediate(kStringEncodingMask));
+  __ test(edi, Immediate(kStringEncodingMask));
   __ j(not_zero, &ascii_string, taken);
 
   // 2-byte string.
@@ -2913,11 +2898,10 @@
   frame_->Push(eax);
   __ jmp(&end);
 
-
   // Handle non-flat strings.
   __ bind(&not_a_flat_string);
-  __ and_(ecx, kStringRepresentationMask);
-  __ cmp(ecx, kConsStringTag);
+  __ and_(edi, kStringRepresentationMask);
+  __ cmp(edi, kConsStringTag);
   __ j(not_equal, &not_a_cons_string_either, not_taken);
 
   // ConsString.
@@ -2926,7 +2910,7 @@
   __ jmp(&try_again_with_new_string);
 
   __ bind(&not_a_cons_string_either);
-  __ cmp(ecx, kSlicedStringTag);
+  __ cmp(edi, kSlicedStringTag);
   __ j(not_equal, &slow_case, not_taken);
 
   // SlicedString.