[turbofan] Remove int32 narrowing during typed lowering.

With Int32Add we lose the int/uint distinction, so later, in simplified lowering we can make a wrong decision. E.g., see the attached test case, where we lower NumberAdd -> Int32Add because inputs are Uint32, but during simplified lowering we change the inputs to Int32, so we get a wrong result.

Simplified lowering will lower the NumberAdd operations anyway, so we should lose performance.

BUG=
R=bmeurer@chromium.org

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

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

src/compiler/js-typed-lowering.cc

@@ -179,48 +179,10 @@ class JSBinopReduction {
     return n;
   }
 
-  // Try narrowing a double or number operation to an Int32 operation.
-  bool TryNarrowingToI32(Type* type, Node* node) {
-    switch (node->opcode()) {
-      case IrOpcode::kFloat64Add:
-      case IrOpcode::kNumberAdd: {
-        JSBinopReduction r(lowering_, node);
-        if (r.BothInputsAre(Type::Integral32())) {
-          node->set_op(lowering_->machine()->Int32Add());
-          // TODO(titzer): narrow bounds instead of overwriting.
-          NodeProperties::SetBounds(node, Bounds(type));
-          return true;
-        }
-      }
-      case IrOpcode::kFloat64Sub:
-      case IrOpcode::kNumberSubtract: {
-        JSBinopReduction r(lowering_, node);
-        if (r.BothInputsAre(Type::Integral32())) {
-          node->set_op(lowering_->machine()->Int32Sub());
-          // TODO(titzer): narrow bounds instead of overwriting.
-          NodeProperties::SetBounds(node, Bounds(type));
-          return true;
-        }
-      }
-      default:
-        return false;
-    }
-  }
-
   Node* ConvertToI32(bool is_signed, Node* node) {
-    Type* type = is_signed ? Type::Signed32() : Type::Unsigned32();
-    if (node->OwnedBy(node_)) {
-      // If this node {node_} has the only edge to {node}, then try narrowing
-      // its operation to an Int32 add or subtract.
-      if (TryNarrowingToI32(type, node)) return node;
-    } else {
-      // Otherwise, {node} has multiple uses. Leave it as is and let the
-      // further lowering passes deal with it, which use a full backwards
-      // fixpoint.
-    }
-
     // Avoid introducing too many eager NumberToXXnt32() operations.
     node = ConvertToNumber(node);
+    Type* type = is_signed ? Type::Signed32() : Type::Unsigned32();
     Type* input_type = NodeProperties::GetBounds(node).upper;
 
     if (input_type->Is(type)) return node;  // already in the value range.

test/cctest/compiler/test-js-typed-lowering.cc

@@ -1231,11 +1231,7 @@ TEST(Int32AddNarrowing) {
             Node* r = R.reduce(or_node);
 
             CHECK_EQ(R.ops[o + 1]->opcode(), r->op()->opcode());
-            CHECK_EQ(IrOpcode::kInt32Add, add_node->opcode());
-            bool is_signed = l ? R.signedness[o] : R.signedness[o + 1];
-
-            Type* add_type = NodeProperties::GetBounds(add_node).upper;
-            CHECK(add_type->Is(I32Type(is_signed)));
+            CHECK_EQ(IrOpcode::kNumberAdd, add_node->opcode());
           }
         }
       }
@@ -1258,40 +1254,33 @@ TEST(Int32AddNarrowing) {
             Node* r = R.reduce(or_node);
 
             CHECK_EQ(R.ops[o + 1]->opcode(), r->op()->opcode());
-            CHECK_EQ(IrOpcode::kInt32Add, add_node->opcode());
-            bool is_signed = l ? R.signedness[o] : R.signedness[o + 1];
-
-            Type* add_type = NodeProperties::GetBounds(add_node).upper;
-            CHECK(add_type->Is(I32Type(is_signed)));
+            CHECK_EQ(IrOpcode::kNumberAdd, add_node->opcode());
           }
         }
       }
     }
   }
-}
-
-
-TEST(Int32AddNarrowingNotOwned) {
-  JSBitwiseTypedLoweringTester R;
+  {
+    JSBitwiseTypedLoweringTester R;
 
-  for (int o = 0; o < R.kNumberOps; o += 2) {
-    Node* n0 = R.Parameter(I32Type(R.signedness[o]));
-    Node* n1 = R.Parameter(I32Type(R.signedness[o + 1]));
-    Node* one = R.graph.NewNode(R.common.NumberConstant(1));
-
-    Node* add_node = R.Binop(R.simplified.NumberAdd(), n0, n1);
-    Node* or_node = R.Binop(R.ops[o], add_node, one);
-    Node* other_use = R.Binop(R.simplified.NumberAdd(), add_node, one);
-    Node* r = R.reduce(or_node);
-    CHECK_EQ(R.ops[o + 1]->opcode(), r->op()->opcode());
-    // Should not be reduced to Int32Add because of the other number add.
-    CHECK_EQ(IrOpcode::kNumberAdd, add_node->opcode());
-    // Conversion to int32 should be done.
-    CheckToI32(add_node, r->InputAt(0), R.signedness[o]);
-    CheckToI32(one, r->InputAt(1), R.signedness[o + 1]);
-    // The other use should also not be touched.
-    CHECK_EQ(add_node, other_use->InputAt(0));
-    CHECK_EQ(one, other_use->InputAt(1));
+    for (int o = 0; o < R.kNumberOps; o += 2) {
+      Node* n0 = R.Parameter(I32Type(R.signedness[o]));
+      Node* n1 = R.Parameter(I32Type(R.signedness[o + 1]));
+      Node* one = R.graph.NewNode(R.common.NumberConstant(1));
+
+      Node* add_node = R.Binop(R.simplified.NumberAdd(), n0, n1);
+      Node* or_node = R.Binop(R.ops[o], add_node, one);
+      Node* other_use = R.Binop(R.simplified.NumberAdd(), add_node, one);
+      Node* r = R.reduce(or_node);
+      CHECK_EQ(R.ops[o + 1]->opcode(), r->op()->opcode());
+      CHECK_EQ(IrOpcode::kNumberAdd, add_node->opcode());
+      // Conversion to int32 should be done.
+      CheckToI32(add_node, r->InputAt(0), R.signedness[o]);
+      CheckToI32(one, r->InputAt(1), R.signedness[o + 1]);
+      // The other use should also not be touched.
+      CHECK_EQ(add_node, other_use->InputAt(0));
+      CHECK_EQ(one, other_use->InputAt(1));
+    }
   }
 }
 

test/mjsunit/regress/regress-unsigned-mul-add.js

+// 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.
+
+function f(a) {
+  var x = a >>> 0;
+  return (x * 1.0 + x * 1.0) << 0;
+}
+
+assertEquals(-2, f(-1));