[turbofan] Fix throwing conversion inserted by JSTypedLowering.

This fixes the graph wiring of implicit JSToNumber nodes inserted by
JSTypedLowering, to be correctly hooked into a surrounding exceptional
continuation.

R=bmeurer@chromium.org
TEST=mjsunit/compiler/try-binop,test262

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

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

src/compiler/js-typed-lowering.cc

@@ -107,17 +107,25 @@ class JSBinopReduction final {
     // already converted values from full code. This way we are sure that we
     // will not re-do any of the side effects.
 
-    Node* left_input =
-        left_type()->Is(Type::PlainPrimitive())
-            ? ConvertPlainPrimitiveToNumber(left())
-            : ConvertToNumber(left(),
-                              CreateFrameStateForLeftInput(frame_state));
-
-    Node* right_input =
-        right_type()->Is(Type::PlainPrimitive())
-            ? ConvertPlainPrimitiveToNumber(right())
-            : ConvertToNumber(right(), CreateFrameStateForRightInput(
+    Node* left_input = nullptr;
+    Node* right_input = nullptr;
+    bool left_is_primitive = left_type()->Is(Type::PlainPrimitive());
+    bool right_is_primitive = right_type()->Is(Type::PlainPrimitive());
+    bool handles_exception = NodeProperties::IsExceptionalCall(node_);
+
+    if (!left_is_primitive && !right_is_primitive && handles_exception) {
+      ConvertBothInputsToNumber(&left_input, &right_input, frame_state);
+    } else {
+      left_input = left_is_primitive
+                       ? ConvertPlainPrimitiveToNumber(left())
+                       : ConvertSingleInputToNumber(
+                             left(), CreateFrameStateForLeftInput(frame_state));
+      right_input = right_is_primitive
+                        ? ConvertPlainPrimitiveToNumber(right())
+                        : ConvertSingleInputToNumber(
+                              right(), CreateFrameStateForRightInput(
                                            frame_state, left_input));
+    }
 
     node_->ReplaceInput(0, left_input);
     node_->ReplaceInput(1, right_input);
@@ -226,6 +234,7 @@ class JSBinopReduction final {
   JSGraph* jsgraph() { return lowering_->jsgraph(); }
   JSOperatorBuilder* javascript() { return lowering_->javascript(); }
   MachineOperatorBuilder* machine() { return lowering_->machine(); }
+  CommonOperatorBuilder* common() { return jsgraph()->common(); }
   Zone* zone() const { return graph()->zone(); }
 
  private:
@@ -324,16 +333,61 @@ class JSBinopReduction final {
         jsgraph()->EmptyFrameState(), graph()->start(), graph()->start());
   }
 
-  Node* ConvertToNumber(Node* node, Node* frame_state) {
-    if (NodeProperties::GetBounds(node).upper->Is(Type::PlainPrimitive())) {
-      return ConvertPlainPrimitiveToNumber(node);
-    } else {
-      Node* const n =
-          graph()->NewNode(javascript()->ToNumber(), node, context(),
-                           frame_state, effect(), control());
-      update_effect(n);
-      return n;
+  Node* ConvertSingleInputToNumber(Node* node, Node* frame_state) {
+    DCHECK(!NodeProperties::GetBounds(node).upper->Is(Type::PlainPrimitive()));
+    Node* const n = graph()->NewNode(javascript()->ToNumber(), node, context(),
+                                     frame_state, effect(), control());
+    NodeProperties::ReplaceUses(node_, node_, node_, n, n);
+    update_effect(n);
+    return n;
+  }
+
+  void ConvertBothInputsToNumber(Node** left_result, Node** right_result,
+                                 Node* frame_state) {
+    Node* projections[2];
+
+    // Find {IfSuccess} and {IfException} continuations of the operation.
+    NodeProperties::CollectControlProjections(node_, projections, 2);
+    IfExceptionHint hint = OpParameter<IfExceptionHint>(projections[1]);
+    Node* if_exception = projections[1];
+    Node* if_success = projections[0];
+
+    // Insert two ToNumber() operations that both potentially throw.
+    Node* left_state = CreateFrameStateForLeftInput(frame_state);
+    Node* left_conv =
+        graph()->NewNode(javascript()->ToNumber(), left(), context(),
+                         left_state, effect(), control());
+    Node* left_success = graph()->NewNode(common()->IfSuccess(), left_conv);
+    Node* right_state = CreateFrameStateForRightInput(frame_state, left_conv);
+    Node* right_conv =
+        graph()->NewNode(javascript()->ToNumber(), right(), context(),
+                         right_state, left_conv, left_success);
+    Node* left_exception =
+        graph()->NewNode(common()->IfException(hint), left_conv, left_conv);
+    Node* right_exception =
+        graph()->NewNode(common()->IfException(hint), right_conv, right_conv);
+    NodeProperties::ReplaceControlInput(if_success, right_conv);
+    update_effect(right_conv);
+
+    // Wire conversions to existing {IfException} continuation.
+    Node* exception_merge = if_exception;
+    Node* exception_value =
+        graph()->NewNode(common()->Phi(kMachAnyTagged, 2), left_exception,
+                         right_exception, exception_merge);
+    Node* exception_effect =
+        graph()->NewNode(common()->EffectPhi(2), left_exception,
+                         right_exception, exception_merge);
+    for (Edge edge : exception_merge->use_edges()) {
+      if (NodeProperties::IsEffectEdge(edge)) edge.UpdateTo(exception_effect);
+      if (NodeProperties::IsValueEdge(edge)) edge.UpdateTo(exception_value);
     }
+    NodeProperties::RemoveBounds(exception_merge);
+    exception_merge->ReplaceInput(0, left_exception);
+    exception_merge->ReplaceInput(1, right_exception);
+    exception_merge->set_op(common()->Merge(2));
+
+    *left_result = left_conv;
+    *right_result = right_conv;
   }
 
   Node* ConvertToUI32(Node* node, Signedness signedness) {

src/compiler/node-properties.cc

@@ -120,8 +120,9 @@ bool NodeProperties::IsControlEdge(Edge edge) {
 
 // static
 bool NodeProperties::IsExceptionalCall(Node* node) {
-  for (Node* const use : node->uses()) {
-    if (use->opcode() == IrOpcode::kIfException) return true;
+  for (Edge const edge : node->use_edges()) {
+    if (!NodeProperties::IsControlEdge(edge)) continue;
+    if (edge.from()->opcode() == IrOpcode::kIfException) return true;
   }
   return false;
 }
@@ -228,11 +229,11 @@ void NodeProperties::CollectControlProjections(Node* node, Node** projections,
         index = 1;
         break;
       case IrOpcode::kIfSuccess:
-        DCHECK_EQ(IrOpcode::kCall, node->opcode());
+        DCHECK(!node->op()->HasProperty(Operator::kNoThrow));
         index = 0;
         break;
       case IrOpcode::kIfException:
-        DCHECK_EQ(IrOpcode::kCall, node->opcode());
+        DCHECK(!node->op()->HasProperty(Operator::kNoThrow));
         index = 1;
         break;
       case IrOpcode::kIfValue:

test/mjsunit/compiler/try-binop.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 boom = { valueOf: function() { throw "boom" } };
+
+function mult_left_plain(x) {
+  try {
+    return 2 * x;
+  } catch (e) {
+    return e;
+  }
+}
+
+%OptimizeFunctionOnNextCall(mult_left_plain);
+assertEquals("boom", mult_left_plain(boom));
+assertEquals(46, mult_left_plain(23));
+
+function mult_right_plain(x) {
+  try {
+    return x * 3;
+  } catch (e) {
+    return e;
+  }
+}
+
+%OptimizeFunctionOnNextCall(mult_right_plain);
+assertEquals("boom", mult_right_plain(boom));
+assertEquals(69, mult_right_plain(23));
+
+function mult_none_plain(x,y) {
+  try {
+    return x * y;
+  } catch (e) {
+    return e;
+  }
+}
+
+%OptimizeFunctionOnNextCall(mult_none_plain);
+assertEquals("boom", mult_none_plain(boom, boom));
+assertEquals("boom", mult_none_plain(boom, 2));
+assertEquals("boom", mult_none_plain(2, boom));
+assertEquals(966, mult_none_plain(23, 42));

test/mjsunit/mjsunit.status

@@ -62,11 +62,6 @@
   # from the deoptimizer to do that.
   'arguments-indirect': [PASS, NO_VARIANTS],
 
-  # TODO(mstarzinger): The implicit JSToNumber and JSToString nodes inserted by
-  # JSTypedLowering are not properly connected to a surrounding exceptional
-  # continuation yet, hence exception are thrown out of the function body.
-  'regress/regress-1327557': [PASS, NO_VARIANTS],
-
   # TODO(verwaest): Some tests are over-restrictive about object layout.
   'array-constructor-feedback': [PASS, NO_VARIANTS],
   'array-feedback': [PASS, NO_VARIANTS],

test/test262-es6/test262-es6.status

@@ -36,38 +36,6 @@
   # Unicode canonicalization is not available with i18n turned off.
   'built-ins/String/prototype/localeCompare/15.5.4.9_CE': [['no_i18n', SKIP]],
 
-  # TODO(mstarzinger): The implicit JSToNumber and JSToString nodes inserted by
-  # JSTypedLowering are not properly connected to a surrounding exceptional
-  # continuation yet, hence exception are thrown out of the function body.
-  'built-ins/String/fromCharCode/S9.7_A3.1_T4': [PASS, NO_VARIANTS],
-  'language/expressions/bitwise-and/S11.10.1_A2.2_T1': [PASS, NO_VARIANTS],
-  'language/expressions/bitwise-and/S11.10.1_A2.3_T1': [PASS, NO_VARIANTS],
-  'language/expressions/bitwise-not/S11.4.8_A2.2_T1': [PASS, NO_VARIANTS],
-  'language/expressions/bitwise-not/S9.5_A3.1_T4': [PASS, NO_VARIANTS],
-  'language/expressions/bitwise-or/S11.10.3_A2.2_T1': [PASS, NO_VARIANTS],
-  'language/expressions/bitwise-or/S11.10.3_A2.3_T1': [PASS, NO_VARIANTS],
-  'language/expressions/bitwise-xor/S11.10.2_A2.2_T1': [PASS, NO_VARIANTS],
-  'language/expressions/bitwise-xor/S11.10.2_A2.3_T1': [PASS, NO_VARIANTS],
-  'language/expressions/division/S11.5.2_A2.2_T1': [PASS, NO_VARIANTS],
-  'language/expressions/division/S11.5.2_A2.3_T1': [PASS, NO_VARIANTS],
-  'language/expressions/greater-than-or-equal/S11.8.4_A2.2_T1': [PASS, NO_VARIANTS],
-  'language/expressions/greater-than/S11.8.2_A2.2_T1': [PASS, NO_VARIANTS],
-  'language/expressions/left-shift/S11.7.1_A2.2_T1': [PASS, NO_VARIANTS],
-  'language/expressions/left-shift/S11.7.1_A2.3_T1': [PASS, NO_VARIANTS],
-  'language/expressions/less-than-or-equal/S11.8.3_A2.2_T1': [PASS, NO_VARIANTS],
-  'language/expressions/less-than/S11.8.1_A2.2_T1': [PASS, NO_VARIANTS],
-  'language/expressions/multiplication/S11.5.1_A2.2_T1': [PASS, NO_VARIANTS],
-  'language/expressions/multiplication/S11.5.1_A2.3_T1': [PASS, NO_VARIANTS],
-  'language/expressions/right-shift/S11.7.2_A2.2_T1': [PASS, NO_VARIANTS],
-  'language/expressions/right-shift/S11.7.2_A2.3_T1': [PASS, NO_VARIANTS],
-  'language/expressions/subtraction/S11.6.2_A2.2_T1': [PASS, NO_VARIANTS],
-  'language/expressions/subtraction/S11.6.2_A2.3_T1': [PASS, NO_VARIANTS],
-  'language/expressions/unary-minus/S11.4.7_A2.2_T1': [PASS, NO_VARIANTS],
-  'language/expressions/unary-plus/S11.4.6_A2.2_T1': [PASS, NO_VARIANTS],
-  'language/expressions/unsigned-right-shift/S11.7.3_A2.2_T1': [PASS, NO_VARIANTS],
-  'language/expressions/unsigned-right-shift/S11.7.3_A2.3_T1': [PASS, NO_VARIANTS],
-  'language/expressions/unsigned-right-shift/S9.6_A3.1_T4': [PASS, NO_VARIANTS],
-
   ###################### NEEDS INVESTIGATION #######################
 
   # Possibly same cause as S8.5_A2.1, below: floating-point tests.

test/test262/test262.status

@@ -36,38 +36,6 @@
   '11.2.3_b': [FAIL],
   '12.2.3_b': [FAIL],
 
-  # TODO(mstarzinger): The implicit JSToNumber and JSToString nodes inserted by
-  # JSTypedLowering are not properly connected to a surrounding exceptional
-  # continuation yet, hence exception are thrown out of the function body.
-  'S11.10.1_A2.2_T1': [PASS, NO_VARIANTS],
-  'S11.10.1_A2.3_T1': [PASS, NO_VARIANTS],
-  'S11.10.2_A2.2_T1': [PASS, NO_VARIANTS],
-  'S11.10.2_A2.3_T1': [PASS, NO_VARIANTS],
-  'S11.10.3_A2.2_T1': [PASS, NO_VARIANTS],
-  'S11.10.3_A2.3_T1': [PASS, NO_VARIANTS],
-  'S11.4.6_A2.2_T1': [PASS, NO_VARIANTS],
-  'S11.4.7_A2.2_T1': [PASS, NO_VARIANTS],
-  'S11.4.8_A2.2_T1': [PASS, NO_VARIANTS],
-  'S11.5.1_A2.2_T1': [PASS, NO_VARIANTS],
-  'S11.5.1_A2.3_T1': [PASS, NO_VARIANTS],
-  'S11.5.2_A2.2_T1': [PASS, NO_VARIANTS],
-  'S11.5.2_A2.3_T1': [PASS, NO_VARIANTS],
-  'S11.6.2_A2.2_T1': [PASS, NO_VARIANTS],
-  'S11.6.2_A2.3_T1': [PASS, NO_VARIANTS],
-  'S11.7.1_A2.2_T1': [PASS, NO_VARIANTS],
-  'S11.7.1_A2.3_T1': [PASS, NO_VARIANTS],
-  'S11.7.2_A2.2_T1': [PASS, NO_VARIANTS],
-  'S11.7.2_A2.3_T1': [PASS, NO_VARIANTS],
-  'S11.7.3_A2.2_T1': [PASS, NO_VARIANTS],
-  'S11.7.3_A2.3_T1': [PASS, NO_VARIANTS],
-  'S11.8.1_A2.2_T1': [PASS, NO_VARIANTS],
-  'S11.8.2_A2.2_T1': [PASS, NO_VARIANTS],
-  'S11.8.3_A2.2_T1': [PASS, NO_VARIANTS],
-  'S11.8.4_A2.2_T1': [PASS, NO_VARIANTS],
-  'S9.5_A3.1_T4': [PASS, NO_VARIANTS],
-  'S9.6_A3.1_T4': [PASS, NO_VARIANTS],
-  'S9.7_A3.1_T4': [PASS, NO_VARIANTS],
-
   ############################### ES6 ###################################
   # ES6 allows block-local functions.
   'Sbp_A1_T1': [PASS, FAIL_OK],