From b8d7bee4a361317b3826e801515a0e833067b9fe Mon Sep 17 00:00:00 2001 From: "olivf@chromium.org" Date: Tue, 23 Jul 2013 09:13:59 +0000 Subject: [PATCH] Avoid tagged values for Instructions that truncate the operands with ToNumber. 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 | 40 ++++++++++++++++++++-------- src/hydrogen-instructions.h | 8 +++++- src/hydrogen-representation-changes.cc | 9 ++++--- src/hydrogen.cc | 48 +++++++++++++++++++++++++++++++--- src/hydrogen.h | 6 +++-- 5 files changed, 91 insertions(+), 20 deletions(-) diff --git a/src/hydrogen-instructions.cc b/src/hydrogen-instructions.cc index 16476a9..dfa5553 100644 --- a/src/hydrogen-instructions.cc +++ b/src/hydrogen-instructions.cc @@ -2310,20 +2310,38 @@ HConstant* HConstant::CopyToRepresentation(Representation r, Zone* zone) const { } -HConstant* HConstant::CopyToTruncatedInt32(Zone* zone) const { +Maybe 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 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(res != NULL, res); +} + + +Maybe 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(res != NULL, res); } diff --git a/src/hydrogen-instructions.h b/src/hydrogen-instructions.h index 46401e3..83fe181 100644 --- a/src/hydrogen-instructions.h +++ b/src/hydrogen-instructions.h @@ -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 CopyToTruncatedInt32(Zone* zone); + Maybe CopyToTruncatedNumber(Zone* zone); bool HasInteger32Value() const { return has_int32_value_; } int32_t Integer32Value() const { ASSERT(HasInteger32Value()); diff --git a/src/hydrogen-representation-changes.cc b/src/hydrogen-representation-changes.cc index e8f0140..45d35b2 100644 --- a/src/hydrogen-representation-changes.cc +++ b/src/hydrogen-representation-changes.cc @@ -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 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) { diff --git a/src/hydrogen.cc b/src/hydrogen.cc index 57220e0..fc3d145 100644 --- a/src/hydrogen.cc +++ b/src/hydrogen.cc @@ -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 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 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* expected) { + if (value->IsConstant()) { + HConstant* constant = HConstant::cast(value); + Maybe number = constant->CopyToTruncatedNumber(zone()); + if (number.has_value) { + *expected = handle(Type::Number(), isolate()); + return AddInstruction(number.value); + } + return value; + } + + Handle 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(value, + graph()->GetConstantUndefined()); + if_nan.Then(); + if_nan.ElseDeopt(); + if_nan.End(); + return Add(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. diff --git a/src/hydrogen.h b/src/hydrogen.h index 797b444..9ce630b 100644 --- a/src/hydrogen.h +++ b/src/hydrogen.h @@ -1118,6 +1118,10 @@ class HGraphBuilder { HValue* AddLoadJSBuiltin(Builtins::JavaScript builtin, HValue* context); + HValue* TruncateToNumber(HValue* value, Handle* 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* exprs); - void PushAndAdd(HInstruction* instr); - // Remove the arguments from the bailout environment and emit instructions // to push them as outgoing parameters. template HInstruction* PreProcessCall(Instruction* call); -- 2.7.4