[turbofan] Introduce a dedicated CheckBounds operator.

This CheckBounds simplified operator is similar to the HBoundsCheck in
Crankshaft, and is hooked up to the new type feedback support in the
SimplifiedLowering. We use it to check the index bounds for keyed
property accesses.

Note to perf sheriffs: This will tank quite a few benchmarks, as the
operator makes some redundant branch elimination ineffective for
certain patterns of keyed accesses. This does require more serious
redundancy elimination, which we will do in a separate CL. So ignore
any regressions from this CL, we know there will be a few.

R=jarin@chromium.org
BUG=v8:4470,v8:5100

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

src/compiler/effect-control-linearizer.cc

@@ -416,6 +416,9 @@ bool EffectControlLinearizer::TryWireInStateEffect(Node* node,
     case IrOpcode::kTruncateTaggedToFloat64:
       state = LowerTruncateTaggedToFloat64(node, *effect, *control);
       break;
+    case IrOpcode::kCheckBounds:
+      state = LowerCheckBounds(node, frame_state, *effect, *control);
+      break;
     case IrOpcode::kCheckedUint32ToInt32:
       state = LowerCheckedUint32ToInt32(node, frame_state, *effect, *control);
       break;
@@ -758,6 +761,22 @@ EffectControlLinearizer::LowerTruncateTaggedToFloat64(Node* node, Node* effect,
   return ValueEffectControl(value, effect, control);
 }
 
+EffectControlLinearizer::ValueEffectControl
+EffectControlLinearizer::LowerCheckBounds(Node* node, Node* frame_state,
+                                          Node* effect, Node* control) {
+  Node* index = node->InputAt(0);
+  Node* limit = node->InputAt(1);
+
+  Node* check = graph()->NewNode(machine()->Uint32LessThan(), index, limit);
+  control = effect = graph()->NewNode(common()->DeoptimizeUnless(), check,
+                                      frame_state, effect, control);
+
+  // Make sure the lowered node does not appear in any use lists.
+  node->TrimInputCount(0);
+
+  return ValueEffectControl(index, effect, control);
+}
+
 EffectControlLinearizer::ValueEffectControl
 EffectControlLinearizer::LowerCheckedUint32ToInt32(Node* node,
                                                    Node* frame_state,
@@ -846,26 +865,36 @@ EffectControlLinearizer::LowerCheckedTaggedToInt32(Node* node,
 
   // In the non-Smi case, check the heap numberness, load the number and convert
   // to int32.
-  // TODO(jarin) Propagate/handle possible truncations here.
   Node* if_false = graph()->NewNode(common()->IfFalse(), branch);
-  ValueEffectControl number_state = BuildCheckedHeapNumberOrOddballToFloat64(
-      value, frame_state, effect, if_false);
-  number_state =
-      BuildCheckedFloat64ToInt32(number_state.value, frame_state,
-                                 number_state.effect, number_state.control);
+  Node* efalse = effect;
+  Node* vfalse;
+  {
+    Node* value_map = efalse =
+        graph()->NewNode(simplified()->LoadField(AccessBuilder::ForMap()),
+                         value, efalse, if_false);
+    Node* check = graph()->NewNode(machine()->WordEqual(), value_map,
+                                   jsgraph()->HeapNumberMapConstant());
+    if_false = efalse = graph()->NewNode(common()->DeoptimizeUnless(), check,
+                                         frame_state, efalse, if_false);
+    vfalse = efalse = graph()->NewNode(
+        simplified()->LoadField(AccessBuilder::ForHeapNumberValue()), value,
+        efalse, if_false);
+    ValueEffectControl state =
+        BuildCheckedFloat64ToInt32(vfalse, frame_state, efalse, if_false);
+    if_false = state.control;
+    efalse = state.effect;
+    vfalse = state.value;
+  }
 
-  Node* merge =
-      graph()->NewNode(common()->Merge(2), if_true, number_state.control);
-  Node* effect_phi = graph()->NewNode(common()->EffectPhi(2), etrue,
-                                      number_state.effect, merge);
-  Node* result =
-      graph()->NewNode(common()->Phi(MachineRepresentation::kWord32, 2), vtrue,
-                       number_state.value, merge);
+  control = graph()->NewNode(common()->Merge(2), if_true, if_false);
+  effect = graph()->NewNode(common()->EffectPhi(2), etrue, efalse, control);
+  value = graph()->NewNode(common()->Phi(MachineRepresentation::kWord32, 2),
+                           vtrue, vfalse, control);
 
   // Make sure the lowered node does not appear in any use lists.
   node->TrimInputCount(0);
 
-  return ValueEffectControl(result, effect_phi, merge);
+  return ValueEffectControl(value, effect, control);
 }
 
 EffectControlLinearizer::ValueEffectControl

src/compiler/effect-control-linearizer.h

@@ -62,6 +62,8 @@ class EffectControlLinearizer {
                                               Node* control);
   ValueEffectControl LowerChangeTaggedToUint32(Node* node, Node* effect,
                                                Node* control);
+  ValueEffectControl LowerCheckBounds(Node* node, Node* frame_state,
+                                      Node* effect, Node* control);
   ValueEffectControl LowerCheckedUint32ToInt32(Node* node, Node* frame_state,
                                                Node* effect, Node* control);
   ValueEffectControl LowerCheckedFloat64ToInt32(Node* node, Node* frame_state,

src/compiler/js-native-context-specialization.cc

@@ -636,6 +636,17 @@ Reduction JSNativeContextSpecialization::ReduceElementAccess(
               receiver, jsgraph()->HeapConstant(transition_target), context,
               frame_state, transition_effect, transition_control);
         }
+
+        // TODO(turbofan): The effect/control linearization will not find a
+        // FrameState after the StoreField or Call that is generated for the
+        // elements kind transition above. This is because those operatos don't
+        // have the kNoWrite flag on it, even tho they are not JavaScript
+        // observable, but at the same time adding kNoWrite would make them
+        // eliminatable during instruction selection (at least the Call one).
+        transition_effect =
+            graph()->NewNode(common()->Checkpoint(), frame_state,
+                             transition_effect, transition_control);
+
         this_controls.push_back(transition_control);
         this_effects.push_back(transition_effect);
       }
@@ -653,6 +664,13 @@ Reduction JSNativeContextSpecialization::ReduceElementAccess(
         this_effect =
             graph()->NewNode(common()->EffectPhi(this_control_count),
                              this_control_count + 1, &this_effects.front());
+
+        // TODO(turbofan): This is another work-around, which is necessary
+        // in addition to the Checkpoint above, as the CheckpointElimination
+        // is not really compositional. We really need a way to address the
+        // "no-write" problem on non-side-effecting nodes.
+        this_effect = graph()->NewNode(common()->Checkpoint(), frame_state,
+                                       this_effect, this_control);
       }
     }
 
@@ -664,30 +682,6 @@ Reduction JSNativeContextSpecialization::ReduceElementAccess(
       AssumePrototypesStable(receiver_type, native_context, holder);
     }
 
-    // Check that the {index} is actually a Number.
-    if (!NumberMatcher(this_index).HasValue()) {
-      Node* check =
-          graph()->NewNode(simplified()->ObjectIsNumber(), this_index);
-      this_control = this_effect =
-          graph()->NewNode(common()->DeoptimizeUnless(), check, frame_state,
-                           this_effect, this_control);
-      this_index = graph()->NewNode(simplified()->TypeGuard(Type::Number()),
-                                    this_index, this_control);
-    }
-
-    // Convert the {index} to an unsigned32 value and check if the result is
-    // equal to the original {index}.
-    if (!NumberMatcher(this_index).IsInRange(0.0, kMaxUInt32)) {
-      Node* this_index32 =
-          graph()->NewNode(simplified()->NumberToUint32(), this_index);
-      Node* check = graph()->NewNode(simplified()->NumberEqual(), this_index32,
-                                     this_index);
-      this_control = this_effect =
-          graph()->NewNode(common()->DeoptimizeUnless(), check, frame_state,
-                           this_effect, this_control);
-      this_index = this_index32;
-    }
-
     // TODO(bmeurer): We currently specialize based on elements kind. We should
     // also be able to properly support strings and other JSObjects here.
     ElementsKind elements_kind = access_info.elements_kind();
@@ -723,10 +717,8 @@ Reduction JSNativeContextSpecialization::ReduceElementAccess(
                   this_elements, this_effect, this_control);
 
     // Check that the {index} is in the valid range for the {receiver}.
-    Node* check = graph()->NewNode(simplified()->NumberLessThan(), this_index,
-                                   this_length);
-    this_control = this_effect =
-        graph()->NewNode(common()->DeoptimizeUnless(), check, frame_state,
+    this_index = this_effect =
+        graph()->NewNode(simplified()->CheckBounds(), this_index, this_length,
                          this_effect, this_control);
 
     // Compute the element access.

src/compiler/opcodes.h

@@ -218,6 +218,7 @@
   V(ChangeFloat64ToTagged)         \
   V(ChangeTaggedToBit)             \
   V(ChangeBitToTagged)             \
+  V(CheckBounds)                   \
   V(CheckedUint32ToInt32)          \
   V(CheckedFloat64ToInt32)         \
   V(CheckedTaggedToInt32)          \

src/compiler/simplified-lowering.cc

@@ -1540,6 +1540,13 @@ class RepresentationSelector {
         }
         return;
       }
+
+      case IrOpcode::kCheckBounds: {
+        VisitBinop(node, UseInfo::CheckedSigned32AsWord32(),
+                   UseInfo::TruncatingWord32(), MachineRepresentation::kWord32);
+        return;
+      }
+
       case IrOpcode::kAllocate: {
         ProcessInput(node, 0, UseInfo::TruncatingWord32());
         ProcessRemainingInputs(node, 1);

src/compiler/simplified-operator.cc

@@ -332,6 +332,12 @@ const Operator* SimplifiedOperatorBuilder::ReferenceEqual(Type* type) {
                                "ReferenceEqual", 2, 0, 0, 1, 0, 0);
 }
 
+const Operator* SimplifiedOperatorBuilder::CheckBounds() {
+  // TODO(bmeurer): Cache this operator. Make it pure!
+  return new (zone()) Operator(IrOpcode::kCheckBounds, Operator::kEliminatable,
+                               "CheckBounds", 2, 1, 1, 1, 1, 0);
+}
+
 const Operator* SimplifiedOperatorBuilder::TypeGuard(Type* type) {
   class TypeGuardOperator final : public Operator1<Type*> {
    public:

src/compiler/simplified-operator.h

@@ -192,6 +192,8 @@ class SimplifiedOperatorBuilder final : public ZoneObject {
   const Operator* TruncateTaggedToWord32();
   const Operator* TruncateTaggedToFloat64();
 
+  const Operator* CheckBounds();
+
   const Operator* CheckedUint32ToInt32();
   const Operator* CheckedFloat64ToInt32();
   const Operator* CheckedTaggedToInt32();

src/compiler/typer.cc

@@ -1946,6 +1946,11 @@ Type* Typer::Visitor::TypeChangeBitToTagged(Node* node) {
   return ChangeRepresentation(arg, Type::TaggedPointer(), zone());
 }
 
+Type* Typer::Visitor::TypeCheckBounds(Node* node) {
+  // TODO(bmeurer): We could do better here based on the limit.
+  return Type::Unsigned31();
+}
+
 Type* Typer::Visitor::TypeCheckedUint32ToInt32(Node* node) {
   return Type::Signed32();
 }

src/compiler/verifier.cc

@@ -928,6 +928,12 @@ void Verifier::Visitor::Check(Node* node) {
       break;
     }
 
+    case IrOpcode::kCheckBounds:
+      CheckValueInputIs(node, 0, Type::Any());
+      CheckValueInputIs(node, 1, Type::Unsigned31());
+      CheckUpperIs(node, Type::Unsigned31());
+      break;
+
     case IrOpcode::kCheckedUint32ToInt32:
     case IrOpcode::kCheckedFloat64ToInt32:
     case IrOpcode::kCheckedTaggedToInt32:

test/mjsunit/compiler/regress-5100.js

+// 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.
+
+// Flags: --allow-natives-syntax
+
+var a = [0, 1];
+a["true"] = "true";
+a["false"] = "false";
+a["null"] = "null";
+a["undefined"] = "undefined";
+
+// Ensure we don't accidentially truncate true when used to index arrays.
+(function() {
+  function f(x) { return a[x]; }
+
+  assertEquals(0, f(0));
+  assertEquals(0, f(0));
+  %OptimizeFunctionOnNextCall(f);
+  assertEquals("true", f(true));
+})();
+
+// Ensure we don't accidentially truncate false when used to index arrays.
+(function() {
+  function f( x) { return a[x]; }
+
+  assertEquals(0, f(0));
+  assertEquals(0, f(0));
+  %OptimizeFunctionOnNextCall(f);
+  assertEquals("false", f(false));
+})();
+
+// Ensure we don't accidentially truncate null when used to index arrays.
+(function() {
+  function f( x) { return a[x]; }
+
+  assertEquals(0, f(0));
+  assertEquals(0, f(0));
+  %OptimizeFunctionOnNextCall(f);
+  assertEquals("null", f(null));
+})();
+
+// Ensure we don't accidentially truncate undefined when used to index arrays.
+(function() {
+  function f( x) { return a[x]; }
+
+  assertEquals(0, f(0));
+  assertEquals(0, f(0));
+  %OptimizeFunctionOnNextCall(f);
+  assertEquals("undefined", f(undefined));
+})();