Revert r4782. Fix issues 728, 732.

Review URL: http://codereview.chromium.org/2701003

git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@4806 ce2b1a6d-e550-0410-aec6-3dcde31c8c00

src/arm/ic-arm.cc

@@ -849,8 +849,8 @@ void KeyedLoadIC::GenerateGeneric(MacroAssembler* masm) {
 
   // Is the string an array index, with cached numeric value?
   __ ldr(r3, FieldMemOperand(r0, String::kHashFieldOffset));
-  __ tst(r3, Operand(String::kIsArrayIndexMask));
-  __ b(ne, &index_string);
+  __ tst(r3, Operand(String::kContainsCachedArrayIndexMask));
+  __ b(eq, &index_string);
 
   // Is the string a symbol?
   // r2: key map

src/ia32/ic-ia32.cc

@@ -404,8 +404,8 @@ void KeyedLoadIC::GenerateGeneric(MacroAssembler* masm) {
   __ j(above_equal, &slow);
   // Is the string an array index, with cached numeric value?
   __ mov(ebx, FieldOperand(eax, String::kHashFieldOffset));
-  __ test(ebx, Immediate(String::kIsArrayIndexMask));
-  __ j(not_zero, &index_string, not_taken);
+  __ test(ebx, Immediate(String::kContainsCachedArrayIndexMask));
+  __ j(zero, &index_string, not_taken);
 
   // Is the string a symbol?
   // ecx: key map.

src/objects-inl.h

@@ -2987,8 +2987,7 @@ StringHasher::StringHasher(int length)
   : length_(length),
     raw_running_hash_(0),
     array_index_(0),
-    is_array_index_(0 < length_ &&
-                    length_ <= String::kMaxCachedArrayIndexLength),
+    is_array_index_(0 < length_ && length_ <= String::kMaxArrayIndexSize),
     is_first_char_(true),
     is_valid_(true) { }
 
@@ -3051,7 +3050,9 @@ uint32_t StringHasher::GetHash() {
 
 bool String::AsArrayIndex(uint32_t* index) {
   uint32_t field = hash_field();
-  if (IsHashFieldComputed(field) && !(field & kIsArrayIndexMask)) return false;
+  if (IsHashFieldComputed(field) && (field & kIsNotArrayIndexMask)) {
+    return false;
+  }
   return SlowAsArrayIndex(index);
 }
 

src/objects.cc

@@ -4850,7 +4850,7 @@ bool String::SlowAsArrayIndex(uint32_t* index) {
   if (length() <= kMaxCachedArrayIndexLength) {
     Hash();  // force computation of hash code
     uint32_t field = hash_field();
-    if ((field & kIsArrayIndexMask) == 0) return false;
+    if ((field & kIsNotArrayIndexMask) != 0) return false;
     // Isolate the array index form the full hash field.
     *index = (kArrayIndexHashMask & field) >> kHashShift;
     return true;
@@ -4869,10 +4869,14 @@ static inline uint32_t HashField(uint32_t hash,
     // For array indexes mix the length into the hash as an array index could
     // be zero.
     ASSERT(length > 0);
+    ASSERT(length <= String::kMaxArrayIndexSize);
     ASSERT(TenToThe(String::kMaxCachedArrayIndexLength) <
            (1 << String::kArrayIndexValueBits));
-    result |= String::kIsArrayIndexMask;
+    ASSERT(String::kMaxArrayIndexSize < (1 << String::kArrayIndexValueBits));
+    result &= ~String::kIsNotArrayIndexMask;
     result |= length << String::kArrayIndexHashLengthShift;
+  } else {
+    result |= String::kIsNotArrayIndexMask;
   }
   return result;
 }

src/objects.h

@@ -4194,11 +4194,11 @@ class String: public HeapObject {
   // computed the 2nd bit tells whether the string can be used as an
   // array index.
   static const int kHashNotComputedMask = 1;
-  static const int kIsArrayIndexMask = 1 << 1;
-  static const int kNofLengthBitFields = 2;
+  static const int kIsNotArrayIndexMask = 1 << 1;
+  static const int kNofHashBitFields = 2;
 
   // Shift constant retrieving hash code from hash field.
-  static const int kHashShift = kNofLengthBitFields;
+  static const int kHashShift = kNofHashBitFields;
 
   // Array index strings this short can keep their index in the hash
   // field.
@@ -4207,18 +4207,35 @@ class String: public HeapObject {
   // For strings which are array indexes the hash value has the string length
   // mixed into the hash, mainly to avoid a hash value of zero which would be
   // the case for the string '0'. 24 bits are used for the array index value.
-  static const int kArrayIndexHashLengthShift = 24 + kNofLengthBitFields;
+  static const int kArrayIndexValueBits = 24;
+  static const int kArrayIndexLengthBits =
+      kBitsPerInt - kArrayIndexValueBits - kNofHashBitFields;
+
+  STATIC_CHECK((kArrayIndexLengthBits > 0));
+
+  static const int kArrayIndexHashLengthShift =
+      kArrayIndexValueBits + kNofHashBitFields;
+
   static const int kArrayIndexHashMask = (1 << kArrayIndexHashLengthShift) - 1;
-  static const int kArrayIndexValueBits =
-      kArrayIndexHashLengthShift - kHashShift;
+
   static const int kArrayIndexValueMask =
       ((1 << kArrayIndexValueBits) - 1) << kHashShift;
 
+  // Check that kMaxCachedArrayIndexLength + 1 is a power of two so we
+  // could use a mask to test if the length of string is less than or equal to
+  // kMaxCachedArrayIndexLength.
+  STATIC_CHECK(IS_POWER_OF_TWO(kMaxCachedArrayIndexLength + 1));
+
+  static const int kContainsCachedArrayIndexMask =
+      (~kMaxCachedArrayIndexLength << kArrayIndexHashLengthShift) |
+      kIsNotArrayIndexMask;
+
   // Value of empty hash field indicating that the hash is not computed.
-  static const int kEmptyHashField = kHashNotComputedMask;
+  static const int kEmptyHashField =
+      kIsNotArrayIndexMask | kHashNotComputedMask;
 
   // Value of hash field containing computed hash equal to zero.
-  static const int kZeroHash = 0;
+  static const int kZeroHash = kIsNotArrayIndexMask;
 
   // Maximal string length.
   static const int kMaxLength = (1 << (32 - 2)) - 1;

src/utils.h

@@ -37,11 +37,13 @@ namespace internal {
 // ----------------------------------------------------------------------------
 // General helper functions
 
+#define IS_POWER_OF_TWO(x) (((x) & ((x) - 1)) == 0)
+
 // Returns true iff x is a power of 2 (or zero). Cannot be used with the
 // maximally negative value of the type T (the -1 overflows).
 template <typename T>
 static inline bool IsPowerOf2(T x) {
-  return (x & (x - 1)) == 0;
+  return IS_POWER_OF_TWO(x);
 }
 
 

src/x64/ic-x64.cc

@@ -442,8 +442,8 @@ void KeyedLoadIC::GenerateGeneric(MacroAssembler* masm) {
   __ j(above_equal, &slow);
   // Is the string an array index, with cached numeric value?
   __ movl(rbx, FieldOperand(rax, String::kHashFieldOffset));
-  __ testl(rbx, Immediate(String::kIsArrayIndexMask));
-  __ j(not_zero, &index_string);  // The value in rbx is used at jump target.
+  __ testl(rbx, Immediate(String::kContainsCachedArrayIndexMask));
+  __ j(zero, &index_string);  // The value in rbx is used at jump target.
 
   // Is the string a symbol?
   ASSERT(kSymbolTag != 0);