[turbofan] Speculatively optimize string character access.

Add a protector cell for string bounds checks that is being used to
protect speculative bounds for String.prototype.charCodeAt and
String.prototype.charAt in TurboFan (and Crankshaft). This way we don't
have the diamond in optimized code, which stands in the way of other
optimizations for charCodeAt that are currently being worked on by
petermarshall@.

BUG=v8:6391
TBR=mlippautz@chromium.org
R=petermarshall@chromium.org

Review-Url: https://codereview.chromium.org/2905623003
Cr-Commit-Position: refs/heads/master@{#45514}

src/builtins/builtins-string-gen.cc

@@ -656,7 +656,13 @@ TF_BUILTIN(StringPrototypeCharAt, CodeStubAssembler) {
            &if_positioninbounds);
 
     BIND(&return_emptystring);
-    Return(EmptyStringConstant());
+    {
+      // Invalidate the "String Bounds Check" protector.
+      Node* invalid = SmiConstant(Isolate::kProtectorInvalid);
+      Node* cell = LoadRoot(Heap::kStringBoundsCheckProtectorRootIndex);
+      StoreObjectFieldNoWriteBarrier(cell, Cell::kValueOffset, invalid);
+      Return(EmptyStringConstant());
+    }
 
     BIND(&if_positioninbounds);
   }
@@ -695,7 +701,13 @@ TF_BUILTIN(StringPrototypeCharCodeAt, CodeStubAssembler) {
            &if_positioninbounds);
 
     BIND(&return_nan);
-    Return(NaNConstant());
+    {
+      // Invalidate the "String Bounds Check" protector.
+      Node* invalid = SmiConstant(Isolate::kProtectorInvalid);
+      Node* cell = LoadRoot(Heap::kStringBoundsCheckProtectorRootIndex);
+      StoreObjectFieldNoWriteBarrier(cell, Cell::kValueOffset, invalid);
+      Return(NaNConstant());
+    }
 
     BIND(&if_positioninbounds);
   }

src/compiler/js-builtin-reducer.cc

@@ -1922,8 +1922,26 @@ Reduction JSBuiltinReducer::ReduceStringCharAt(Node* node) {
     Node* effect = NodeProperties::GetEffectInput(node);
     Node* control = NodeProperties::GetControlInput(node);
 
-    if (index_type->Is(Type::Integral32OrMinusZeroOrNaN())) {
-      if (Node* receiver = GetStringWitness(node)) {
+    if (Node* receiver = GetStringWitness(node)) {
+      if (isolate()->IsStringBoundsCheckIntact()) {
+        // Determine the {receiver} length.
+        Node* receiver_length = effect = graph()->NewNode(
+            simplified()->LoadField(AccessBuilder::ForStringLength()), receiver,
+            effect, control);
+
+        // Check that the {index} is within the range of {receiver}.
+        index = effect = graph()->NewNode(simplified()->CheckBounds(), index,
+                                          receiver_length, effect, control);
+
+        // Return the character from the {receiver} as single character string.
+        Node* value = graph()->NewNode(simplified()->StringCharAt(), receiver,
+                                       index, control);
+
+        ReplaceWithValue(node, value, effect, control);
+        return Replace(value);
+      }
+
+      if (index_type->Is(Type::Integral32OrMinusZeroOrNaN())) {
         if (!index_type->Is(Type::Unsigned32())) {
           // 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
@@ -1976,8 +1994,26 @@ Reduction JSBuiltinReducer::ReduceStringCharCodeAt(Node* node) {
     Node* effect = NodeProperties::GetEffectInput(node);
     Node* control = NodeProperties::GetControlInput(node);
 
-    if (index_type->Is(Type::Integral32OrMinusZeroOrNaN())) {
-      if (Node* receiver = GetStringWitness(node)) {
+    if (Node* receiver = GetStringWitness(node)) {
+      if (isolate()->IsStringBoundsCheckIntact()) {
+        // Determine the {receiver} length.
+        Node* receiver_length = effect = graph()->NewNode(
+            simplified()->LoadField(AccessBuilder::ForStringLength()), receiver,
+            effect, control);
+
+        // Check that the {index} is within the range of {receiver}.
+        index = effect = graph()->NewNode(simplified()->CheckBounds(), index,
+                                          receiver_length, effect, control);
+
+        // Return the character from the {receiver} as character code.
+        Node* value = graph()->NewNode(simplified()->StringCharCodeAt(),
+                                       receiver, index, control);
+
+        ReplaceWithValue(node, value, effect, control);
+        return Replace(value);
+      }
+
+      if (index_type->Is(Type::Integral32OrMinusZeroOrNaN())) {
         if (!index_type->Is(Type::Unsigned32())) {
           // 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

src/crankshaft/hydrogen.cc

@@ -8351,7 +8351,7 @@ bool HOptimizedGraphBuilder::TryInlineBuiltinMethodCall(
     }
     case kStringCharCodeAt:
     case kStringCharAt:
-      if (argument_count == 2) {
+      if (argument_count == 2 && isolate()->IsStringBoundsCheckIntact()) {
         HValue* index = Pop();
         HValue* string = Pop();
         Drop(1);  // Function.

src/heap/heap.cc

@@ -2855,6 +2855,10 @@ void Heap::CreateInitialObjects() {
       handle(Smi::FromInt(Isolate::kProtectorValid), isolate()));
   set_species_protector(*species_cell);
 
+  Handle<Cell> string_bounds_check_cell = factory->NewCell(
+      handle(Smi::FromInt(Isolate::kProtectorValid), isolate()));
+  set_string_bounds_check_protector(*string_bounds_check_cell);
+
   cell = factory->NewPropertyCell();
   cell->set_value(Smi::FromInt(Isolate::kProtectorValid));
   set_string_length_protector(*cell);

src/heap/heap.h

@@ -172,6 +172,7 @@ using v8::MemoryPressureLevel;
   V(PropertyCell, array_protector, ArrayProtector)                             \
   V(Cell, is_concat_spreadable_protector, IsConcatSpreadableProtector)         \
   V(Cell, species_protector, SpeciesProtector)                                 \
+  V(Cell, string_bounds_check_protector, StringBoundsCheckProtector)           \
   V(PropertyCell, string_length_protector, StringLengthProtector)              \
   V(Cell, fast_array_iteration_protector, FastArrayIterationProtector)         \
   V(PropertyCell, array_iterator_protector, ArrayIteratorProtector)            \

src/isolate-inl.h

@@ -133,6 +133,11 @@ bool Isolate::IsArraySpeciesLookupChainIntact() {
          Smi::cast(species_cell->value())->value() == kProtectorValid;
 }
 
+bool Isolate::IsStringBoundsCheckIntact() {
+  Cell* string_bounds_check_cell = heap()->string_bounds_check_protector();
+  return string_bounds_check_cell->value() == Smi::FromInt(kProtectorValid);
+}
+
 bool Isolate::IsStringLengthOverflowIntact() {
   PropertyCell* string_length_cell = heap()->string_length_protector();
   return string_length_cell->value() == Smi::FromInt(kProtectorValid);

src/isolate.h

@@ -1041,6 +1041,7 @@ class Isolate {
   inline bool IsArraySpeciesLookupChainIntact();
   bool IsIsConcatSpreadableLookupChainIntact();
   bool IsIsConcatSpreadableLookupChainIntact(JSReceiver* receiver);
+  inline bool IsStringBoundsCheckIntact();
   inline bool IsStringLengthOverflowIntact();
   inline bool IsArrayIteratorLookupChainIntact();
 

test/mjsunit/regress/regress-6391.js

+// Copyright 2017 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: --allow-natives-syntax --opt --no-always-opt
+
+(function() {
+  function foo(i) { return "a".charAt(i); }
+
+  assertEquals("a", foo(0));
+  assertEquals("a", foo(0));
+  assertEquals("", foo(1));
+  %OptimizeFunctionOnNextCall(foo);
+  assertEquals("", foo(1));
+  assertOptimized(foo);
+})();
+
+(function() {
+  function foo(i) { return "a".charCodeAt(i); }
+
+  assertEquals(97, foo(0));
+  assertEquals(97, foo(0));
+  assertEquals(NaN, foo(1));
+  %OptimizeFunctionOnNextCall(foo);
+  assertEquals(NaN, foo(1));
+  assertOptimized(foo);
+})();