[turbofan] Add frame state before JavaScript comparisons.

Use these check points to optimize comparisons where we already know
that one side cannot be a String (or turn into a string via
ToPrimitive).

Also remove bunch of useless DoNotCrash tests for the scheduler that are
painful to maintain and add almost no value.

R=jarin@chromium.org

Review URL: https://codereview.chromium.org/1140583004

Cr-Commit-Position: refs/heads/master@{#28383}

src/compiler/ast-graph-builder.cc

@@ -1311,10 +1311,12 @@ void AstGraphBuilder::VisitForInBody(ForInStatement* stmt) {
   Node* obj = environment()->Peek(4);
 
   // Check loop termination condition.
+  FrameStateBeforeAndAfter states(this, BailoutId::None());
   Node* exit_cond = NewNode(javascript()->LessThan(LanguageMode::SLOPPY),
                             index, cache_length);
   // TODO(jarin): provide real bailout id.
-  PrepareFrameState(exit_cond, BailoutId::None());
+  states.AddToNode(exit_cond, BailoutId::None(),
+                   OutputFrameStateCombine::Ignore());
   for_loop.BreakUnless(exit_cond);
   Node* pair = NewNode(javascript()->CallRuntime(Runtime::kForInNext, 4), obj,
                        cache_array, cache_type, index);
@@ -2531,10 +2533,11 @@ void AstGraphBuilder::VisitCompareOperation(CompareOperation* expr) {
   }
   VisitForValue(expr->left());
   VisitForValue(expr->right());
+  FrameStateBeforeAndAfter states(this, expr->right()->id());
   Node* right = environment()->Pop();
   Node* left = environment()->Pop();
   Node* value = NewNode(op, left, right);
-  PrepareFrameState(value, expr->id(), ast_context()->GetStateCombine());
+  states.AddToNode(value, expr->id(), ast_context()->GetStateCombine());
   ast_context()->ProduceValue(value);
 }
 

src/compiler/js-typed-lowering.cc

@@ -121,11 +121,6 @@ class JSBinopReduction final {
   JSBinopReduction(JSTypedLowering* lowering, Node* node)
       : lowering_(lowering), node_(node) {}
 
-  void ConvertPrimitiveInputsToNumber() {
-    node_->ReplaceInput(0, ConvertPrimitiveToNumber(left()));
-    node_->ReplaceInput(1, ConvertPrimitiveToNumber(right()));
-  }
-
   void ConvertInputsToNumber(Node* frame_state) {
     // To convert the inputs to numbers, we have to provide frame states
     // for lazy bailouts in the ToNumber conversions.
@@ -137,13 +132,13 @@ class JSBinopReduction final {
 
     Node* left_input =
         left_type()->Is(Type::PlainPrimitive())
-            ? ConvertPrimitiveToNumber(left())
+            ? ConvertPlainPrimitiveToNumber(left())
             : ConvertToNumber(left(),
                               CreateFrameStateForLeftInput(frame_state));
 
     Node* right_input =
         right_type()->Is(Type::PlainPrimitive())
-            ? ConvertPrimitiveToNumber(right())
+            ? ConvertPlainPrimitiveToNumber(right())
             : ConvertToNumber(right(), CreateFrameStateForRightInput(
                                            frame_state, left_input));
 
@@ -164,9 +159,10 @@ class JSBinopReduction final {
 
   // Convert inputs for bitwise shift operation (ES5 spec 11.7).
   void ConvertInputsForShift(Signedness left_signedness) {
-    node_->ReplaceInput(
-        0, ConvertToUI32(ConvertPrimitiveToNumber(left()), left_signedness));
-    Node* rnum = ConvertToUI32(ConvertPrimitiveToNumber(right()), kUnsigned);
+    node_->ReplaceInput(0, ConvertToUI32(ConvertPlainPrimitiveToNumber(left()),
+                                         left_signedness));
+    Node* rnum =
+        ConvertToUI32(ConvertPlainPrimitiveToNumber(right()), kUnsigned);
     Type* rnum_type = NodeProperties::GetBounds(rnum).upper;
     if (!rnum_type->Is(lowering_->zero_thirtyone_range_)) {
       rnum = graph()->NewNode(machine()->Word32And(), rnum,
@@ -333,13 +329,20 @@ class JSBinopReduction final {
                             frame_state->InputAt(3), frame_state->InputAt(4));
   }
 
-  Node* ConvertPrimitiveToNumber(Node* node) {
-    return lowering_->ConvertPrimitiveToNumber(node);
+  Node* ConvertPlainPrimitiveToNumber(Node* node) {
+    DCHECK(NodeProperties::GetBounds(node).upper->Is(Type::PlainPrimitive()));
+    // Avoid inserting too many eager ToNumber() operations.
+    Reduction const reduction = lowering_->ReduceJSToNumberInput(node);
+    if (reduction.Changed()) return reduction.replacement();
+    // TODO(jarin) Use PlainPrimitiveToNumber once we have it.
+    return graph()->NewNode(
+        javascript()->ToNumber(), node, jsgraph()->NoContextConstant(),
+        jsgraph()->EmptyFrameState(), graph()->start(), graph()->start());
   }
 
   Node* ConvertToNumber(Node* node, Node* frame_state) {
     if (NodeProperties::GetBounds(node).upper->Is(Type::PlainPrimitive())) {
-      return ConvertPrimitiveToNumber(node);
+      return ConvertPlainPrimitiveToNumber(node);
     } else {
       Node* const n =
           graph()->NewNode(javascript()->ToNumber(), node, context(),
@@ -470,21 +473,7 @@ Reduction JSTypedLowering::ReduceJSComparison(Node* node) {
     }
     return r.ChangeToPureOperator(stringOp);
   }
-  if (r.IsStrong() && !r.BothInputsAre(Type::Number())) {
-    return NoChange();
-  }
-#if 0
-  // TODO(turbofan): General ToNumber disabled for now because:
-  //   a) The inserted ToNumber operation screws up observability of valueOf.
-  //   b) Deoptimization at ToNumber doesn't have corresponding bailout id.
-  Type* maybe_string = Type::Union(Type::String(), Type::Receiver(), zone());
-  if (r.OneInputCannotBe(maybe_string)) {
-    // If one input cannot be a string, then emit a number comparison.
-    ...
-  }
-#endif
-  if (r.BothInputsAre(Type::PlainPrimitive()) &&
-      r.OneInputCannotBe(Type::StringOrReceiver())) {
+  if (r.OneInputCannotBe(Type::StringOrReceiver())) {
     const Operator* less_than;
     const Operator* less_than_or_equal;
     if (r.BothInputsAre(Type::Unsigned32())) {
@@ -495,7 +484,11 @@ Reduction JSTypedLowering::ReduceJSComparison(Node* node) {
       less_than_or_equal = machine()->Int32LessThanOrEqual();
     } else {
       // TODO(turbofan): mixed signed/unsigned int32 comparisons.
-      r.ConvertPrimitiveInputsToNumber();
+      if (r.IsStrong() && !r.BothInputsAre(Type::Number())) {
+        return NoChange();
+      }
+      Node* frame_state = NodeProperties::GetFrameStateInput(node, 1);
+      r.ConvertInputsToNumber(frame_state);
       less_than = simplified()->NumberLessThan();
       less_than_or_equal = simplified()->NumberLessThanOrEqual();
     }
@@ -1192,18 +1185,6 @@ Reduction JSTypedLowering::Reduce(Node* node) {
 }
 
 
-Node* JSTypedLowering::ConvertPrimitiveToNumber(Node* input) {
-  DCHECK(NodeProperties::GetBounds(input).upper->Is(Type::PlainPrimitive()));
-  // Avoid inserting too many eager ToNumber() operations.
-  Reduction const reduction = ReduceJSToNumberInput(input);
-  if (reduction.Changed()) return reduction.replacement();
-  // TODO(jarin) Use PlainPrimitiveToNumber once we have it.
-  return graph()->NewNode(
-      javascript()->ToNumber(), input, jsgraph()->NoContextConstant(),
-      jsgraph()->EmptyFrameState(), graph()->start(), graph()->start());
-}
-
-
 Node* JSTypedLowering::Word32Shl(Node* const lhs, int32_t const rhs) {
   if (rhs == 0) return lhs;
   return graph()->NewNode(machine()->Word32Shl(), lhs,

src/compiler/js-typed-lowering.h

@@ -64,8 +64,6 @@ class JSTypedLowering final : public Reducer {
   Reduction ReduceUI32Shift(Node* node, Signedness left_signedness,
                             const Operator* shift_op);
 
-  Node* ConvertPrimitiveToNumber(Node* input);
-
   Node* Word32Shl(Node* const lhs, int32_t const rhs);
 
   Factory* factory() const;

src/compiler/operator-properties.cc

@@ -40,13 +40,9 @@ int OperatorProperties::GetFrameStateInputCount(const Operator* op) {
 
     // Compare operations
     case IrOpcode::kJSEqual:
-    case IrOpcode::kJSGreaterThan:
-    case IrOpcode::kJSGreaterThanOrEqual:
+    case IrOpcode::kJSNotEqual:
     case IrOpcode::kJSHasProperty:
     case IrOpcode::kJSInstanceOf:
-    case IrOpcode::kJSLessThan:
-    case IrOpcode::kJSLessThanOrEqual:
-    case IrOpcode::kJSNotEqual:
 
     // Object operations
     case IrOpcode::kJSCreateLiteralArray:
@@ -93,6 +89,15 @@ int OperatorProperties::GetFrameStateInputCount(const Operator* op) {
     case IrOpcode::kJSSubtract:
       return 2;
 
+    // Compare operators that can deopt in the middle the operation (e.g.,
+    // as a result of lazy deopt in ToNumber conversion) need a second frame
+    // state so that we can resume before the operation.
+    case IrOpcode::kJSGreaterThan:
+    case IrOpcode::kJSGreaterThanOrEqual:
+    case IrOpcode::kJSLessThan:
+    case IrOpcode::kJSLessThanOrEqual:
+      return 2;
+
     default:
       return 0;
   }

test/mjsunit/compiler/deopt-tonumber-compare.js

+// Copyright 2015 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 m = (function() {
+  "use asm";
+  function f(x) {
+    return x < 0;
+  }
+  function g(x) {
+    return 0 < x;
+  }
+  return { f: f, g: g };
+})();
+var f = m.f;
+var g = m.g;
+
+var counter = 0;
+
+function deopt(f) {
+  return {
+    toString : function() {
+      %DeoptimizeFunction(f);
+      counter++;
+      return "2";
+    }
+  };
+}
+
+assertEquals(false, f(deopt(f)));
+assertEquals(1, counter);
+
+assertEquals(true, g(deopt(g)));
+assertEquals(2, counter);
+
+%OptimizeFunctionOnNextCall(f);
+assertEquals(false, f(deopt(f)));
+assertEquals(3, counter);
+
+%OptimizeFunctionOnNextCall(g);
+assertEquals(true, g(deopt(g)));
+assertEquals(4, counter);

test/unittests/compiler/js-operator-unittest.cc

@@ -186,10 +186,10 @@ const SharedOperatorWithLanguageMode kSharedOperatorsWithLanguageMode[] = {
         control_input_count, value_output_count, effect_output_count,        \
         control_output_count                                                 \
   }
-    SHARED(LessThan, Operator::kNoProperties, 2, 1, 1, 1, 1, 1, 2),
-    SHARED(GreaterThan, Operator::kNoProperties, 2, 1, 1, 1, 1, 1, 2),
-    SHARED(LessThanOrEqual, Operator::kNoProperties, 2, 1, 1, 1, 1, 1, 2),
-    SHARED(GreaterThanOrEqual, Operator::kNoProperties, 2, 1, 1, 1, 1, 1, 2),
+    SHARED(LessThan, Operator::kNoProperties, 2, 2, 1, 1, 1, 1, 2),
+    SHARED(GreaterThan, Operator::kNoProperties, 2, 2, 1, 1, 1, 1, 2),
+    SHARED(LessThanOrEqual, Operator::kNoProperties, 2, 2, 1, 1, 1, 1, 2),
+    SHARED(GreaterThanOrEqual, Operator::kNoProperties, 2, 2, 1, 1, 1, 1, 2),
     SHARED(BitwiseOr, Operator::kNoProperties, 2, 2, 1, 1, 1, 1, 2),
     SHARED(BitwiseXor, Operator::kNoProperties, 2, 2, 1, 1, 1, 1, 2),
     SHARED(BitwiseAnd, Operator::kNoProperties, 2, 2, 1, 1, 1, 1, 2),