[turbofan] Fix optimized lowering of Math.imul.

We eagerly inserted Int32Mul for Math.imul during builtin lowering and
messed up with the types, which confused the representation selection.
This adds a proper NumberImul operator, and fixes the builtin reducer to
do the right thing according to the spec.

R=mstarzinger@chromium.org
BUG=v8:5006
LOG=n

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

src/compiler/js-builtin-reducer.cc

@@ -117,13 +117,15 @@ Reduction JSBuiltinReducer::ReduceMathMax(Node* node) {
   return NoChange();
 }
 
-
-// ES6 draft 08-24-14, section 20.2.2.19.
+// ES6 section 20.2.2.19 Math.imul ( x, y )
 Reduction JSBuiltinReducer::ReduceMathImul(Node* node) {
   JSCallReduction r(node);
-  if (r.InputsMatchTwo(Type::Integral32(), Type::Integral32())) {
-    // Math.imul(a:int32, b:int32) -> Int32Mul(a, b)
-    Node* value = graph()->NewNode(machine()->Int32Mul(), r.left(), r.right());
+  if (r.InputsMatchTwo(Type::Number(), Type::Number())) {
+    // Math.imul(a:number, b:number) -> NumberImul(NumberToUint32(a),
+    //                                             NumberToUint32(b))
+    Node* a = graph()->NewNode(simplified()->NumberToUint32(), r.left());
+    Node* b = graph()->NewNode(simplified()->NumberToUint32(), r.right());
+    Node* value = graph()->NewNode(simplified()->NumberImul(), a, b);
     return Replace(value);
   }
   return NoChange();

src/compiler/opcodes.h

@@ -186,6 +186,7 @@
   V(NumberShiftLeft)               \
   V(NumberShiftRight)              \
   V(NumberShiftRightLogical)       \
+  V(NumberImul)                    \
   V(NumberClz32)                   \
   V(NumberCeil)                    \
   V(NumberFloor)                   \

src/compiler/representation-change.cc

@@ -491,6 +491,8 @@ const Operator* RepresentationChanger::Uint32OperatorFor(
       return machine()->Uint32LessThanOrEqual();
     case IrOpcode::kNumberClz32:
       return machine()->Word32Clz();
+    case IrOpcode::kNumberImul:
+      return machine()->Int32Mul();
     default:
       UNREACHABLE();
       return nullptr;

src/compiler/simplified-lowering.cc

@@ -1043,6 +1043,12 @@ class RepresentationSelector {
         if (lower()) NodeProperties::ChangeOp(node, Uint32Op(node));
         break;
       }
+      case IrOpcode::kNumberImul: {
+        VisitBinop(node, UseInfo::TruncatingWord32(),
+                   UseInfo::TruncatingWord32(), MachineRepresentation::kWord32);
+        if (lower()) NodeProperties::ChangeOp(node, Uint32Op(node));
+        break;
+      }
       case IrOpcode::kNumberCeil: {
         VisitUnop(node, UseInfo::TruncatingFloat64(),
                   MachineRepresentation::kFloat64);

src/compiler/simplified-operator.cc

@@ -189,6 +189,7 @@ const ElementAccess& ElementAccessOf(const Operator* op) {
   V(NumberShiftLeft, Operator::kNoProperties, 2)           \
   V(NumberShiftRight, Operator::kNoProperties, 2)          \
   V(NumberShiftRightLogical, Operator::kNoProperties, 2)   \
+  V(NumberImul, Operator::kCommutative, 2)                 \
   V(NumberClz32, Operator::kNoProperties, 1)               \
   V(NumberCeil, Operator::kNoProperties, 1)                \
   V(NumberFloor, Operator::kNoProperties, 1)               \

src/compiler/simplified-operator.h

@@ -146,6 +146,7 @@ class SimplifiedOperatorBuilder final : public ZoneObject {
   const Operator* NumberShiftLeft();
   const Operator* NumberShiftRight();
   const Operator* NumberShiftRightLogical();
+  const Operator* NumberImul();
   const Operator* NumberClz32();
   const Operator* NumberCeil();
   const Operator* NumberFloor();

src/compiler/typer.cc

@@ -1745,6 +1745,8 @@ Type* Typer::Visitor::TypeNumberShiftRightLogical(Node* node) {
   return Type::Unsigned32();
 }
 
+Type* Typer::Visitor::TypeNumberImul(Node* node) { return Type::Signed32(); }
+
 Type* Typer::Visitor::TypeNumberClz32(Node* node) {
   return typer_->cache_.kZeroToThirtyTwo;
 }

src/compiler/verifier.cc

@@ -700,6 +700,12 @@ void Verifier::Visitor::Check(Node* node) {
       CheckValueInputIs(node, 1, Type::Unsigned32());
       CheckUpperIs(node, Type::Unsigned32());
       break;
+    case IrOpcode::kNumberImul:
+      // (Unsigned32, Unsigned32) -> Signed32
+      CheckValueInputIs(node, 0, Type::Unsigned32());
+      CheckValueInputIs(node, 1, Type::Unsigned32());
+      CheckUpperIs(node, Type::Signed32());
+      break;
     case IrOpcode::kNumberClz32:
       // Unsigned32 -> Unsigned32
       CheckValueInputIs(node, 0, Type::Unsigned32());

test/mjsunit/regress/regress-5006.js

+// Copyright 2016 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
+
+function foo(x) { return Math.imul(x|0, 2); }
+print(foo(1));
+print(foo(1));
+%OptimizeFunctionOnNextCall(foo);
+print(foo(1));

test/unittests/compiler/js-builtin-reducer-unittest.cc

@@ -149,8 +149,8 @@ TEST_F(JSBuiltinReducerTest, MathImul) {
   Node* control = graph()->start();
   Node* context = UndefinedConstant();
   Node* frame_state = graph()->start();
-  TRACED_FOREACH(Type*, t0, kIntegral32Types) {
-    TRACED_FOREACH(Type*, t1, kIntegral32Types) {
+  TRACED_FOREACH(Type*, t0, kNumberTypes) {
+    TRACED_FOREACH(Type*, t1, kNumberTypes) {
       Node* p0 = Parameter(t0, 0);
       Node* p1 = Parameter(t1, 1);
       Node* call = graph()->NewNode(javascript()->CallFunction(4), function,
@@ -159,7 +159,8 @@ TEST_F(JSBuiltinReducerTest, MathImul) {
       Reduction r = Reduce(call);
 
       ASSERT_TRUE(r.Changed());
-      EXPECT_THAT(r.replacement(), IsInt32Mul(p0, p1));
+      EXPECT_THAT(r.replacement(),
+                  IsNumberImul(IsNumberToUint32(p0), IsNumberToUint32(p1)));
     }
   }
 }

test/unittests/compiler/node-test-utils.cc

@@ -2196,6 +2196,7 @@ IS_BINOP_MATCHER(NumberMultiply)
 IS_BINOP_MATCHER(NumberShiftLeft)
 IS_BINOP_MATCHER(NumberShiftRight)
 IS_BINOP_MATCHER(NumberShiftRightLogical)
+IS_BINOP_MATCHER(NumberImul)
 IS_BINOP_MATCHER(Word32And)
 IS_BINOP_MATCHER(Word32Or)
 IS_BINOP_MATCHER(Word32Xor)

test/unittests/compiler/node-test-utils.h

@@ -209,6 +209,8 @@ Matcher<Node*> IsNumberShiftRight(const Matcher<Node*>& lhs_matcher,
                                   const Matcher<Node*>& rhs_matcher);
 Matcher<Node*> IsNumberShiftRightLogical(const Matcher<Node*>& lhs_matcher,
                                          const Matcher<Node*>& rhs_matcher);
+Matcher<Node*> IsNumberImul(const Matcher<Node*>& lhs_matcher,
+                            const Matcher<Node*>& rhs_matcher);
 Matcher<Node*> IsAllocate(const Matcher<Node*>& size_matcher,
                           const Matcher<Node*>& effect_matcher,
                           const Matcher<Node*>& control_matcher);