Remove SMI length check from TF_BUILTIN(ArrayIncludes, CodeStubAssembler)

src/builtins/builtins-array.cc

 // Copyright 2016 the V8 project authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "src/builtins/builtins.h"
 #include "src/builtins/builtins-utils.h"
 
 #include "src/code-factory.h"
 #include "src/code-stub-assembler.h"
 #include "src/contexts.h"
@@ skipping 1667 matching lines @@
   assembler.Return(
       assembler.CallRuntime(Runtime::kArrayIsArray, context, object));
 }
 
 TF_BUILTIN(ArrayIncludes, CodeStubAssembler) {
   Node* const array = Parameter(0);
   Node* const search_element = Parameter(1);
   Node* const start_from = Parameter(2);
   Node* const context = Parameter(3 + 2);
 
-  Variable len_var(this, MachineType::PointerRepresentation()),
-      index_var(this, MachineType::PointerRepresentation());
+  Variable index_var(this, MachineType::PointerRepresentation());
 
   Label init_k(this), return_true(this), return_false(this), call_runtime(this);
   Label init_len(this), select_loop(this);
 
   index_var.Bind(IntPtrConstant(0));
-  len_var.Bind(IntPtrConstant(0));
 
   // Take slow path if not a JSArray, if retrieving elements requires
   // traversing prototype, or if access checks are required.
   BranchIfFastJSArray(array, context,
                       CodeStubAssembler::FastJSArrayAccessMode::INBOUNDS_READ,
                       &init_len, &call_runtime);
 
+  Node* len = nullptr;

[vabr (Chromium), 2017/02/25 21:25:55]

I tried to initialize |len| with LoadAndUntagObjectField directly, after line
1713 (see patch set 4), but RawMachineAssembler::AddNode complained that its
|current_block_| is null. I'm not sure how that notion of a block corresponds to
the structure of the code here, but assigning the result of
LoadAndUntagObjectField inside the code block on lines 1703-1713 helped.

[Benedikt Meurer, 2017/02/26 18:34:27]

Just remove the { and } in 1704, and declare the Node* Len with the assignment
below.

[vabr (Chromium), 2017/02/26 20:30:24]

Done.

   Bind(&init_len);
   {
-    // Handle case where JSArray length is not an Smi in the runtime
-    Node* len = LoadObjectField(array, JSArray::kLengthOffset);
-    GotoIfNot(TaggedIsSmi(len), &call_runtime);
-
-    len_var.Bind(SmiToWord(len));
+    // JSArray length is always an Smi for fast arrays.
+    CSA_ASSERT(this,
+               TaggedIsSmi(LoadObjectField(array, JSArray::kLengthOffset)));
+    len = LoadAndUntagObjectField(array, JSArray::kLengthOffset);
 
     GotoIf(IsUndefined(start_from), &select_loop);
 
     // Bailout to slow path if startIndex is not an Smi.
     Branch(TaggedIsSmi(start_from), &init_k, &call_runtime);
   }
 
   Bind(&init_k);
   CSA_ASSERT(this, TaggedIsSmi(start_from));
   Node* const untagged_start_from = SmiToWord(start_from);
   index_var.Bind(Select(
       IntPtrGreaterThanOrEqual(untagged_start_from, IntPtrConstant(0)),
       [=]() { return untagged_start_from; },
       [=]() {
-        Node* const index = IntPtrAdd(len_var.value(), untagged_start_from);
+        Node* const index = IntPtrAdd(len, untagged_start_from);
         return SelectConstant(IntPtrLessThan(index, IntPtrConstant(0)),
                               IntPtrConstant(0), index,
                               MachineType::PointerRepresentation());
       },
       MachineType::PointerRepresentation()));
 
   Goto(&select_loop);
   Bind(&select_loop);
   static int32_t kElementsKind[] = {
       FAST_SMI_ELEMENTS,   FAST_HOLEY_SMI_ELEMENTS, FAST_ELEMENTS,
@@ skipping 29 matching lines @@
     search_num.Bind(LoadHeapNumberValue(search_element));
     Goto(&heap_num_loop);
 
     Bind(&not_heap_num);
     Node* search_type = LoadMapInstanceType(map);
     GotoIf(IsStringInstanceType(search_type), &string_loop);
     Goto(&ident_loop);
 
     Bind(&ident_loop);
     {
-      GotoIfNot(UintPtrLessThan(index_var.value(), len_var.value()),
-                &return_false);
+      GotoIfNot(UintPtrLessThan(index_var.value(), len), &return_false);
       Node* element_k = LoadFixedArrayElement(elements, index_var.value());
       GotoIf(WordEqual(element_k, search_element), &return_true);
 
       index_var.Bind(IntPtrAdd(index_var.value(), IntPtrConstant(1)));
       Goto(&ident_loop);
     }
 
     Bind(&undef_loop);
     {
-      GotoIfNot(UintPtrLessThan(index_var.value(), len_var.value()),
-                &return_false);
+      GotoIfNot(UintPtrLessThan(index_var.value(), len), &return_false);
       Node* element_k = LoadFixedArrayElement(elements, index_var.value());
       GotoIf(WordEqual(element_k, UndefinedConstant()), &return_true);
       GotoIf(WordEqual(element_k, TheHoleConstant()), &return_true);
 
       index_var.Bind(IntPtrAdd(index_var.value(), IntPtrConstant(1)));
       Goto(&undef_loop);
     }
 
     Bind(&heap_num_loop);
     {
       Label nan_loop(this, &index_var), not_nan_loop(this, &index_var);
       BranchIfFloat64IsNaN(search_num.value(), &nan_loop, &not_nan_loop);
 
       Bind(&not_nan_loop);
       {
         Label continue_loop(this), not_smi(this);
-        GotoIfNot(UintPtrLessThan(index_var.value(), len_var.value()),
-                  &return_false);
+        GotoIfNot(UintPtrLessThan(index_var.value(), len), &return_false);
         Node* element_k = LoadFixedArrayElement(elements, index_var.value());
         GotoIfNot(TaggedIsSmi(element_k), &not_smi);
         Branch(Float64Equal(search_num.value(), SmiToFloat64(element_k)),
                &return_true, &continue_loop);
 
         Bind(&not_smi);
         GotoIfNot(IsHeapNumber(element_k), &continue_loop);
         Branch(Float64Equal(search_num.value(), LoadHeapNumberValue(element_k)),
                &return_true, &continue_loop);
 
         Bind(&continue_loop);
         index_var.Bind(IntPtrAdd(index_var.value(), IntPtrConstant(1)));
         Goto(&not_nan_loop);
       }
 
       Bind(&nan_loop);
       {
         Label continue_loop(this);
-        GotoIfNot(UintPtrLessThan(index_var.value(), len_var.value()),
-                  &return_false);
+        GotoIfNot(UintPtrLessThan(index_var.value(), len), &return_false);
         Node* element_k = LoadFixedArrayElement(elements, index_var.value());
         GotoIf(TaggedIsSmi(element_k), &continue_loop);
         GotoIfNot(IsHeapNumber(element_k), &continue_loop);
         BranchIfFloat64IsNaN(LoadHeapNumberValue(element_k), &return_true,
                              &continue_loop);
 
         Bind(&continue_loop);
         index_var.Bind(IntPtrAdd(index_var.value(), IntPtrConstant(1)));
         Goto(&nan_loop);
       }
     }
 
     Bind(&string_loop);
     {
       Label continue_loop(this);
-      GotoIfNot(UintPtrLessThan(index_var.value(), len_var.value()),
-                &return_false);
+      GotoIfNot(UintPtrLessThan(index_var.value(), len), &return_false);
       Node* element_k = LoadFixedArrayElement(elements, index_var.value());
       GotoIf(TaggedIsSmi(element_k), &continue_loop);
       GotoIfNot(IsStringInstanceType(LoadInstanceType(element_k)),
                 &continue_loop);
 
       // TODO(bmeurer): Consider inlining the StringEqual logic here.
       Node* result = CallStub(CodeFactory::StringEqual(isolate()), context,
                               search_element, element_k);
       Branch(WordEqual(BooleanConstant(true), result), &return_true,
              &continue_loop);
@@ skipping 18 matching lines @@
     GotoIfNot(IsHeapNumber(search_element), &return_false);
 
     search_num.Bind(LoadHeapNumberValue(search_element));
 
     BranchIfFloat64IsNaN(search_num.value(), &nan_loop, &not_nan_loop);
 
     // Search for HeapNumber
     Bind(&not_nan_loop);
     {
       Label continue_loop(this);
-      GotoIfNot(UintPtrLessThan(index_var.value(), len_var.value()),
-                &return_false);
+      GotoIfNot(UintPtrLessThan(index_var.value(), len), &return_false);
       Node* element_k = LoadFixedDoubleArrayElement(elements, index_var.value(),
                                                     MachineType::Float64());
       Branch(Float64Equal(element_k, search_num.value()), &return_true,
              &continue_loop);
       Bind(&continue_loop);
       index_var.Bind(IntPtrAdd(index_var.value(), IntPtrConstant(1)));
       Goto(&not_nan_loop);
     }
 
     // Search for NaN
     Bind(&nan_loop);
     {
       Label continue_loop(this);
-      GotoIfNot(UintPtrLessThan(index_var.value(), len_var.value()),
-                &return_false);
+      GotoIfNot(UintPtrLessThan(index_var.value(), len), &return_false);
       Node* element_k = LoadFixedDoubleArrayElement(elements, index_var.value(),
                                                     MachineType::Float64());
       BranchIfFloat64IsNaN(element_k, &return_true, &continue_loop);
       Bind(&continue_loop);
       index_var.Bind(IntPtrAdd(index_var.value(), IntPtrConstant(1)));
       Goto(&nan_loop);
     }
   }
 
   Bind(&if_holey_doubles);
@@ skipping 11 matching lines @@
     GotoIfNot(IsHeapNumber(search_element), &return_false);
 
     search_num.Bind(LoadHeapNumberValue(search_element));
 
     BranchIfFloat64IsNaN(search_num.value(), &nan_loop, &not_nan_loop);
 
     // Search for HeapNumber
     Bind(&not_nan_loop);
     {
       Label continue_loop(this);
-      GotoIfNot(UintPtrLessThan(index_var.value(), len_var.value()),
-                &return_false);
+      GotoIfNot(UintPtrLessThan(index_var.value(), len), &return_false);
 
       // Load double value or continue if it contains a double hole.
       Node* element_k = LoadFixedDoubleArrayElement(
           elements, index_var.value(), MachineType::Float64(), 0,
           CodeStubAssembler::INTPTR_PARAMETERS, &continue_loop);
 
       Branch(Float64Equal(element_k, search_num.value()), &return_true,
              &continue_loop);
       Bind(&continue_loop);
       index_var.Bind(IntPtrAdd(index_var.value(), IntPtrConstant(1)));
       Goto(&not_nan_loop);
     }
 
     // Search for NaN
     Bind(&nan_loop);
     {
       Label continue_loop(this);
-      GotoIfNot(UintPtrLessThan(index_var.value(), len_var.value()),
-                &return_false);
+      GotoIfNot(UintPtrLessThan(index_var.value(), len), &return_false);
 
       // Load double value or continue if it contains a double hole.
       Node* element_k = LoadFixedDoubleArrayElement(
           elements, index_var.value(), MachineType::Float64(), 0,
           CodeStubAssembler::INTPTR_PARAMETERS, &continue_loop);
 
       BranchIfFloat64IsNaN(element_k, &return_true, &continue_loop);
       Bind(&continue_loop);
       index_var.Bind(IntPtrAdd(index_var.value(), IntPtrConstant(1)));
       Goto(&nan_loop);
     }
 
     // Search for the Hole
     Bind(&hole_loop);
     {
-      GotoIfNot(UintPtrLessThan(index_var.value(), len_var.value()),
-                &return_false);
+      GotoIfNot(UintPtrLessThan(index_var.value(), len), &return_false);
 
       // Check if the element is a double hole, but don't load it.
       LoadFixedDoubleArrayElement(
           elements, index_var.value(), MachineType::None(), 0,
           CodeStubAssembler::INTPTR_PARAMETERS, &return_true);
 
       index_var.Bind(IntPtrAdd(index_var.value(), IntPtrConstant(1)));
       Goto(&hole_loop);
     }
   }
@@ skipping 875 matching lines @@
   {
     Node* message = assembler.SmiConstant(MessageTemplate::kDetachedOperation);
     assembler.CallRuntime(Runtime::kThrowTypeError, context, message,
                           assembler.HeapConstant(operation));
     assembler.Unreachable();
   }
 }
 
 }  // namespace internal
 }  // namespace v8