[ic] Inline loads for heapnumber and cached string as ArrayIndex

Bug: chromium:1016738, chromium:1016709, v8:9449
Change-Id: I5b50f21b3e40651e16201e63b4a7010b1bf0c639
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1897890Reviewed-by: Toon Verwaest <verwaest@chromium.org>
Reviewed-by: Georg Neis <neis@chromium.org>
Commit-Queue: Sathya Gunasekaran  <gsathya@chromium.org>
Cr-Commit-Position: refs/heads/master@{#64766}

src/codegen/code-stub-assembler.cc

@@ -9776,20 +9776,50 @@ TNode<IntPtrT> CodeStubAssembler::TryToIntptr(
   Label done(this, &var_intptr_key), key_is_smi(this), key_is_heapnumber(this);
   GotoIf(TaggedIsSmi(key), &key_is_smi);
 
-  if (var_instance_type != nullptr) {
-    *var_instance_type = LoadInstanceType(CAST(key));
-  }
-
-  Node* function = ExternalConstant(
-      ExternalReference::object_to_array_index_slow_function());
-  TNode<Int32T> result = UncheckedCast<Int32T>(
-      CallCFunction(function, MachineType::Int32(),
-                    std::make_pair(MachineType::AnyTagged(), key)));
-  GotoIf(Word32Equal(Int32Constant(-1), result), if_not_intptr);
-  if (if_bailout == nullptr) if_bailout = if_not_intptr;
-  GotoIf(Word32Equal(Int32Constant(-2), result), if_bailout);
-  var_intptr_key = ChangeInt32ToIntPtr(result);
-  Goto(&done);
+  TNode<Map> map = LoadMap(CAST(key));
+  TNode<Int32T> instance_type = LoadMapInstanceType(map);
+  if (var_instance_type != nullptr) *var_instance_type = instance_type;
+  GotoIf(IsHeapNumberInstanceType(instance_type), &key_is_heapnumber);
+  GotoIfNot(IsStringInstanceType(instance_type), if_not_intptr);
+
+  Label if_has_cached_index(this), if_calculate_index(this);
+  TNode<Uint32T> hash = LoadNameHashField(CAST(key));
+
+  Branch(IsClearWord32(hash, Name::kDoesNotContainCachedArrayIndexMask),
+         &if_has_cached_index, &if_calculate_index);
+
+  BIND(&if_has_cached_index);
+  {
+    TNode<IntPtrT> index =
+        Signed(DecodeWordFromWord32<String::ArrayIndexValueBits>(hash));
+    CSA_ASSERT(this, IntPtrLessThan(index, IntPtrConstant(INT_MAX)));
+    var_intptr_key = index;
+    Goto(&done);
+  }
+
+  BIND(&if_calculate_index);
+  {
+    Node* function =
+        ExternalConstant(ExternalReference::string_to_int32_function());
+    TNode<Int32T> result = UncheckedCast<Int32T>(
+        CallCFunction(function, MachineType::Int32(),
+                      std::make_pair(MachineType::AnyTagged(), key)));
+    GotoIf(Word32Equal(Int32Constant(-1), result), if_not_intptr);
+    if (if_bailout == nullptr) if_bailout = if_not_intptr;
+    GotoIf(Word32Equal(Int32Constant(-2), result), if_bailout);
+    var_intptr_key = ChangeInt32ToIntPtr(result);
+    Goto(&done);
+  }
+
+  BIND(&key_is_heapnumber);
+  {
+    TNode<Float64T> value = LoadHeapNumberValue(CAST(key));
+    TNode<Int32T> int_value = RoundFloat64ToInt32(value);
+    GotoIfNot(Float64Equal(value, ChangeInt32ToFloat64(int_value)),
+              if_not_intptr);
+    var_intptr_key = ChangeInt32ToIntPtr(int_value);
+    Goto(&done);
+  }
 
   BIND(&key_is_smi);
   {

src/codegen/external-reference.cc

@@ -639,8 +639,7 @@ FUNCTION_REFERENCE(copy_typed_array_elements_to_typed_array,
 FUNCTION_REFERENCE(copy_typed_array_elements_slice, CopyTypedArrayElementsSlice)
 FUNCTION_REFERENCE(try_internalize_string_function,
                    StringTable::LookupStringIfExists_NoAllocate)
-FUNCTION_REFERENCE(object_to_array_index_slow_function,
-                   Object::ToArrayIndexSlow)
+FUNCTION_REFERENCE(string_to_int32_function, String::ToInt32)
 
 static Address LexicographicCompareWrapper(Isolate* isolate, Address smi_x,
                                            Address smi_y) {

src/codegen/external-reference.h

@@ -159,7 +159,6 @@ class StatsCounter;
   V(mutable_big_int_absolute_sub_and_canonicalize_function,                   \
     "MutableBigInt_AbsoluteSubAndCanonicalize")                               \
   V(new_deoptimizer_function, "Deoptimizer::New()")                           \
-  V(object_to_array_index_slow_function, "Object::ToArrayIndexSlow")          \
   V(orderedhashmap_gethash_raw, "orderedhashmap_gethash_raw")                 \
   V(printf_function, "printf")                                                \
   V(refill_math_random, "MathRandom::RefillCache")                            \
@@ -168,6 +167,7 @@ class StatsCounter;
   V(search_string_raw_two_one, "search_string_raw_two_one")                   \
   V(search_string_raw_two_two, "search_string_raw_two_two")                   \
   V(smi_lexicographic_compare_function, "smi_lexicographic_compare_function") \
+  V(string_to_int32_function, "String::ToInt32")                              \
   V(try_internalize_string_function, "try_internalize_string_function")       \
   V(wasm_call_trap_callback_for_testing,                                      \
     "wasm::call_trap_callback_for_testing")                                   \

src/compiler/effect-control-linearizer.cc

@@ -2598,22 +2598,58 @@ Node* EffectControlLinearizer::LowerCheckedTaggedToArrayIndex(
   // In the Smi case, just convert to int32.
   __ Goto(&done, ChangeSmiToInt32(value));
 
+  // In the non-Smi case, check the heap numberness, load the number and convert
+  // to int32.
   __ Bind(&if_not_smi);
-  MachineSignature::Builder builder(graph()->zone(), 1, 1);
-  builder.AddReturn(MachineType::Int32());
-  builder.AddParam(MachineType::TaggedPointer());
-  Node* object_to_array_index_function = __ ExternalConstant(
-      ExternalReference::object_to_array_index_slow_function());
-  auto call_descriptor =
-      Linkage::GetSimplifiedCDescriptor(graph()->zone(), builder.Build());
-  Node* index = __ Call(common()->Call(call_descriptor),
-                        object_to_array_index_function, value);
-
-  __ DeoptimizeIf(DeoptimizeReason::kNotAnArrayIndex, params.feedback(),
-                  __ Int32LessThan(index, __ Int32Constant(0)), frame_state);
+  auto if_not_heap_number = __ MakeDeferredLabel();
+  Node* value_map = __ LoadField(AccessBuilder::ForMap(), value);
+  Node* is_heap_number = __ TaggedEqual(value_map, __ HeapNumberMapConstant());
+  __ GotoIfNot(is_heap_number, &if_not_heap_number);
+
+  Node* vfalse = __ LoadField(AccessBuilder::ForHeapNumberValue(), value);
+  vfalse =
+      BuildCheckedFloat64ToInt32(CheckForMinusZeroMode::kDontCheckForMinusZero,
+                                 params.feedback(), vfalse, frame_state);
+  __ Goto(&done, vfalse);
+
+  __ Bind(&if_not_heap_number);
+  auto calculate_index = __ MakeDeferredLabel();
+  Node* value_instance_type =
+      __ LoadField(AccessBuilder::ForMapInstanceType(), value_map);
+  Node* is_string = __ Uint32LessThan(value_instance_type,
+                                      __ Uint32Constant(FIRST_NONSTRING_TYPE));
+  __ DeoptimizeIfNot(DeoptimizeReason::kNotAString, params.feedback(),
+                     is_string, frame_state);
+  Node* hash = __ LoadField(AccessBuilder::ForNameHashField(), value);
+  Node* has_cached_index = __ Word32Equal(
+      __ Word32And(hash,
+                   __ Int32Constant(Name::kDoesNotContainCachedArrayIndexMask)),
+      __ Int32Constant(0));
+  __ GotoIfNot(has_cached_index, &calculate_index);
+
+  Node* index = __ Word32Shr(
+      __ Word32And(hash, __ Int32Constant(String::ArrayIndexValueBits::kMask)),
+      __ Int32Constant(String::ArrayIndexValueBits::kShift));
 
   __ Goto(&done, index);
 
+  __ Bind(&calculate_index);
+  {
+    MachineSignature::Builder builder(graph()->zone(), 1, 1);
+    builder.AddReturn(MachineType::Int32());
+    builder.AddParam(MachineType::TaggedPointer());
+    Node* string_to_int32_function =
+        __ ExternalConstant(ExternalReference::string_to_int32_function());
+    auto call_descriptor =
+        Linkage::GetSimplifiedCDescriptor(graph()->zone(), builder.Build());
+    Node* index = __ Call(common()->Call(call_descriptor),
+                          string_to_int32_function, value);
+
+    __ DeoptimizeIf(DeoptimizeReason::kNotAnArrayIndex, params.feedback(),
+                    __ Int32LessThan(index, __ Int32Constant(0)), frame_state);
+
+    __ Goto(&done, index);
+  }
   __ Bind(&done);
   return done.PhiAt(0);
 }

src/objects/objects.cc

@@ -600,37 +600,6 @@ Object Object::ToBoolean(Isolate* isolate) {
   return isolate->heap()->ToBoolean(BooleanValue(isolate));
 }
 
-int32_t Object::ToArrayIndexSlow(Address addr) {
-  DisallowHeapAllocation no_gc;
-  Object key(addr);
-
-  // Smi case should be handled by the fast path.
-  DCHECK(!key.IsSmi());
-
-  uint32_t index;
-  bool success = false;
-  if (key.IsHeapNumber()) {
-    double num = HeapNumber::cast(key).value();
-    success = DoubleToUint32IfEqualToSelf(num, &index);
-  } else if (key.IsString()) {
-    success = String::cast(key).AsArrayIndex(&index);
-  }
-
-  if (!success) return -1;
-  if (index <= INT_MAX) return index;
-
-  // TODO(gsathya): This check exists because we only support upto
-  // INT_MAX for element access in the builtins. We return -2 to
-  // distinguish the case where index <= JSArray::kMaxArrayIndex and
-  // index > INT_MAX so the builtin can handle this appropriately.
-  //
-  // Once we change the builtins to correctly support element access
-  // for indices up to JSArray::kMaxArrayIndex, this check can go
-  // away.
-  if (index <= JSArray::kMaxArrayIndex) return -2;
-  return -1;
-}
-
 namespace {
 
 // TODO(bmeurer): Maybe we should introduce a marker interface Number,

src/objects/objects.h

@@ -315,18 +315,6 @@ class Object : public TaggedImpl<HeapObjectReferenceType::STRONG, Address> {
   V8_EXPORT_PRIVATE bool ToInt32(int32_t* value);
   inline bool ToUint32(uint32_t* value) const;
 
-  // Converts a HeapNumber or String to an int32. Negative numbers are
-  // not supported. This is used for calculating array indices but
-  // differs from an Array Index in the regard that this does not
-  // support the full array index range. This only supports positive
-  // numbers less than INT_MAX.
-  //
-  // if val < 0, returns -1
-  // if 0 <= val <= INT_MAX, returns val
-  // if INT_MAX < val <= JSArray::kMaxArrayIndex, returns -2
-  // if JSArray::kMaxArrayIndex < val, returns -1
-  static int32_t ToArrayIndexSlow(Address addr);
-
   inline Representation OptimalRepresentation(Isolate* isolate) const;
 
   inline ElementsKind OptimalElementsKind(Isolate* isolate) const;

src/objects/string.cc

@@ -903,6 +903,25 @@ ComparisonResult String::Compare(Isolate* isolate, Handle<String> x,
   return result;
 }
 
+int32_t String::ToInt32(Address key_addr) {
+  DisallowHeapAllocation no_gc;
+  String key(key_addr);
+  uint32_t index;
+  if (!key.AsArrayIndex(&index)) return -1;
+  if (index <= INT_MAX) return index;
+
+  // TODO(gsathya): This check exists because we only support upto
+  // INT_MAX for element access in the builtins. We return -2 to
+  // distinguish the case where index <= JSArray::kMaxArrayIndex and
+  // index > INT_MAX so the builtin can handle this appropriately.
+  //
+  // Once we change the builtins to correctly support element access
+  // for indices up to JSArray::kMaxArrayIndex, this check can go
+  // away.
+  if (index <= JSArray::kMaxArrayIndex) return -2;
+  return -1;
+}
+
 Object String::IndexOf(Isolate* isolate, Handle<Object> receiver,
                        Handle<Object> search, Handle<Object> position) {
   if (receiver->IsNullOrUndefined(isolate)) {

src/objects/string.h

@@ -323,6 +323,10 @@ class String : public TorqueGeneratedString<String, Name> {
   // integer in the range of a size_t. Useful for TypedArray accesses.
   inline bool AsIntegerIndex(size_t* index);
 
+  // TODO(gsathya): Change this to ToArrayIndex once CSA can handle
+  // UintPtr for element index access.
+  static int32_t ToInt32(Address key);
+
   // Trimming.
   enum TrimMode { kTrim, kTrimStart, kTrimEnd };
   static Handle<String> Trim(Isolate* isolate, Handle<String> string,