[Turbofan]: Eliminate the check for -0 if it's not possible/observable.

In int32 multiplication, if we have a positive integer as input, then we know we can't produce a -0 answer. The same is true if truncation is applied (x * y | 0). Without this information, we have to rather annoyingly check if the result of multiplication is 0, then OR the inputs to check for negativity, and possibly return -0. In TurboFan, we'll deopt in this case.

BUG=

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

src/compiler/effect-control-linearizer.cc

@@ -1325,6 +1325,7 @@ EffectControlLinearizer::LowerCheckedUint32Mod(Node* node, Node* frame_state,
 EffectControlLinearizer::ValueEffectControl
 EffectControlLinearizer::LowerCheckedInt32Mul(Node* node, Node* frame_state,
                                               Node* effect, Node* control) {
+  CheckForMinusZeroMode mode = CheckMinusZeroModeOf(node->op());
   Node* zero = jsgraph()->Int32Constant(0);
   Node* lhs = node->InputAt(0);
   Node* rhs = node->InputAt(1);
@@ -1339,28 +1340,30 @@ EffectControlLinearizer::LowerCheckedInt32Mul(Node* node, Node* frame_state,
 
   Node* value = graph()->NewNode(common()->Projection(0), projection, control);
 
-  Node* check_zero = graph()->NewNode(machine()->Word32Equal(), value, zero);
-  Node* branch_zero = graph()->NewNode(common()->Branch(BranchHint::kFalse),
-                                       check_zero, control);
+  if (mode == CheckForMinusZeroMode::kCheckForMinusZero) {
+    Node* check_zero = graph()->NewNode(machine()->Word32Equal(), value, zero);
+    Node* branch_zero = graph()->NewNode(common()->Branch(BranchHint::kFalse),
+                                         check_zero, control);
 
-  Node* if_zero = graph()->NewNode(common()->IfTrue(), branch_zero);
-  Node* e_if_zero = effect;
-  {
-    // We may need to return negative zero.
-    Node* or_inputs = graph()->NewNode(machine()->Word32Or(), lhs, rhs);
-    Node* check_or =
-        graph()->NewNode(machine()->Int32LessThan(), or_inputs, zero);
-    if_zero = e_if_zero =
-        graph()->NewNode(common()->DeoptimizeIf(DeoptimizeReason::kMinusZero),
-                         check_or, frame_state, e_if_zero, if_zero);
-  }
+    Node* if_zero = graph()->NewNode(common()->IfTrue(), branch_zero);
+    Node* e_if_zero = effect;
+    {
+      // We may need to return negative zero.
+      Node* or_inputs = graph()->NewNode(machine()->Word32Or(), lhs, rhs);
+      Node* check_or =
+          graph()->NewNode(machine()->Int32LessThan(), or_inputs, zero);
+      if_zero = e_if_zero =
+          graph()->NewNode(common()->DeoptimizeIf(DeoptimizeReason::kMinusZero),
+                           check_or, frame_state, e_if_zero, if_zero);
+    }
 
-  Node* if_not_zero = graph()->NewNode(common()->IfFalse(), branch_zero);
-  Node* e_if_not_zero = effect;
+    Node* if_not_zero = graph()->NewNode(common()->IfFalse(), branch_zero);
+    Node* e_if_not_zero = effect;
 
-  control = graph()->NewNode(common()->Merge(2), if_zero, if_not_zero);
-  effect = graph()->NewNode(common()->EffectPhi(2), e_if_zero, e_if_not_zero,
-                            control);
+    control = graph()->NewNode(common()->Merge(2), if_zero, if_not_zero);
+    effect = graph()->NewNode(common()->EffectPhi(2), e_if_zero, e_if_not_zero,
+                              control);
+  }
 
   return ValueEffectControl(value, effect, control);
 }

src/compiler/representation-change.cc

@@ -627,8 +627,6 @@ const Operator* RepresentationChanger::Int32OverflowOperatorFor(
       return simplified()->CheckedInt32Div();
     case IrOpcode::kSpeculativeNumberModulus:
       return simplified()->CheckedInt32Mod();
-    case IrOpcode::kSpeculativeNumberMultiply:
-      return simplified()->CheckedInt32Mul();
     default:
       UNREACHABLE();
       return nullptr;

src/compiler/simplified-lowering.cc

@@ -1077,6 +1077,22 @@ class RepresentationSelector {
     return jsgraph_->simplified();
   }
 
+  void LowerToCheckedInt32Mul(Node* node, Truncation truncation,
+                              Type* input0_type, Type* input1_type) {
+    // If one of the inputs is positive and/or truncation is being applied,
+    // there is no need to return -0.
+    CheckForMinusZeroMode mz_mode =
+        truncation.TruncatesToWord32() ||
+                (input0_type->Is(Type::OrderedNumber()) &&
+                 input0_type->Min() > 0) ||
+                (input1_type->Is(Type::OrderedNumber()) &&
+                 input1_type->Min() > 0)
+            ? CheckForMinusZeroMode::kDontCheckForMinusZero
+            : CheckForMinusZeroMode::kCheckForMinusZero;
+
+    NodeProperties::ChangeOp(node, simplified()->CheckedInt32Mul(mz_mode));
+  }
+
   void ChangeToInt32OverflowOp(Node* node) {
     NodeProperties::ChangeOp(node, Int32OverflowOp(node));
   }
@@ -1333,6 +1349,8 @@ class RepresentationSelector {
         }
         // Try to use type feedback.
         BinaryOperationHints::Hint hint = BinaryOperationHintOf(node->op());
+        Type* input0_type = TypeOf(node->InputAt(0));
+        Type* input1_type = TypeOf(node->InputAt(1));
 
         // Handle the case when no int32 checks on inputs are necessary
         // (but an overflow check is needed on the output).
@@ -1342,7 +1360,10 @@ class RepresentationSelector {
               hint == BinaryOperationHints::kSigned32) {
             VisitBinop(node, UseInfo::TruncatingWord32(),
                        MachineRepresentation::kWord32, Type::Signed32());
-            if (lower()) ChangeToInt32OverflowOp(node);
+            if (lower()) {
+              LowerToCheckedInt32Mul(node, truncation, input0_type,
+                                     input1_type);
+            }
             return;
           }
         }
@@ -1351,7 +1372,9 @@ class RepresentationSelector {
             hint == BinaryOperationHints::kSigned32) {
           VisitBinop(node, UseInfo::CheckedSigned32AsWord32(),
                      MachineRepresentation::kWord32, Type::Signed32());
-          if (lower()) ChangeToInt32OverflowOp(node);
+          if (lower()) {
+            LowerToCheckedInt32Mul(node, truncation, input0_type, input1_type);
+          }
           return;
         }
 

src/compiler/simplified-operator.cc

@@ -201,6 +201,26 @@ CheckFloat64HoleMode CheckFloat64HoleModeOf(const Operator* op) {
   return OpParameter<CheckFloat64HoleMode>(op);
 }
 
+CheckForMinusZeroMode CheckMinusZeroModeOf(const Operator* op) {
+  DCHECK_EQ(IrOpcode::kCheckedInt32Mul, op->opcode());
+  return OpParameter<CheckForMinusZeroMode>(op);
+}
+
+size_t hash_value(CheckForMinusZeroMode mode) {
+  return static_cast<size_t>(mode);
+}
+
+std::ostream& operator<<(std::ostream& os, CheckForMinusZeroMode mode) {
+  switch (mode) {
+    case CheckForMinusZeroMode::kCheckForMinusZero:
+      return os << "check-for-minus-zero";
+    case CheckForMinusZeroMode::kDontCheckForMinusZero:
+      return os << "dont-check-for-minus-zero";
+  }
+  UNREACHABLE();
+  return os;
+}
+
 size_t hash_value(CheckTaggedHoleMode mode) {
   return static_cast<size_t>(mode);
 }
@@ -334,7 +354,6 @@ CompareOperationHints::Hint CompareOperationHintOf(const Operator* op) {
   V(CheckedInt32Mod, 2, 1)       \
   V(CheckedUint32Div, 2, 1)      \
   V(CheckedUint32Mod, 2, 1)      \
-  V(CheckedInt32Mul, 2, 1)       \
   V(CheckedUint32ToInt32, 1, 1)  \
   V(CheckedFloat64ToInt32, 1, 1) \
   V(CheckedTaggedToInt32, 1, 1)  \
@@ -362,6 +381,20 @@ struct SimplifiedOperatorGlobalCache final {
   CHECKED_OP_LIST(CHECKED)
 #undef CHECKED
 
+  template <CheckForMinusZeroMode kMode>
+  struct CheckedInt32MulOperator final
+      : public Operator1<CheckForMinusZeroMode> {
+    CheckedInt32MulOperator()
+        : Operator1<CheckForMinusZeroMode>(
+              IrOpcode::kCheckedInt32Mul,
+              Operator::kFoldable | Operator::kNoThrow, "CheckedInt32Mul", 2, 1,
+              1, 1, 1, 0, kMode) {}
+  };
+  CheckedInt32MulOperator<CheckForMinusZeroMode::kCheckForMinusZero>
+      kCheckedInt32MulCheckForMinusZeroOperator;
+  CheckedInt32MulOperator<CheckForMinusZeroMode::kDontCheckForMinusZero>
+      kCheckedInt32MulDontCheckForMinusZeroOperator;
+
   template <CheckFloat64HoleMode kMode>
   struct CheckFloat64HoleNaNOperator final
       : public Operator1<CheckFloat64HoleMode> {
@@ -441,6 +474,18 @@ PURE_OP_LIST(GET_FROM_CACHE)
 CHECKED_OP_LIST(GET_FROM_CACHE)
 #undef GET_FROM_CACHE
 
+const Operator* SimplifiedOperatorBuilder::CheckedInt32Mul(
+    CheckForMinusZeroMode mode) {
+  switch (mode) {
+    case CheckForMinusZeroMode::kCheckForMinusZero:
+      return &cache_.kCheckedInt32MulCheckForMinusZeroOperator;
+    case CheckForMinusZeroMode::kDontCheckForMinusZero:
+      return &cache_.kCheckedInt32MulDontCheckForMinusZeroOperator;
+  }
+  UNREACHABLE();
+  return nullptr;
+}
+
 const Operator* SimplifiedOperatorBuilder::CheckFloat64Hole(
     CheckFloat64HoleMode mode) {
   switch (mode) {

src/compiler/simplified-operator.h

@@ -129,6 +129,17 @@ std::ostream& operator<<(std::ostream&, CheckTaggedHoleMode);
 
 CheckTaggedHoleMode CheckTaggedHoleModeOf(const Operator*) WARN_UNUSED_RESULT;
 
+enum class CheckForMinusZeroMode : uint8_t {
+  kCheckForMinusZero,
+  kDontCheckForMinusZero,
+};
+
+size_t hash_value(CheckForMinusZeroMode);
+
+std::ostream& operator<<(std::ostream&, CheckForMinusZeroMode);
+
+CheckForMinusZeroMode CheckMinusZeroModeOf(const Operator*) WARN_UNUSED_RESULT;
+
 Type* TypeOf(const Operator* op) WARN_UNUSED_RESULT;
 
 BinaryOperationHints::Hint BinaryOperationHintOf(const Operator* op);
@@ -261,7 +272,7 @@ class SimplifiedOperatorBuilder final : public ZoneObject {
   const Operator* CheckedInt32Mod();
   const Operator* CheckedUint32Div();
   const Operator* CheckedUint32Mod();
-  const Operator* CheckedInt32Mul();
+  const Operator* CheckedInt32Mul(CheckForMinusZeroMode);
   const Operator* CheckedUint32ToInt32();
   const Operator* CheckedFloat64ToInt32();
   const Operator* CheckedTaggedToInt32();

test/mjsunit/math-mul.js → test/mjsunit/compiler/math-mul.js

@@ -4,6 +4,20 @@
 
 // Flags: --allow-natives-syntax
 
+// For TurboFan, make sure we can eliminate the -0 return value check
+// by recognizing a constant value.
+function gotaconstant(y) { return 15 * y; }
+assertEquals(45, gotaconstant(3));
+gotaconstant(3);
+%OptimizeFunctionOnNextCall(gotaconstant);
+gotaconstant(3);
+
+function gotaconstant_truncated(x, y) { return x * y | 0; }
+assertEquals(45, gotaconstant_truncated(3, 15));
+gotaconstant_truncated(3, 15);
+%OptimizeFunctionOnNextCall(gotaconstant_truncated);
+gotaconstant_truncated(3, 15);
+
 function test(x, y) { return x * y; }
 
 assertEquals(12, test(3, 4));

test/mjsunit/mjsunit.status

@@ -246,10 +246,7 @@
   'getters-on-elements': [PASS, NO_IGNITION],
   'harmony/do-expressions': [PASS, NO_IGNITION],
   'math-floor-of-div-minus-zero': [PASS, NO_IGNITION],
-
-  # TODO(mvstanton): restore NO_IGNITION.
-  'regress/regress-2132': [PASS, NO_VARIANTS],
-
+  'regress/regress-2132': [PASS, NO_IGNITION],
   'regress/regress-2339': [PASS, NO_IGNITION],
   'regress/regress-3176': [PASS, NO_IGNITION],
   'regress/regress-3709': [PASS, NO_IGNITION],