[Turbofan]: Add integer multiplication with overflow to typed lowering.

BUG=

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

src/code-stub-assembler.cc

@@ -401,6 +401,67 @@ Node* CodeStubAssembler::SmiMod(Node* a, Node* b) {
   return var_result.value();
 }
 
+Node* CodeStubAssembler::SmiMul(Node* a, Node* b) {
+  Variable var_result(this, MachineRepresentation::kTagged);
+  Variable var_lhs_float64(this, MachineRepresentation::kFloat64),
+      var_rhs_float64(this, MachineRepresentation::kFloat64);
+  Label return_result(this, &var_result);
+
+  // Both {a} and {b} are Smis. Convert them to integers and multiply.
+  Node* lhs32 = SmiToWord32(a);
+  Node* rhs32 = SmiToWord32(b);
+  Node* pair = Int32MulWithOverflow(lhs32, rhs32);
+
+  Node* overflow = Projection(1, pair);
+
+  // Check if the multiplication overflowed.
+  Label if_overflow(this, Label::kDeferred), if_notoverflow(this);
+  Branch(overflow, &if_overflow, &if_notoverflow);
+  Bind(&if_notoverflow);
+  {
+    // If the answer is zero, we may need to return -0.0, depending on the
+    // input.
+    Label answer_zero(this), answer_not_zero(this);
+    Node* answer = Projection(0, pair);
+    Node* zero = Int32Constant(0);
+    Branch(WordEqual(answer, zero), &answer_zero, &answer_not_zero);
+    Bind(&answer_not_zero);
+    {
+      var_result.Bind(ChangeInt32ToTagged(answer));
+      Goto(&return_result);
+    }
+    Bind(&answer_zero);
+    {
+      Node* or_result = Word32Or(lhs32, rhs32);
+      Label if_should_be_negative_zero(this), if_should_be_zero(this);
+      Branch(Int32LessThan(or_result, zero), &if_should_be_negative_zero,
+             &if_should_be_zero);
+      Bind(&if_should_be_negative_zero);
+      {
+        var_result.Bind(MinusZeroConstant());
+        Goto(&return_result);
+      }
+      Bind(&if_should_be_zero);
+      {
+        var_result.Bind(zero);
+        Goto(&return_result);
+      }
+    }
+  }
+  Bind(&if_overflow);
+  {
+    var_lhs_float64.Bind(SmiToFloat64(a));
+    var_rhs_float64.Bind(SmiToFloat64(b));
+    Node* value = Float64Mul(var_lhs_float64.value(), var_rhs_float64.value());
+    Node* result = ChangeFloat64ToTagged(value);
+    var_result.Bind(result);
+    Goto(&return_result);
+  }
+
+  Bind(&return_result);
+  return var_result.value();
+}
+
 Node* CodeStubAssembler::WordIsSmi(Node* a) {
   return WordEqual(WordAnd(a, IntPtrConstant(kSmiTagMask)), IntPtrConstant(0));
 }

src/code-stub-assembler.h

@@ -82,6 +82,8 @@ class CodeStubAssembler : public compiler::CodeAssembler {
   compiler::Node* SmiMin(compiler::Node* a, compiler::Node* b);
   // Computes a % b for Smi inputs a and b; result is not necessarily a Smi.
   compiler::Node* SmiMod(compiler::Node* a, compiler::Node* b);
+  // Computes a * b for Smi inputs a and b; result is not necessarily a Smi.
+  compiler::Node* SmiMul(compiler::Node* a, compiler::Node* b);
 
   // Allocate an object of the given size.
   compiler::Node* Allocate(compiler::Node* size, AllocationFlags flags = kNone);

src/code-stubs.cc

@@ -1097,57 +1097,11 @@ compiler::Node* MultiplyStub::Generate(CodeStubAssembler* assembler,
 
       assembler->Bind(&rhs_is_smi);
       {
-        // Both {lhs} and {rhs} are Smis. Convert them to integers and multiply.
-        Node* lhs32 = assembler->SmiToWord32(lhs);
-        Node* rhs32 = assembler->SmiToWord32(rhs);
-        Node* pair = assembler->Int32MulWithOverflow(lhs32, rhs32);
-
-        Node* overflow = assembler->Projection(1, pair);
-
-        // Check if the multiplication overflowed.
-        Label if_overflow(assembler, Label::kDeferred),
-            if_notoverflow(assembler);
-        assembler->Branch(overflow, &if_overflow, &if_notoverflow);
-        assembler->Bind(&if_notoverflow);
-        {
-          // If the answer is zero, we may need to return -0.0, depending on the
-          // input.
-          Label answer_zero(assembler), answer_not_zero(assembler);
-          Node* answer = assembler->Projection(0, pair);
-          Node* zero = assembler->Int32Constant(0);
-          assembler->Branch(assembler->WordEqual(answer, zero), &answer_zero,
-                            &answer_not_zero);
-          assembler->Bind(&answer_not_zero);
-          {
-            var_result.Bind(assembler->ChangeInt32ToTagged(answer));
-            assembler->Goto(&return_result);
-          }
-          assembler->Bind(&answer_zero);
-          {
-            Node* or_result = assembler->Word32Or(lhs32, rhs32);
-            Label if_should_be_negative_zero(assembler),
-                if_should_be_zero(assembler);
-            assembler->Branch(assembler->Int32LessThan(or_result, zero),
-                              &if_should_be_negative_zero, &if_should_be_zero);
-            assembler->Bind(&if_should_be_negative_zero);
-            {
-              var_result.Bind(assembler->MinusZeroConstant());
+        // Both {lhs} and {rhs} are Smis. The result is not necessarily a smi,
+        // in case of overflow.
+        var_result.Bind(assembler->SmiMul(lhs, rhs));
         assembler->Goto(&return_result);
       }
-            assembler->Bind(&if_should_be_zero);
-            {
-              var_result.Bind(zero);
-              assembler->Goto(&return_result);
-            }
-          }
-        }
-        assembler->Bind(&if_overflow);
-        {
-          var_lhs_float64.Bind(assembler->SmiToFloat64(lhs));
-          var_rhs_float64.Bind(assembler->SmiToFloat64(rhs));
-          assembler->Goto(&do_fmul);
-        }
-      }
 
       assembler->Bind(&rhs_is_not_smi);
       {

src/compiler/effect-control-linearizer.cc

@@ -648,6 +648,9 @@ bool EffectControlLinearizer::TryWireInStateEffect(Node* node,
     case IrOpcode::kCheckedUint32Mod:
       state = LowerCheckedUint32Mod(node, frame_state, *effect, *control);
       break;
+    case IrOpcode::kCheckedInt32Mul:
+      state = LowerCheckedInt32Mul(node, frame_state, *effect, *control);
+      break;
     case IrOpcode::kCheckedUint32ToInt32:
       state = LowerCheckedUint32ToInt32(node, frame_state, *effect, *control);
       break;
@@ -1298,6 +1301,47 @@ EffectControlLinearizer::LowerCheckedUint32Mod(Node* node, Node* frame_state,
   return ValueEffectControl(value, effect, control);
 }
 
+EffectControlLinearizer::ValueEffectControl
+EffectControlLinearizer::LowerCheckedInt32Mul(Node* node, Node* frame_state,
+                                              Node* effect, Node* control) {
+  Node* zero = jsgraph()->Int32Constant(0);
+  Node* lhs = node->InputAt(0);
+  Node* rhs = node->InputAt(1);
+
+  Node* projection =
+      graph()->NewNode(machine()->Int32MulWithOverflow(), lhs, rhs, control);
+
+  Node* check = graph()->NewNode(common()->Projection(1), projection, control);
+  control = effect = graph()->NewNode(common()->DeoptimizeIf(), check,
+                                      frame_state, effect, control);
+
+  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);
+
+  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(), 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;
+
+  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);
+}
+
 EffectControlLinearizer::ValueEffectControl
 EffectControlLinearizer::LowerCheckedUint32ToInt32(Node* node,
                                                    Node* frame_state,

src/compiler/effect-control-linearizer.h

@@ -83,6 +83,8 @@ class EffectControlLinearizer {
                                            Node* effect, Node* control);
   ValueEffectControl LowerCheckedUint32Mod(Node* node, Node* frame_state,
                                            Node* effect, Node* control);
+  ValueEffectControl LowerCheckedInt32Mul(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-graph.cc

@@ -92,7 +92,6 @@ Node* JSGraph::ZeroConstant() {
   return CACHED(kZeroConstant, NumberConstant(0.0));
 }
 
-
 Node* JSGraph::OneConstant() {
   return CACHED(kOneConstant, NumberConstant(1.0));
 }

src/compiler/js-typed-lowering.cc

@@ -510,6 +510,14 @@ Reduction JSTypedLowering::ReduceJSSubtract(Node* node) {
 Reduction JSTypedLowering::ReduceJSMultiply(Node* node) {
   JSBinopReduction r(this, node);
   BinaryOperationHints::Hint feedback = r.GetNumberBinaryOperationFeedback();
+  if (feedback == BinaryOperationHints::kNumberOrOddball &&
+      r.BothInputsAre(Type::PlainPrimitive())) {
+    // JSMultiply(x:plain-primitive,
+    //            y:plain-primitive) => NumberMultiply(ToNumber(x), ToNumber(y))
+    r.ConvertInputsToNumber();
+    return r.ChangeToPureOperator(simplified()->NumberMultiply(),
+                                  Type::Number());
+  }
   if (feedback != BinaryOperationHints::kAny) {
     return r.ChangeToSpeculativeOperator(
         simplified()->SpeculativeNumberMultiply(feedback), Type::Number());

src/compiler/opcodes.h

@@ -184,6 +184,7 @@
   V(CheckedInt32Mod)                  \
   V(CheckedUint32Div)                 \
   V(CheckedUint32Mod)                 \
+  V(CheckedInt32Mul)                  \
   V(CheckedUint32ToInt32)             \
   V(CheckedFloat64ToInt32)            \
   V(CheckedTaggedToInt32)             \

src/compiler/redundancy-elimination.cc

@@ -29,6 +29,7 @@ Reduction RedundancyElimination::Reduce(Node* node) {
     case IrOpcode::kCheckedInt32Sub:
     case IrOpcode::kCheckedInt32Div:
     case IrOpcode::kCheckedInt32Mod:
+    case IrOpcode::kCheckedInt32Mul:
     case IrOpcode::kCheckedTaggedToFloat64:
     case IrOpcode::kCheckedTaggedToInt32:
     case IrOpcode::kCheckedUint32ToInt32:

src/compiler/representation-change.cc

@@ -627,6 +627,8 @@ 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

@@ -1311,8 +1311,7 @@ class RepresentationSelector {
         }
         return;
       }
-      case IrOpcode::kSpeculativeNumberMultiply:
-      case IrOpcode::kNumberMultiply: {
+      case IrOpcode::kSpeculativeNumberMultiply: {
         if (BothInputsAre(node, Type::Integral32()) &&
             (NodeProperties::GetType(node)->Is(Type::Signed32()) ||
              NodeProperties::GetType(node)->Is(Type::Unsigned32()) ||
@@ -1328,19 +1327,57 @@ class RepresentationSelector {
           if (lower()) ChangeToPureOp(node, Int32Op(node));
           return;
         }
-        // Number x Number => Float64Mul
-        if (BothInputsAre(node, Type::NumberOrUndefined())) {
-          VisitFloat64Binop(node);
-          if (lower()) ChangeToPureOp(node, Float64Op(node));
+        // Try to use type feedback.
+        BinaryOperationHints::Hint hint = BinaryOperationHintOf(node->op());
+
+        // Handle the case when no int32 checks on inputs are necessary
+        // (but an overflow check is needed on the output).
+        if (BothInputsAre(node, Type::Signed32())) {
+          // If both the inputs the feedback are int32, use the overflow op.
+          if (hint == BinaryOperationHints::kSignedSmall ||
+              hint == BinaryOperationHints::kSigned32) {
+            VisitBinop(node, UseInfo::TruncatingWord32(),
+                       MachineRepresentation::kWord32, Type::Signed32());
+            if (lower()) ChangeToInt32OverflowOp(node);
             return;
           }
+        }
+
+        if (hint == BinaryOperationHints::kSignedSmall ||
+            hint == BinaryOperationHints::kSigned32) {
+          VisitBinop(node, UseInfo::CheckedSigned32AsWord32(),
+                     MachineRepresentation::kWord32, Type::Signed32());
+          if (lower()) ChangeToInt32OverflowOp(node);
+          return;
+        }
+
         // Checked float64 x float64 => float64
-        DCHECK_EQ(IrOpcode::kSpeculativeNumberMultiply, node->opcode());
         VisitBinop(node, UseInfo::CheckedNumberOrOddballAsFloat64(),
                    MachineRepresentation::kFloat64, Type::Number());
         if (lower()) ChangeToPureOp(node, Float64Op(node));
         return;
       }
+      case IrOpcode::kNumberMultiply: {
+        if (BothInputsAre(node, Type::Integral32()) &&
+            (NodeProperties::GetType(node)->Is(Type::Signed32()) ||
+             NodeProperties::GetType(node)->Is(Type::Unsigned32()) ||
+             (truncation.TruncatesToWord32() &&
+              NodeProperties::GetType(node)->Is(
+                  type_cache_.kSafeIntegerOrMinusZero)))) {
+          // Multiply reduces to Int32Mul if the inputs are integers, and
+          // (a) the output is either known to be Signed32, or
+          // (b) the output is known to be Unsigned32, or
+          // (c) the uses are truncating and the result is in the safe
+          //     integer range.
+          VisitWord32TruncatingBinop(node);
+          if (lower()) ChangeToPureOp(node, Int32Op(node));
+          return;
+        }
+        // Number x Number => Float64Mul
+        VisitFloat64Binop(node);
+        if (lower()) ChangeToPureOp(node, Float64Op(node));
+        return;
+      }
       case IrOpcode::kSpeculativeNumberDivide: {
         if (BothInputsAreUnsigned32(node) && truncation.TruncatesToWord32()) {
           // => unsigned Uint32Div

src/compiler/simplified-operator.cc

@@ -325,6 +325,7 @@ 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)  \

src/compiler/simplified-operator.h

@@ -257,6 +257,7 @@ class SimplifiedOperatorBuilder final : public ZoneObject {
   const Operator* CheckedInt32Mod();
   const Operator* CheckedUint32Div();
   const Operator* CheckedUint32Mod();
+  const Operator* CheckedInt32Mul();
   const Operator* CheckedUint32ToInt32();
   const Operator* CheckedFloat64ToInt32();
   const Operator* CheckedTaggedToInt32();

src/compiler/verifier.cc

@@ -951,6 +951,7 @@ void Verifier::Visitor::Check(Node* node) {
     case IrOpcode::kCheckedInt32Mod:
     case IrOpcode::kCheckedUint32Div:
     case IrOpcode::kCheckedUint32Mod:
+    case IrOpcode::kCheckedInt32Mul:
     case IrOpcode::kCheckedUint32ToInt32:
     case IrOpcode::kCheckedFloat64ToInt32:
     case IrOpcode::kCheckedTaggedToInt32:

test/mjsunit/math-mul.js

@@ -2,12 +2,20 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+// Flags: --allow-natives-syntax
+
 function test(x, y) { return x * y; }
 
-assertEquals(8, test(2, 4));
+assertEquals(12, test(3, 4));
+assertEquals(16, test(4, 4));
+
+%OptimizeFunctionOnNextCall(test);
+assertEquals(27, test(9, 3));
+
 assertEquals(-0, test(-3, 0));
 assertEquals(-0, test(0, -0));
 
+
 const SMI_MAX = (1 << 29) - 1 + (1 << 29);  // Create without overflowing.
 const SMI_MIN = -SMI_MAX - 1;  // Create without overflowing.
 

test/mjsunit/mjsunit.status

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