[turbofan] Correctify typed lowering.

We cannot add new JSToNumber nodes here in general, because:

 a) The inserted ToNumber operation screws up observability of valueOf.
 b) Deoptimization at ToNumber doesn't have corresponding bailout id.

TEST=cctest,mjsunit
R=jarin@chromium.org

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

git-svn-id: https://v8.googlecode.com/svn/branches/bleeding_edge@24660 ce2b1a6d-e550-0410-aec6-3dcde31c8c00

diff --git a/src/compiler/js-typed-lowering.cc b/src/compiler/js-typed-lowering.cc
index ad08196..cc8d6ce 100644 (file)
--- a/src/compiler/js-typed-lowering.cc
+++ b/src/compiler/js-typed-lowering.cc
@@ -228,12 +228,21 @@ Reduction JSTypedLowering::ReduceJSAdd(Node* node) {
     return r.ChangeToPureOperator(simplified()->NumberAdd());
   }
   Type* maybe_string = Type::Union(Type::String(), Type::Receiver(), zone());
-  if (r.NeitherInputCanBe(maybe_string)) {
+  if (r.BothInputsAre(Type::Primitive()) && r.NeitherInputCanBe(maybe_string)) {
     // JSAdd(x:-string, y:-string) => NumberAdd(ToNumber(x), ToNumber(y))
     r.ConvertInputsToNumber();
     return r.ChangeToPureOperator(simplified()->NumberAdd());
   }
 #if 0
+  // TODO(turbofan): General ToNumber disabled for now because:
+  //   a) The inserted ToNumber operation screws up observability of valueOf.
+  //   b) Deoptimization at ToNumber doesn't have corresponding bailout id.
+  Type* maybe_string = Type::Union(Type::String(), Type::Receiver(), zone());
+  if (r.NeitherInputCanBe(maybe_string)) {
+    ...
+  }
+#endif
+#if 0
   // TODO(turbofan): Lowering of StringAdd is disabled for now because:
   //   a) The inserted ToString operation screws up valueOf vs. toString order.
   //   b) Deoptimization at ToString doesn't have corresponding bailout id.
@@ -253,6 +262,14 @@ Reduction JSTypedLowering::ReduceJSAdd(Node* node) {
 Reduction JSTypedLowering::ReduceNumberBinop(Node* node,
                                              const Operator* numberOp) {
   JSBinopReduction r(this, node);
+  if (r.BothInputsAre(Type::Primitive())) {
+    r.ConvertInputsToNumber();
+    return r.ChangeToPureOperator(numberOp);
+  }
+#if 0
+  // TODO(turbofan): General ToNumber disabled for now because:
+  //   a) The inserted ToNumber operation screws up observability of valueOf.
+  //   b) Deoptimization at ToNumber doesn't have corresponding bailout id.
   if (r.OneInputIs(Type::Primitive())) {
     // If at least one input is a primitive, then insert appropriate conversions
     // to number and reduce this operator to the given numeric one.
@@ -260,6 +277,7 @@ Reduction JSTypedLowering::ReduceNumberBinop(Node* node,
     r.ConvertInputsToNumber();
     return r.ChangeToPureOperator(numberOp);
   }
+#endif
   // TODO(turbofan): relax/remove the effects of this operator in other cases.
   return NoChange();
 }
@@ -269,20 +287,27 @@ Reduction JSTypedLowering::ReduceI32Binop(Node* node, bool left_signed,
                                           bool right_signed,
                                           const Operator* intOp) {
   JSBinopReduction r(this, node);
-  // TODO(titzer): some Smi bitwise operations don't really require going
-  // all the way to int32, which can save tagging/untagging for some operations
-  // on some platforms.
-  // TODO(turbofan): make this heuristic configurable for code size.
-  r.ConvertInputsToInt32(left_signed, right_signed);
-  return r.ChangeToPureOperator(intOp);
+  if (r.BothInputsAre(Type::Primitive())) {
+    // TODO(titzer): some Smi bitwise operations don't really require going
+    // all the way to int32, which can save tagging/untagging for some
+    // operations
+    // on some platforms.
+    // TODO(turbofan): make this heuristic configurable for code size.
+    r.ConvertInputsToInt32(left_signed, right_signed);
+    return r.ChangeToPureOperator(intOp);
+  }
+  return NoChange();
 }
 
 
 Reduction JSTypedLowering::ReduceI32Shift(Node* node, bool left_signed,
                                           const Operator* shift_op) {
   JSBinopReduction r(this, node);
-  r.ConvertInputsForShift(left_signed);
-  return r.ChangeToPureOperator(shift_op);
+  if (r.BothInputsAre(Type::Primitive())) {
+    r.ConvertInputsForShift(left_signed);
+    return r.ChangeToPureOperator(shift_op);
+  }
+  return NoChange();
 }
 
 
@@ -311,9 +336,18 @@ Reduction JSTypedLowering::ReduceJSComparison(Node* node) {
     }
     return r.ChangeToPureOperator(stringOp);
   }
+#if 0
+  // TODO(turbofan): General ToNumber disabled for now because:
+  //   a) The inserted ToNumber operation screws up observability of valueOf.
+  //   b) Deoptimization at ToNumber doesn't have corresponding bailout id.
   Type* maybe_string = Type::Union(Type::String(), Type::Receiver(), zone());
   if (r.OneInputCannotBe(maybe_string)) {
     // If one input cannot be a string, then emit a number comparison.
+    ...
+  }
+#endif
+  Type* maybe_string = Type::Union(Type::String(), Type::Receiver(), zone());
+  if (r.BothInputsAre(Type::Primitive()) && r.OneInputCannotBe(maybe_string)) {
     const Operator* less_than;
     const Operator* less_than_or_equal;
     if (r.BothInputsAre(Type::Unsigned32())) {

diff --git a/test/cctest/compiler/test-js-typed-lowering.cc b/test/cctest/compiler/test-js-typed-lowering.cc
index 9dbe330..d48a462 100644 (file)
--- a/test/cctest/compiler/test-js-typed-lowering.cc
+++ b/test/cctest/compiler/test-js-typed-lowering.cc
@@ -308,7 +308,7 @@ TEST(Int32BitwiseShifts) {
       Type::Unsigned32(),  Type::Signed32(),      Type::MinusZero(),
       Type::NaN(),         Type::OtherNumber(),   Type::Undefined(),
       Type::Null(),        Type::Boolean(),       Type::Number(),
-      Type::String(),      Type::Object()};
+      Type::String()};
 
   for (size_t i = 0; i < arraysize(types); ++i) {
     Node* p0 = R.Parameter(types[i], 0);
@@ -367,8 +367,7 @@ TEST(Int32BitwiseBinops) {
       Type::SignedSmall(), Type::UnsignedSmall(), Type::Unsigned32(),
       Type::Signed32(),    Type::MinusZero(),     Type::NaN(),
       Type::OtherNumber(), Type::Undefined(),     Type::Null(),
-      Type::Boolean(),     Type::Number(),        Type::String(),
-      Type::Object()};
+      Type::Boolean(),     Type::Number(),        Type::String()};
 
   for (size_t i = 0; i < arraysize(types); ++i) {
     Node* p0 = R.Parameter(types[i], 0);
@@ -691,33 +690,22 @@ TEST(NumberComparison) {
       R.javascript.GreaterThan(),        R.simplified.NumberLessThan(),
       R.javascript.GreaterThanOrEqual(), R.simplified.NumberLessThanOrEqual()};
 
-  for (size_t i = 0; i < arraysize(kJSTypes); i++) {
-    Type* t0 = kJSTypes[i];
-    // Skip Type::String and Type::Receiver which might coerce into a string.
-    if (t0->Is(Type::String()) || t0->Is(Type::Receiver())) continue;
-    Node* p0 = R.Parameter(t0, 0);
+  Node* const p0 = R.Parameter(Type::Number(), 0);
+  Node* const p1 = R.Parameter(Type::Number(), 1);
 
-    for (size_t j = 0; j < arraysize(kJSTypes); j++) {
-      Type* t1 = kJSTypes[j];
-      // Skip Type::String and Type::Receiver which might coerce into a string.
-      if (t1->Is(Type::String()) || t0->Is(Type::Receiver())) continue;
-      Node* p1 = R.Parameter(t1, 1);
+  for (size_t k = 0; k < arraysize(ops); k += 2) {
+    Node* cmp = R.Binop(ops[k], p0, p1);
+    Node* r = R.reduce(cmp);
 
-      for (size_t k = 0; k < arraysize(ops); k += 2) {
-        Node* cmp = R.Binop(ops[k], p0, p1);
-        Node* r = R.reduce(cmp);
-
-        R.CheckPureBinop(ops[k + 1], r);
-        if (k >= 4) {
-          // GreaterThan and GreaterThanOrEqual commute the inputs
-          // and use the LessThan and LessThanOrEqual operators.
-          CheckIsConvertedToNumber(p1, r->InputAt(0));
-          CheckIsConvertedToNumber(p0, r->InputAt(1));
-        } else {
-          CheckIsConvertedToNumber(p0, r->InputAt(0));
-          CheckIsConvertedToNumber(p1, r->InputAt(1));
-        }
-      }
+    R.CheckPureBinop(ops[k + 1], r);
+    if (k >= 4) {
+      // GreaterThan and GreaterThanOrEqual commute the inputs
+      // and use the LessThan and LessThanOrEqual operators.
+      CheckIsConvertedToNumber(p1, r->InputAt(0));
+      CheckIsConvertedToNumber(p0, r->InputAt(1));
+    } else {
+      CheckIsConvertedToNumber(p0, r->InputAt(0));
+      CheckIsConvertedToNumber(p1, r->InputAt(1));
     }
   }
 }
@@ -754,36 +742,6 @@ TEST(MixedComparison1) {
 }
 
 
-TEST(ObjectComparison) {
-  JSTypedLoweringTester R;
-
-  Node* p0 = R.Parameter(Type::Number(), 0);
-  Node* p1 = R.Parameter(Type::Object(), 1);
-
-  Node* cmp = R.Binop(R.javascript.LessThan(), p0, p1);
-  Node* effect_use = R.UseForEffect(cmp);
-
-  R.CheckEffectInput(R.start(), cmp);
-  R.CheckEffectInput(cmp, effect_use);
-
-  Node* r = R.reduce(cmp);
-
-  R.CheckPureBinop(R.simplified.NumberLessThan(), r);
-
-  Node* i0 = r->InputAt(0);
-  Node* i1 = r->InputAt(1);
-
-  CHECK_EQ(p0, i0);
-  CHECK_NE(p1, i1);
-  CHECK_EQ(IrOpcode::kParameter, i0->opcode());
-  CHECK_EQ(IrOpcode::kJSToNumber, i1->opcode());
-
-  // Check effect chain is correct.
-  R.CheckEffectInput(R.start(), i1);
-  R.CheckEffectInput(i1, effect_use);
-}
-
-
 TEST(UnaryNot) {
   JSTypedLoweringTester R;
   const Operator* opnot = R.javascript.UnaryNot();
@@ -1031,7 +989,7 @@ TEST(OrderNumberBinopEffects1) {
   };
 
   for (size_t j = 0; j < arraysize(ops); j += 2) {
-    BinopEffectsTester B(ops[j], Type::Object(), Type::String());
+    BinopEffectsTester B(ops[j], Type::String(), Type::String());
     CHECK_EQ(ops[j + 1]->opcode(), B.result->op()->opcode());
 
     Node* i0 = B.CheckConvertedInput(IrOpcode::kJSToNumber, 0, true);
@@ -1167,7 +1125,7 @@ TEST(Int32BinopEffects) {
 
   for (int j = 0; j < R.kNumberOps; j += 2) {
     bool signed_left = R.signedness[j], signed_right = R.signedness[j + 1];
-    BinopEffectsTester B(R.ops[j], Type::Number(), Type::Object());
+    BinopEffectsTester B(R.ops[j], Type::Number(), Type::Primitive());
 
     B.R.CheckPureBinop(B.result->opcode(), B.result);
 
@@ -1184,7 +1142,7 @@ TEST(Int32BinopEffects) {
 
   for (int j = 0; j < R.kNumberOps; j += 2) {
     bool signed_left = R.signedness[j], signed_right = R.signedness[j + 1];
-    BinopEffectsTester B(R.ops[j], Type::Object(), Type::Number());
+    BinopEffectsTester B(R.ops[j], Type::Primitive(), Type::Number());
 
     B.R.CheckPureBinop(B.result->opcode(), B.result);
 
@@ -1201,7 +1159,7 @@ TEST(Int32BinopEffects) {
 
   for (int j = 0; j < R.kNumberOps; j += 2) {
     bool signed_left = R.signedness[j], signed_right = R.signedness[j + 1];
-    BinopEffectsTester B(R.ops[j], Type::Object(), Type::Object());
+    BinopEffectsTester B(R.ops[j], Type::Primitive(), Type::Primitive());
 
     B.R.CheckPureBinop(B.result->opcode(), B.result);