Avoid tagged values for Instructions that truncate the operands with ToNumber.
authorolivf@chromium.org <olivf@chromium.org@ce2b1a6d-e550-0410-aec6-3dcde31c8c00>
Tue, 23 Jul 2013 09:13:59 +0000 (09:13 +0000)
committerolivf@chromium.org <olivf@chromium.org@ce2b1a6d-e550-0410-aec6-3dcde31c8c00>
Tue, 23 Jul 2013 09:13:59 +0000 (09:13 +0000)
I case the ToNumber is applied to a non numeric value but its not observable (some constants and oddballs) we should already do it in hydrogen...

BUG=
R=verwaest@chromium.org

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

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

src/hydrogen-instructions.cc
src/hydrogen-instructions.h
src/hydrogen-representation-changes.cc
src/hydrogen.cc
src/hydrogen.h

index 16476a9..dfa5553 100644 (file)
@@ -2310,20 +2310,38 @@ HConstant* HConstant::CopyToRepresentation(Representation r, Zone* zone) const {
 }
 
 
-HConstant* HConstant::CopyToTruncatedInt32(Zone* zone) const {
+Maybe<HConstant*> HConstant::CopyToTruncatedInt32(Zone* zone) {
+  HConstant* res = NULL;
   if (has_int32_value_) {
-    return new(zone) HConstant(int32_value_,
-                               Representation::Integer32(),
-                               is_not_in_new_space_,
-                               handle_);
+    res = new(zone) HConstant(int32_value_,
+                              Representation::Integer32(),
+                              is_not_in_new_space_,
+                              handle_);
+  } else if (has_double_value_) {
+    res = new(zone) HConstant(DoubleToInt32(double_value_),
+                              Representation::Integer32(),
+                              is_not_in_new_space_,
+                              handle_);
+  } else {
+    ASSERT(!HasNumberValue());
+    Maybe<HConstant*> number = CopyToTruncatedNumber(zone);
+    if (number.has_value) return number.value->CopyToTruncatedInt32(zone);
   }
-  if (has_double_value_) {
-    return new(zone) HConstant(DoubleToInt32(double_value_),
-                               Representation::Integer32(),
-                               is_not_in_new_space_,
-                               handle_);
+  return Maybe<HConstant*>(res != NULL, res);
+}
+
+
+Maybe<HConstant*> HConstant::CopyToTruncatedNumber(Zone* zone) {
+  HConstant* res = NULL;
+  if (handle()->IsBoolean()) {
+    res = handle()->BooleanValue() ?
+      new(zone) HConstant(1) : new(zone) HConstant(0);
+  } else if (handle()->IsUndefined()) {
+    res = new(zone) HConstant(OS::nan_value());
+  } else if (handle()->IsNull()) {
+    res = new(zone) HConstant(0);
   }
-  return NULL;
+  return Maybe<HConstant*>(res != NULL, res);
 }
 
 
index 46401e3..83fe181 100644 (file)
@@ -408,6 +408,11 @@ class HType {
     return ((type_ & kString) == kString);
   }
 
+  bool IsNonString() const {
+    return IsTaggedPrimitive() || IsSmi() || IsHeapNumber() ||
+        IsBoolean() || IsJSArray();
+  }
+
   bool IsBoolean() const {
     ASSERT(type_ != kUninitialized);
     return ((type_ & kBoolean) == kBoolean);
@@ -3351,7 +3356,8 @@ class HConstant: public HTemplateInstruction<0> {
   virtual HType CalculateInferredType();
   bool IsInteger() { return handle()->IsSmi(); }
   HConstant* CopyToRepresentation(Representation r, Zone* zone) const;
-  HConstant* CopyToTruncatedInt32(Zone* zone) const;
+  Maybe<HConstant*> CopyToTruncatedInt32(Zone* zone);
+  Maybe<HConstant*> CopyToTruncatedNumber(Zone* zone);
   bool HasInteger32Value() const { return has_int32_value_; }
   int32_t Integer32Value() const {
     ASSERT(HasInteger32Value());
index e8f0140..45d35b2 100644 (file)
@@ -51,9 +51,12 @@ void HRepresentationChangesPhase::InsertRepresentationChangeForUse(
   if (value->IsConstant()) {
     HConstant* constant = HConstant::cast(value);
     // Try to create a new copy of the constant with the new representation.
-    new_value = (is_truncating && to.IsInteger32())
-        ? constant->CopyToTruncatedInt32(graph()->zone())
-        : constant->CopyToRepresentation(to, graph()->zone());
+    if (is_truncating && to.IsInteger32()) {
+      Maybe<HConstant*> res = constant->CopyToTruncatedInt32(graph()->zone());
+      if (res.has_value) new_value = res.value;
+    } else {
+      new_value = constant->CopyToRepresentation(to, graph()->zone());
+    }
   }
 
   if (new_value == NULL) {
index 57220e0..fc3d145 100644 (file)
@@ -3112,7 +3112,7 @@ void HGraph::RestoreActualValues() {
 }
 
 
-void HOptimizedGraphBuilder::PushAndAdd(HInstruction* instr) {
+void HGraphBuilder::PushAndAdd(HInstruction* instr) {
   Push(instr);
   AddInstruction(instr);
 }
@@ -7460,8 +7460,8 @@ void HOptimizedGraphBuilder::VisitTypeof(UnaryOperation* expr) {
 
 void HOptimizedGraphBuilder::VisitSub(UnaryOperation* expr) {
   CHECK_ALIVE(VisitForValue(expr->expression()));
-  HValue* value = Pop();
   Handle<Type> operand_type = expr->expression()->bounds().lower;
+  HValue* value = TruncateToNumber(Pop(), &operand_type);
   HInstruction* instr = BuildUnaryMathOp(value, operand_type, Token::SUB);
   return ast_context()->ReturnInstruction(instr, expr->id());
 }
@@ -7469,8 +7469,8 @@ void HOptimizedGraphBuilder::VisitSub(UnaryOperation* expr) {
 
 void HOptimizedGraphBuilder::VisitBitNot(UnaryOperation* expr) {
   CHECK_ALIVE(VisitForValue(expr->expression()));
-  HValue* value = Pop();
   Handle<Type> operand_type = expr->expression()->bounds().lower;
+  HValue* value = TruncateToNumber(Pop(), &operand_type);
   HInstruction* instr = BuildUnaryMathOp(value, operand_type, Token::BIT_NOT);
   return ast_context()->ReturnInstruction(instr, expr->id());
 }
@@ -7800,6 +7800,40 @@ bool CanBeZero(HValue* right) {
 }
 
 
+HValue* HGraphBuilder::TruncateToNumber(HValue* value, Handle<Type>* expected) {
+  if (value->IsConstant()) {
+    HConstant* constant = HConstant::cast(value);
+    Maybe<HConstant*> number = constant->CopyToTruncatedNumber(zone());
+    if (number.has_value) {
+      *expected = handle(Type::Number(), isolate());
+      return AddInstruction(number.value);
+    }
+    return value;
+  }
+
+  Handle<Type> expected_type = *expected;
+  Representation rep = Representation::FromType(expected_type);
+  if (!rep.IsTagged()) return value;
+
+  // If our type feedback suggests that we can non-observably truncate to number
+  // we introduce the appropriate check here. This avoids 'value' having a
+  // tagged representation later on.
+  if (expected_type->Is(Type::Oddball())) {
+    // TODO(olivf) The BinaryOpStub only records undefined. It might pay off to
+    // also record booleans and convert them to 0/1 here.
+    IfBuilder if_nan(this);
+    if_nan.If<HCompareObjectEqAndBranch>(value,
+        graph()->GetConstantUndefined());
+    if_nan.Then();
+    if_nan.ElseDeopt();
+    if_nan.End();
+    return Add<HConstant>(OS::nan_value(), Representation::Double());
+  }
+
+  return value;
+}
+
+
 HInstruction* HOptimizedGraphBuilder::BuildBinaryOperation(
     BinaryOperation* expr,
     HValue* left,
@@ -7813,6 +7847,14 @@ HInstruction* HOptimizedGraphBuilder::BuildBinaryOperation(
   Representation right_rep = Representation::FromType(right_type);
   Representation result_rep = Representation::FromType(result_type);
 
+  if (expr->op() != Token::ADD ||
+      (left->type().IsNonString() && right->type().IsNonString())) {
+    // For addition we can only truncate the arguments to number if we can
+    // prove that we will not end up in string concatenation mode.
+    left = TruncateToNumber(left, &left_type);
+    right = TruncateToNumber(right, &right_type);
+  }
+
   if (left_type->Is(Type::None())) {
     AddSoftDeoptimize();
     // TODO(rossberg): we should be able to get rid of non-continuous defaults.
index 797b444..9ce630b 100644 (file)
@@ -1118,6 +1118,10 @@ class HGraphBuilder {
 
   HValue* AddLoadJSBuiltin(Builtins::JavaScript builtin, HValue* context);
 
+  HValue* TruncateToNumber(HValue* value, Handle<Type>* expected);
+
+  void PushAndAdd(HInstruction* instr);
+
   enum SoftDeoptimizeMode {
     MUST_EMIT_SOFT_DEOPT,
     CAN_OMIT_SOFT_DEOPT
@@ -1666,8 +1670,6 @@ class HOptimizedGraphBuilder: public HGraphBuilder, public AstVisitor {
   // Visit a list of expressions from left to right, each in a value context.
   void VisitExpressions(ZoneList<Expression*>* exprs);
 
-  void PushAndAdd(HInstruction* instr);
-
   // Remove the arguments from the bailout environment and emit instructions
   // to push them as outgoing parameters.
   template <class Instruction> HInstruction* PreProcessCall(Instruction* call);