[turbofan] Allow deoptimization for JSToNumber operator.

R=bmeurer@chromium.org

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

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

diff --git a/src/arm/full-codegen-arm.cc b/src/arm/full-codegen-arm.cc
index 3dc54203b17c5de74f7a6ac5a4b2d58fb31c7ee6..7a0af0686eb824691d0ffab3e5400f0efaa2d8f5 100644 (file)
--- a/src/arm/full-codegen-arm.cc
+++ b/src/arm/full-codegen-arm.cc
@@ -4676,6 +4676,7 @@ void FullCodeGenerator::VisitCountOperation(CountOperation* expr) {
   }
   ToNumberStub convert_stub(isolate());
   __ CallStub(&convert_stub);
+  PrepareForBailoutForId(expr->ToNumberId(), TOS_REG);
 
   // Save result for postfix expressions.
   if (expr->is_postfix()) {

diff --git a/src/arm64/full-codegen-arm64.cc b/src/arm64/full-codegen-arm64.cc
index 0d3d34b6950e111392d2dc7276195f5f84518acb..3a46a93f5fcb6bb8566d335473f7a5121a48f1e3 100644 (file)
--- a/src/arm64/full-codegen-arm64.cc
+++ b/src/arm64/full-codegen-arm64.cc
@@ -4360,6 +4360,7 @@ void FullCodeGenerator::VisitCountOperation(CountOperation* expr) {
   }
   ToNumberStub convert_stub(isolate());
   __ CallStub(&convert_stub);
+  PrepareForBailoutForId(expr->ToNumberId(), TOS_REG);
 
   // Save result for postfix expressions.
   if (expr->is_postfix()) {

diff --git a/src/ast.h b/src/ast.h
index a550c6cfd23e1f67ef2423b4901c48367b29868c..d562e91d981042edc30479fe125fe8fb8fda5bfd 100644 (file)
--- a/src/ast.h
+++ b/src/ast.h
@@ -2170,13 +2170,14 @@ class CountOperation FINAL : public Expression {
   }
   void set_type(Type* type) { type_ = type; }
 
-  static int num_ids() { return parent_num_ids() + 3; }
+  static int num_ids() { return parent_num_ids() + 4; }
   BailoutId AssignmentId() const { return BailoutId(local_id(0)); }
+  BailoutId ToNumberId() const { return BailoutId(local_id(1)); }
   TypeFeedbackId CountBinOpFeedbackId() const {
-    return TypeFeedbackId(local_id(1));
+    return TypeFeedbackId(local_id(2));
   }
   TypeFeedbackId CountStoreFeedbackId() const {
-    return TypeFeedbackId(local_id(2));
+    return TypeFeedbackId(local_id(3));
   }
 
  protected:

diff --git a/src/compiler/ast-graph-builder.cc b/src/compiler/ast-graph-builder.cc
index 5c91b95153d6448fda9816f1340546aac6952186..c42d26c0629b631f38ba4d7b75bc47a964cc9975 100644 (file)
--- a/src/compiler/ast-graph-builder.cc
+++ b/src/compiler/ast-graph-builder.cc
@@ -1488,6 +1488,8 @@ void AstGraphBuilder::VisitCountOperation(CountOperation* expr) {
 
   // Convert old value into a number.
   old_value = NewNode(javascript()->ToNumber(), old_value);
+  PrepareFrameState(old_value, expr->ToNumberId(),
+                    OutputFrameStateCombine::Push());
 
   // Save result for postfix expressions at correct stack depth.
   if (is_postfix) environment()->Poke(stack_depth, old_value);

diff --git a/src/compiler/change-lowering.cc b/src/compiler/change-lowering.cc
index 7ddc751ab53199af0e8fc501b6f7eb12215f0d3c..2b369d97cb0f040142b74cfeebada497636473bd 100644 (file)
--- a/src/compiler/change-lowering.cc
+++ b/src/compiler/change-lowering.cc
@@ -227,7 +227,12 @@ Reduction ChangeLowering::ChangeTaggedToFloat64(Node* value, Node* control) {
     d1.Chain(control);
 
     Node* number =
-        graph()->NewNode(value->op(), object, context, effect, d1.if_true);
+        OperatorProperties::HasFrameStateInput(value->op())
+            ? graph()->NewNode(value->op(), object, context,
+                               NodeProperties::GetFrameStateInput(value),
+                               effect, d1.if_true)
+            : graph()->NewNode(value->op(), object, context, effect,
+                               d1.if_true);
     Diamond d2(graph(), common(), TestNotSmi(number));
     d2.Nest(d1, true);
     Node* phi2 = d2.Phi(kMachFloat64, LoadHeapNumberValue(number, d2.if_true),

diff --git a/src/compiler/js-graph.cc b/src/compiler/js-graph.cc
index 7759ba14418c29c26aaa165d470958ba1abc0f93..68728a1764b78206373c02fc6ac8258f0f1519a0 100644 (file)
--- a/src/compiler/js-graph.cc
+++ b/src/compiler/js-graph.cc
@@ -191,6 +191,19 @@ Node* JSGraph::Float64Constant(double value) {
 }
 
 
+Node* JSGraph::EmptyFrameState() {
+  if (!empty_frame_state_.is_set()) {
+    Node* values = graph()->NewNode(common()->StateValues(0));
+    Node* state_node = graph()->NewNode(
+        common()->FrameState(JS_FRAME, BailoutId(0),
+                             OutputFrameStateCombine::Ignore()),
+        values, values, values, NoContextConstant(), UndefinedConstant());
+    empty_frame_state_.set(state_node);
+  }
+  return empty_frame_state_.get();
+}
+
+
 Node* JSGraph::ExternalConstant(ExternalReference reference) {
   Node** loc = cache_.FindExternalConstant(reference);
   if (*loc == NULL) {

diff --git a/src/compiler/js-graph.h b/src/compiler/js-graph.h
index 040a745e3cfd553a7796f36846197084c46db7f2..2f5ebd9ce9cd66ee6569d6a2ca3e45cb76ba514f 100644 (file)
--- a/src/compiler/js-graph.h
+++ b/src/compiler/js-graph.h
@@ -109,6 +109,10 @@ class JSGraph : public ZoneObject {
   // stubs and runtime functions that do not require a context.
   Node* NoContextConstant() { return ZeroConstant(); }
 
+  // Creates an empty frame states for cases where we know that a function
+  // cannot deopt.
+  Node* EmptyFrameState();
+
   JSOperatorBuilder* javascript() { return javascript_; }
   CommonOperatorBuilder* common() { return common_; }
   MachineOperatorBuilder* machine() { return machine_; }
@@ -135,6 +139,7 @@ class JSGraph : public ZoneObject {
   SetOncePointer<Node> zero_constant_;
   SetOncePointer<Node> one_constant_;
   SetOncePointer<Node> nan_constant_;
+  SetOncePointer<Node> empty_frame_state_;
 
   CommonNodeCache cache_;
 

diff --git a/src/compiler/js-typed-lowering.cc b/src/compiler/js-typed-lowering.cc
index 761837576b3b40401063da876012e38ecf5eb18c..56ffc5f715c27ed97c002bf5f06558beb405ed7a 100644 (file)
--- a/src/compiler/js-typed-lowering.cc
+++ b/src/compiler/js-typed-lowering.cc
@@ -198,8 +198,16 @@ class JSBinopReduction FINAL {
     if (NodeProperties::GetBounds(node).upper->Is(Type::PlainPrimitive())) {
       return lowering_->ConvertToNumber(node);
     }
-    Node* n = graph()->NewNode(javascript()->ToNumber(), node, context(),
-                               effect(), control());
+    // TODO(jarin) This ToNumber conversion can deoptimize, but we do not really
+    // have a frame state to deoptimize to. Either we provide such a frame state
+    // or we exclude the values that could lead to deoptimization (e.g., by
+    // triggering eager deopt if the value is not plain).
+    Node* const n = FLAG_turbo_deoptimization
+                        ? graph()->NewNode(
+                              javascript()->ToNumber(), node, context(),
+                              jsgraph()->EmptyFrameState(), effect(), control())
+                        : graph()->NewNode(javascript()->ToNumber(), node,
+                                           context(), effect(), control());
     update_effect(n);
     return n;
   }
@@ -624,15 +632,20 @@ Reduction JSTypedLowering::ReduceJSToNumber(Node* node) {
     }
     // Remember this conversion.
     InsertConversion(node);
-    if (node->InputAt(1) != jsgraph()->NoContextConstant() ||
-        node->InputAt(2) != graph()->start() ||
-        node->InputAt(3) != graph()->start()) {
+    if (NodeProperties::GetContextInput(node) !=
+            jsgraph()->NoContextConstant() ||
+        NodeProperties::GetEffectInput(node) != graph()->start() ||
+        NodeProperties::GetControlInput(node) != graph()->start()) {
       // JSToNumber(x:plain-primitive,context,effect,control)
       //   => JSToNumber(x,no-context,start,start)
       RelaxEffects(node);
-      node->ReplaceInput(1, jsgraph()->NoContextConstant());
-      node->ReplaceInput(2, graph()->start());
-      node->ReplaceInput(3, graph()->start());
+      NodeProperties::ReplaceContextInput(node, jsgraph()->NoContextConstant());
+      NodeProperties::ReplaceControlInput(node, graph()->start());
+      NodeProperties::ReplaceEffectInput(node, graph()->start());
+      if (OperatorProperties::HasFrameStateInput(node->op())) {
+        NodeProperties::ReplaceFrameStateInput(node,
+                                               jsgraph()->EmptyFrameState());
+      }
       return Changed(node);
     }
   }
@@ -752,8 +765,15 @@ Reduction JSTypedLowering::ReduceJSStoreProperty(Node* node) {
         if (number_reduction.Changed()) {
           value = number_reduction.replacement();
         } else {
-          value = effect = graph()->NewNode(javascript()->ToNumber(), value,
-                                            context, effect, control);
+          if (OperatorProperties::HasFrameStateInput(
+                  javascript()->ToNumber())) {
+            value = effect =
+                graph()->NewNode(javascript()->ToNumber(), value, context,
+                                 jsgraph()->EmptyFrameState(), effect, control);
+          } else {
+            value = effect = graph()->NewNode(javascript()->ToNumber(), value,
+                                              context, effect, control);
+          }
         }
       }
       // For integer-typed arrays, convert to the integer type.
@@ -785,8 +805,8 @@ Reduction JSTypedLowering::ReduceJSStoreProperty(Node* node) {
       node->ReplaceInput(2, length);
       node->ReplaceInput(3, value);
       node->ReplaceInput(4, effect);
-      DCHECK_EQ(control, node->InputAt(5));
-      DCHECK_EQ(6, node->InputCount());
+      node->ReplaceInput(5, control);
+      node->TrimInputCount(6);
       return Changed(node);
     }
   }
@@ -932,9 +952,16 @@ Node* JSTypedLowering::ConvertToNumber(Node* input) {
   // Avoid inserting too many eager ToNumber() operations.
   Reduction const reduction = ReduceJSToNumberInput(input);
   if (reduction.Changed()) return reduction.replacement();
-  Node* const conversion = graph()->NewNode(javascript()->ToNumber(), input,
-                                            jsgraph()->NoContextConstant(),
-                                            graph()->start(), graph()->start());
+  // TODO(jarin) Use PlainPrimitiveToNumber once we have it.
+  Node* const conversion =
+      FLAG_turbo_deoptimization
+          ? graph()->NewNode(javascript()->ToNumber(), input,
+                             jsgraph()->NoContextConstant(),
+                             jsgraph()->EmptyFrameState(), graph()->start(),
+                             graph()->start())
+          : graph()->NewNode(javascript()->ToNumber(), input,
+                             jsgraph()->NoContextConstant(), graph()->start(),
+                             graph()->start());
   InsertConversion(conversion);
   return conversion;
 }

diff --git a/src/compiler/node-properties-inl.h b/src/compiler/node-properties-inl.h
index 0d296141e1486d0752b5dcb6dee6820b506595c4..e0f94de5d28ec7f2d802e00ea5508f00278c3dcc 100644 (file)
--- a/src/compiler/node-properties-inl.h
+++ b/src/compiler/node-properties-inl.h
@@ -144,6 +144,10 @@ inline bool NodeProperties::IsControl(Node* node) {
 // -----------------------------------------------------------------------------
 // Miscellaneous mutators.
 
+inline void NodeProperties::ReplaceContextInput(Node* node, Node* context) {
+  node->ReplaceInput(FirstContextIndex(node), context);
+}
+
 inline void NodeProperties::ReplaceControlInput(Node* node, Node* control) {
   node->ReplaceInput(FirstControlIndex(node), control);
 }

diff --git a/src/compiler/node-properties.h b/src/compiler/node-properties.h
index 025be7857ce658bf9493923c5e85f574dd7a2b4b..13f31d11598330e80b980036dd39d2e52628fb10 100644 (file)
--- a/src/compiler/node-properties.h
+++ b/src/compiler/node-properties.h
@@ -32,6 +32,7 @@ class NodeProperties {
 
   static inline bool IsControl(Node* node);
 
+  static inline void ReplaceContextInput(Node* node, Node* context);
   static inline void ReplaceControlInput(Node* node, Node* control);
   static inline void ReplaceEffectInput(Node* node, Node* effect,
                                         int index = 0);

diff --git a/src/compiler/operator-properties.cc b/src/compiler/operator-properties.cc
index abfc5fd99c4a6c4b6af55be025ccaa99e7519bb8..09d964b3c5d3ecb80525fc4a60a22ea12a784671 100644 (file)
--- a/src/compiler/operator-properties.cc
+++ b/src/compiler/operator-properties.cc
@@ -70,6 +70,7 @@ bool OperatorProperties::HasFrameStateInput(const Operator* op) {
 
     // Conversions
     case IrOpcode::kJSToObject:
+    case IrOpcode::kJSToNumber:
 
     // Other
     case IrOpcode::kJSDeleteProperty:

diff --git a/src/ia32/full-codegen-ia32.cc b/src/ia32/full-codegen-ia32.cc
index 1ba409571534ee945cf8a3d16e972c61c5bcb1c4..a4a0d7c6fea68fb2922b393b4792ddc96c063ade 100644 (file)
--- a/src/ia32/full-codegen-ia32.cc
+++ b/src/ia32/full-codegen-ia32.cc
@@ -4621,6 +4621,7 @@ void FullCodeGenerator::VisitCountOperation(CountOperation* expr) {
   }
   ToNumberStub convert_stub(isolate());
   __ CallStub(&convert_stub);
+  PrepareForBailoutForId(expr->ToNumberId(), TOS_REG);
 
   // Save result for postfix expressions.
   if (expr->is_postfix()) {

diff --git a/src/mips/full-codegen-mips.cc b/src/mips/full-codegen-mips.cc
index c1a829148048addf289e22c3f97b4f3942154f4d..eccb60437fef04950e95a076cbfd9cb0d143c726 100644 (file)
--- a/src/mips/full-codegen-mips.cc
+++ b/src/mips/full-codegen-mips.cc
@@ -4681,6 +4681,7 @@ void FullCodeGenerator::VisitCountOperation(CountOperation* expr) {
   }
   ToNumberStub convert_stub(isolate());
   __ CallStub(&convert_stub);
+  PrepareForBailoutForId(expr->ToNumberId(), TOS_REG);
 
   // Save result for postfix expressions.
   if (expr->is_postfix()) {

diff --git a/src/mips64/full-codegen-mips64.cc b/src/mips64/full-codegen-mips64.cc
index 9d4ed095397ab11c211acedd5f4af8152b9bf6a7..703a248189ea2692a141fbc7ec064269b0c240d6 100644 (file)
--- a/src/mips64/full-codegen-mips64.cc
+++ b/src/mips64/full-codegen-mips64.cc
@@ -4681,6 +4681,7 @@ void FullCodeGenerator::VisitCountOperation(CountOperation* expr) {
   }
   ToNumberStub convert_stub(isolate());
   __ CallStub(&convert_stub);
+  PrepareForBailoutForId(expr->ToNumberId(), TOS_REG);
 
   // Save result for postfix expressions.
   if (expr->is_postfix()) {

diff --git a/src/ppc/full-codegen-ppc.cc b/src/ppc/full-codegen-ppc.cc
index 1bb4f54f4ac07fca102fea4e2bf3668eaace403d..1a02f6958d9068a5a601b587261d4ef5154c006e 100644 (file)
--- a/src/ppc/full-codegen-ppc.cc
+++ b/src/ppc/full-codegen-ppc.cc
@@ -4730,6 +4730,7 @@ void FullCodeGenerator::VisitCountOperation(CountOperation* expr) {
   }
   ToNumberStub convert_stub(isolate());
   __ CallStub(&convert_stub);
+  PrepareForBailoutForId(expr->ToNumberId(), TOS_REG);
 
   // Save result for postfix expressions.
   if (expr->is_postfix()) {

diff --git a/src/x64/full-codegen-x64.cc b/src/x64/full-codegen-x64.cc
index 24747ee9e85b11d7e3fed1b50e70db0d2073e9b2..2740831b41bab844766d56ddd94ac453e5b27ce2 100644 (file)
--- a/src/x64/full-codegen-x64.cc
+++ b/src/x64/full-codegen-x64.cc
@@ -4636,6 +4636,7 @@ void FullCodeGenerator::VisitCountOperation(CountOperation* expr) {
 
   ToNumberStub convert_stub(isolate());
   __ CallStub(&convert_stub);
+  PrepareForBailoutForId(expr->ToNumberId(), TOS_REG);
 
   // Save result for postfix expressions.
   if (expr->is_postfix()) {

diff --git a/src/x87/full-codegen-x87.cc b/src/x87/full-codegen-x87.cc
index 0f292cc49beec7d05ae333aaa198f9cb984cbeb5..1f46fdc039f0d8df7cfcd747109a6f483b8bed04 100644 (file)
--- a/src/x87/full-codegen-x87.cc
+++ b/src/x87/full-codegen-x87.cc
@@ -4607,6 +4607,7 @@ void FullCodeGenerator::VisitCountOperation(CountOperation* expr) {
   }
   ToNumberStub convert_stub(isolate());
   __ CallStub(&convert_stub);
+  PrepareForBailoutForId(expr->ToNumberId(), TOS_REG);
 
   // Save result for postfix expressions.
   if (expr->is_postfix()) {

diff --git a/test/cctest/compiler/test-js-typed-lowering.cc b/test/cctest/compiler/test-js-typed-lowering.cc
index 3023837f4c8286ecc768b22a5c0d747fc92c11bc..e23ad0cd169d091297556920d918e7378e70cc12 100644 (file)
--- a/test/cctest/compiler/test-js-typed-lowering.cc
+++ b/test/cctest/compiler/test-js-typed-lowering.cc
@@ -123,8 +123,13 @@ class JSTypedLoweringTester : public HandleAndZoneScope {
 
   Node* UseForEffect(Node* node) {
     // TODO(titzer): use EffectPhi after fixing EffectCount
-    return graph.NewNode(javascript.ToNumber(), node, context(), node,
-                         control());
+    if (OperatorProperties::HasFrameStateInput(javascript.ToNumber())) {
+      return graph.NewNode(javascript.ToNumber(), node, context(),
+                           EmptyFrameState(context()), node, control());
+    } else {
+      return graph.NewNode(javascript.ToNumber(), node, context(), node,
+                           control());
+    }
   }
 
   void CheckEffectInput(Node* effect, Node* use) {
@@ -737,12 +742,25 @@ TEST(RemoveToNumberEffects) {
 
     switch (i) {
       case 0:
+        // TODO(jarin) Replace with a query of FLAG_turbo_deoptimization.
+        if (OperatorProperties::HasFrameStateInput(R.javascript.ToNumber())) {
+          effect_use = R.graph.NewNode(R.javascript.ToNumber(), p0, R.context(),
+                                       frame_state, ton, R.start());
+        } else {
         effect_use = R.graph.NewNode(R.javascript.ToNumber(), p0, R.context(),
                                      ton, R.start());
+        }
         break;
       case 1:
-        effect_use = R.graph.NewNode(R.javascript.ToNumber(), ton, R.context(),
-                                     ton, R.start());
+        // TODO(jarin) Replace with a query of FLAG_turbo_deoptimization.
+        if (OperatorProperties::HasFrameStateInput(R.javascript.ToNumber())) {
+          effect_use =
+              R.graph.NewNode(R.javascript.ToNumber(), ton, R.context(),
+                              frame_state, ton, R.start());
+        } else {
+          effect_use = R.graph.NewNode(R.javascript.ToNumber(), ton,
+                                       R.context(), ton, R.start());
+        }
         break;
       case 2:
         effect_use = R.graph.NewNode(R.common.EffectPhi(1), ton, R.start());

diff --git a/test/mjsunit/compiler/regress-445907.js b/test/mjsunit/compiler/regress-445907.js
new file mode 100644 (file)
index 0000000..c820753
--- /dev/null
+++ b/test/mjsunit/compiler/regress-445907.js
@@ -0,0 +1,14 @@
+// Copyright 2014 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: --turbo-deoptimization
+
+v = [];
+v.length = (1 << 30);
+
+function f() {
+  v++;
+}
+
+assertThrows(f);