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 1684 matching lines @@
   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);
 
   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);
+    // JSArray length is always an Smi for fast arrays.
+    CSA_ASSERT(this,
+               TaggedIsSmi(LoadObjectField(array, JSArray::kLengthOffset)));
+    Node* len = LoadAndUntagObjectField(array, JSArray::kLengthOffset);
 
-    len_var.Bind(SmiToWord(len));
+    len_var.Bind(len);

[caitp, 2017/02/25 18:03:06]

Do we still need `len_var`? It never changes at any point in the algorithm, I
think

[vabr (Chromium), 2017/02/25 18:35:48]

So that I learn more:
(1) Changing would mean that there is a len_var.Bind() call somewhere after this
line?
(2) If I remove len_var, then I should keep the Node* pointer (len) and replace
all len_var.value() calls with it, correct?

[caitp, 2017/02/25 18:40:12]

There's another Bind() call before this line, but IIRC the intent was just to
make the compiler happy having a bound value for len in the call_runtime block
(which doesn't use it). So, as far as I can tell (based on a quick look), this
is the only Bind that does anything useful (e.g. changes the value
meaningfully).
Yeah, I think that would be better.

 
     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);
@@ skipping 1129 matching lines @@
   {
     Node* message = assembler.SmiConstant(MessageTemplate::kDetachedOperation);
     assembler.CallRuntime(Runtime::kThrowTypeError, context, message,
                           assembler.HeapConstant(operation));
     assembler.Unreachable();
   }
 }
 
 }  // namespace internal
 }  // namespace v8