[turbofan] Speculate on bounds checks for String#char[Code]At

With the new builtin optimization guard we can just speculatively assume
that the index passed to String#charAt and String#charCodeAt (in
optimized
code) is going to be within the valid range for the receiver. This is
what Crankshaft used to do, and it avoids Smi checks on the result for
String#charCodeAt, since it can no longer return NaN.

This gives rise to further optimizations of these builtins (i.e. to
completely avoid the tagging of char codes), and by itself already
improves the regression test originally reported from 650ms to
610ms.

Bug: v8:7127, v8:7326
Change-Id: I6c160540a1e002a37e44fa7f920e5e8f8c2c4210
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_chromium_rel_ng
Reviewed-on: https://chromium-review.googlesource.com/873382
Commit-Queue: Sigurd Schneider <sigurds@chromium.org>
Reviewed-by: Jaroslav Sevcik <jarin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#50888}

src/compiler/js-call-reducer.cc

@@ -2955,15 +2955,13 @@ Reduction JSCallReducer::ReduceJSCall(Node* node) {
           return ReduceStringPrototypeIndexOf(function, node);
         case Builtins::kStringPrototypeCharAt:
           return ReduceStringPrototypeStringAt(simplified()->StringCharAt(),
-                                               jsgraph()->EmptyStringConstant(),
                                                node);
         case Builtins::kStringPrototypeCharCodeAt:
           return ReduceStringPrototypeStringAt(simplified()->StringCharCodeAt(),
-                                               jsgraph()->NaNConstant(), node);
+                                               node);
         case Builtins::kStringPrototypeCodePointAt:
           return ReduceStringPrototypeStringAt(
-              simplified()->StringCodePointAt(), jsgraph()->UndefinedConstant(),
-              node);
+              simplified()->StringCodePointAt(), node);
         case Builtins::kAsyncFunctionPromiseCreate:
           return ReduceAsyncFunctionPromiseCreate(node);
         case Builtins::kAsyncFunctionPromiseRelease:
@@ -3859,7 +3857,7 @@ Reduction JSCallReducer::ReduceArrayPrototypeShift(Node* node) {
 // and
 // ES6 section 21.1.3.3 String.prototype.codePointAt ( pos )
 Reduction JSCallReducer::ReduceStringPrototypeStringAt(
-    const Operator* string_access_operator, Node* default_return, Node* node) {
+    const Operator* string_access_operator, Node* node) {
   DCHECK(string_access_operator->opcode() == IrOpcode::kStringCharAt ||
          string_access_operator->opcode() == IrOpcode::kStringCharCodeAt ||
          string_access_operator->opcode() == IrOpcode::kStringCodePointAt);
@@ -3870,10 +3868,13 @@ Reduction JSCallReducer::ReduceStringPrototypeStringAt(
   }
 
   Node* receiver = NodeProperties::GetValueInput(node, 1);
-  Node* index = jsgraph()->ZeroConstant();
+  Node* index = node->op()->ValueInputCount() >= 3
+                    ? NodeProperties::GetValueInput(node, 2)
+                    : jsgraph()->ZeroConstant();
   Node* effect = NodeProperties::GetEffectInput(node);
   Node* control = NodeProperties::GetControlInput(node);
 
+  // Ensure that the {receiver} is actually a String.
   receiver = effect = graph()->NewNode(simplified()->CheckString(p.feedback()),
                                        receiver, effect, control);
 
@@ -3881,41 +3882,15 @@ Reduction JSCallReducer::ReduceStringPrototypeStringAt(
   Node* receiver_length =
       graph()->NewNode(simplified()->StringLength(), receiver);
 
-  if (node->op()->ValueInputCount() >= 3) {
-    index = effect = graph()->NewNode(simplified()->CheckSmi(p.feedback()),
-                                      NodeProperties::GetValueInput(node, 2),
-                                      effect, control);
-    // Map -0 and NaN to 0 (as per ToInteger), and the values in
-    // the [-2^31,-1] range to the [2^31,2^32-1] range, which will
-    // be considered out-of-bounds as well, because of the maximal
-    // String length limit in V8.
-    STATIC_ASSERT(String::kMaxLength <= kMaxInt);
-    index = graph()->NewNode(simplified()->NumberToUint32(), index);
-  }
-
-  // Check if {index} is less than {receiver} length.
-  Node* check =
-      graph()->NewNode(simplified()->NumberLessThan(), index, receiver_length);
-  Node* branch =
-      graph()->NewNode(common()->Branch(BranchHint::kTrue), check, control);
-
-  // Get the character from {receiver} via operator {charAtOrCharCodeAt}.
-  Node* if_true = graph()->NewNode(common()->IfTrue(), branch);
+  // Check that the {index} is within range.
+  index = effect = graph()->NewNode(simplified()->CheckBounds(p.feedback()),
+                                    index, receiver_length, effect, control);
 
-  index = graph()->NewNode(simplified()->MaskIndexWithBound(), index,
-                           receiver_length);
-  Node* etrue;
-  Node* vtrue = etrue = graph()->NewNode(string_access_operator, receiver,
-                                         index, effect, if_true);
-
-  // Return the {default_return} otherwise.
-  Node* if_false = graph()->NewNode(common()->IfFalse(), branch);
-  Node* vfalse = default_return;
-
-  control = graph()->NewNode(common()->Merge(2), if_true, if_false);
-  Node* value = graph()->NewNode(
-      common()->Phi(MachineRepresentation::kTagged, 2), vtrue, vfalse, control);
-  effect = graph()->NewNode(common()->EffectPhi(2), etrue, effect, control);
+  // Return the character from the {receiver} as single character string.
+  Node* masked_index = graph()->NewNode(simplified()->MaskIndexWithBound(),
+                                        index, receiver_length);
+  Node* value = effect = graph()->NewNode(string_access_operator, receiver,
+                                          masked_index, effect, control);
 
   ReplaceWithValue(node, value, effect, control);
   return Replace(value);

src/compiler/js-call-reducer.h

@@ -98,7 +98,7 @@ class JSCallReducer final : public AdvancedReducer {
   Reduction ReduceStringPrototypeIndexOf(Handle<JSFunction> function,
                                          Node* node);
   Reduction ReduceStringPrototypeStringAt(
-      const Operator* string_access_operator, Node* default_return, Node* node);
+      const Operator* string_access_operator, Node* node);
   Reduction ReduceAsyncFunctionPromiseCreate(Node* node);
   Reduction ReduceAsyncFunctionPromiseRelease(Node* node);
 

test/mjsunit/constant-folding-2.js

@@ -242,6 +242,7 @@ test(function stringCharAt() {
   assertEquals("", "abc".charAt(-Infinity));
   assertEquals("a", "abc".charAt(-0));
   assertEquals("a", "abc".charAt(+0));
+  assertEquals("", "".charAt());
   assertEquals("", "abc".charAt(1 + 4294967295));
 }, 10);
 
@@ -255,6 +256,7 @@ test(function stringCharCodeAt() {
   assertSame(NaN, "abc".charCodeAt(-Infinity));
   assertSame(97, "abc".charCodeAt(-0));
   assertSame(97, "abc".charCodeAt(+0));
+  assertSame(NaN, "".charCodeAt());
   assertSame(NaN, "abc".charCodeAt(1 + 4294967295));
 }, 10);
 
@@ -269,6 +271,7 @@ test(function stringCodePointAt() {
   assertSame(undefined, "äϠ�".codePointAt(-Infinity));
   assertSame(228, "äϠ�".codePointAt(-0));
   assertSame(97, "aϠ�".codePointAt(+0));
+  assertSame(undefined, "".codePointAt());
   assertSame(undefined, "äϠ�".codePointAt(1 + 4294967295));
 }, 10);
 

test/mjsunit/regress/regress-charat-empty.js

+// Copyright 2018 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.
+
+// Flags: --opt --no-always-opt --allow-natives-syntax
+(() => {
+  function f(s) {
+    return s.charAt();
+  }
+  f("");
+  f("");
+  %OptimizeFunctionOnNextCall(f);
+  f("");
+})();