Reviewers: jarin,

Message:
PTAL

Description:
[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.

Also fix a bug in simplified lowering; according to the spec, the input
to Word32Shr has to be uint32.

TEST=cctest,mjsunit
[email protected]

Please review this at https://codereview.chromium.org/649543004/

Base URL: https://v8.googlecode.com/svn/branches/bleeding_edge

Affected files (+70, -9 lines):
  M src/compiler/js-typed-lowering.cc
  M src/compiler/simplified-lowering.cc


Index: src/compiler/js-typed-lowering.cc
diff --git a/src/compiler/js-typed-lowering.cc b/src/compiler/js-typed-lowering.cc index ad081968c9c522267703327b7199092ae337327f..fdd5a5673e75c65d27fa893910e4c749f1b859ce 100644
--- a/src/compiler/js-typed-lowering.cc
+++ b/src/compiler/js-typed-lowering.cc
@@ -227,12 +227,17 @@ Reduction JSTypedLowering::ReduceJSAdd(Node* node) {
     // JSAdd(x:number, y:number) => NumberAdd(x, y)
     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)) {
     // JSAdd(x:-string, y:-string) => NumberAdd(ToNumber(x), ToNumber(y))
     r.ConvertInputsToNumber();
     return r.ChangeToPureOperator(simplified()->NumberAdd());
   }
+#endif
 #if 0
   // TODO(turbofan): Lowering of StringAdd is disabled for now because:
// a) The inserted ToString operation screws up valueOf vs. toString order.
@@ -253,6 +258,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 +273,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 +283,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,6 +332,45 @@ Reduction JSTypedLowering::ReduceJSComparison(Node* node) {
     }
     return r.ChangeToPureOperator(stringOp);
   }
+  if (r.BothInputsAre(Type::Number())) {
+    const Operator* less_than;
+    const Operator* less_than_or_equal;
+    if (r.BothInputsAre(Type::Unsigned32())) {
+      less_than = machine()->Uint32LessThan();
+      less_than_or_equal = machine()->Uint32LessThanOrEqual();
+    } else if (r.BothInputsAre(Type::Signed32())) {
+      less_than = machine()->Int32LessThan();
+      less_than_or_equal = machine()->Int32LessThanOrEqual();
+    } else {
+      // TODO(turbofan): mixed signed/unsigned int32 comparisons.
+      less_than = simplified()->NumberLessThan();
+      less_than_or_equal = simplified()->NumberLessThanOrEqual();
+    }
+    const Operator* comparison;
+    switch (node->opcode()) {
+      case IrOpcode::kJSLessThan:
+        comparison = less_than;
+        break;
+      case IrOpcode::kJSGreaterThan:
+        comparison = less_than;
+        r.SwapInputs();  // a > b => b < a
+        break;
+      case IrOpcode::kJSLessThanOrEqual:
+        comparison = less_than_or_equal;
+        break;
+      case IrOpcode::kJSGreaterThanOrEqual:
+        comparison = less_than_or_equal;
+        r.SwapInputs();  // a >= b => b <= a
+        break;
+      default:
+        return NoChange();
+    }
+    return r.ChangeToPureOperator(comparison);
+  }
+#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.
@@ -349,6 +409,7 @@ Reduction JSTypedLowering::ReduceJSComparison(Node* node) {
     }
     return r.ChangeToPureOperator(comparison);
   }
+#endif
   // TODO(turbofan): relax/remove effects of this operator in other cases.
   return NoChange();  // Keep a generic comparison.
 }
Index: src/compiler/simplified-lowering.cc
diff --git a/src/compiler/simplified-lowering.cc b/src/compiler/simplified-lowering.cc index 7a0f32493239279bb25136f23e1fbd7c76a1c4b3..8db430e09403b11532841ec401dbe4550f0d6545 100644
--- a/src/compiler/simplified-lowering.cc
+++ b/src/compiler/simplified-lowering.cc
@@ -726,7 +726,7 @@ class RepresentationSelector {
       }
       case IrOpcode::kWord32Shr:
         // We output unsigned int32 for shift right because JavaScript.
-        return VisitBinop(node, kRepWord32, kRepWord32 | kTypeUint32);
+        return VisitBinop(node, kMachUint32, kMachUint32);
       case IrOpcode::kWord32And:
       case IrOpcode::kWord32Or:
       case IrOpcode::kWord32Xor:


--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
--- You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to [email protected].
For more options, visit https://groups.google.com/d/optout.

Reply via email to