From 7e540e6b8ac4673d3f23cbfd149ffbbabb92cf5d Mon Sep 17 00:00:00 2001 From: "bmeurer@chromium.org" Date: Tue, 3 Dec 2013 11:24:56 +0000 Subject: [PATCH] Revert "Use constant types to represent the fixed right arg of a MOD." This reverts commit r18228 for crashing on Windows. TBR=mvstanton@chromium.org Review URL: https://codereview.chromium.org/101663002 git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@18229 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- src/ast.h | 6 ++++ src/code-stubs-hydrogen.cc | 13 +++++---- src/hydrogen.cc | 60 ++++++++++++++++++++------------------ src/hydrogen.h | 3 +- src/ic.cc | 16 +--------- src/ic.h | 5 +++- src/type-info.cc | 3 ++ src/type-info.h | 1 + src/typing.cc | 4 ++- 9 files changed, 59 insertions(+), 52 deletions(-) diff --git a/src/ast.h b/src/ast.h index 1fe593e935..0bbb90452a 100644 --- a/src/ast.h +++ b/src/ast.h @@ -1952,6 +1952,8 @@ class BinaryOperation V8_FINAL : public Expression { BailoutId RightId() const { return right_id_; } TypeFeedbackId BinaryOperationFeedbackId() const { return reuse(id()); } + Maybe fixed_right_arg() const { return fixed_right_arg_; } + void set_fixed_right_arg(Maybe arg) { fixed_right_arg_ = arg; } virtual void RecordToBooleanTypeFeedback( TypeFeedbackOracle* oracle) V8_OVERRIDE; @@ -1975,6 +1977,10 @@ class BinaryOperation V8_FINAL : public Expression { Expression* left_; Expression* right_; + // TODO(rossberg): the fixed arg should probably be represented as a Constant + // type for the RHS. + Maybe fixed_right_arg_; + // The short-circuit logical operations need an AST ID for their // right-hand subexpression. const BailoutId right_id_; diff --git a/src/code-stubs-hydrogen.cc b/src/code-stubs-hydrogen.cc index fac4cf3edc..e52ec65c4b 100644 --- a/src/code-stubs-hydrogen.cc +++ b/src/code-stubs-hydrogen.cc @@ -922,13 +922,14 @@ HValue* CodeStubGraphBuilder::BuildCodeInitializedStub() { Push(BuildBinaryOperation( state.op(), left, right, handle(Type::String(), isolate()), right_type, - result_type)); + result_type, state.fixed_right_arg())); } if_leftisstring.Else(); { Push(BuildBinaryOperation( state.op(), left, right, - left_type, right_type, result_type)); + left_type, right_type, result_type, + state.fixed_right_arg())); } if_leftisstring.End(); result = Pop(); @@ -940,13 +941,14 @@ HValue* CodeStubGraphBuilder::BuildCodeInitializedStub() { Push(BuildBinaryOperation( state.op(), left, right, left_type, handle(Type::String(), isolate()), - result_type)); + result_type, state.fixed_right_arg())); } if_rightisstring.Else(); { Push(BuildBinaryOperation( state.op(), left, right, - left_type, right_type, result_type)); + left_type, right_type, result_type, + state.fixed_right_arg())); } if_rightisstring.End(); result = Pop(); @@ -954,7 +956,8 @@ HValue* CodeStubGraphBuilder::BuildCodeInitializedStub() { } else { result = BuildBinaryOperation( state.op(), left, right, - left_type, right_type, result_type); + left_type, right_type, result_type, + state.fixed_right_arg()); } // If we encounter a generic argument, the number conversion is diff --git a/src/hydrogen.cc b/src/hydrogen.cc index 534f6126b6..900e07ecdf 100644 --- a/src/hydrogen.cc +++ b/src/hydrogen.cc @@ -8732,9 +8732,11 @@ HValue* HOptimizedGraphBuilder::BuildBinaryOperation( Handle left_type = expr->left()->bounds().lower; Handle right_type = expr->right()->bounds().lower; Handle result_type = expr->bounds().lower; + Maybe fixed_right_arg = expr->fixed_right_arg(); HValue* result = HGraphBuilder::BuildBinaryOperation( - expr->op(), left, right, left_type, right_type, result_type); + expr->op(), left, right, left_type, right_type, + result_type, fixed_right_arg); // Add a simulate after instructions with observable side effects, and // after phis, which are the result of BuildBinaryOperation when we // inlined some complex subgraph. @@ -8753,45 +8755,34 @@ HValue* HGraphBuilder::BuildBinaryOperation( HValue* right, Handle left_type, Handle right_type, - Handle result_type) { + Handle result_type, + Maybe fixed_right_arg) { Representation left_rep = Representation::FromType(left_type); Representation right_rep = Representation::FromType(right_type); + bool maybe_string_add = op == Token::ADD && + (left_type->Maybe(Type::String()) || + right_type->Maybe(Type::String())); + if (left_type->Is(Type::None())) { Add("Insufficient type feedback for LHS of binary operation", Deoptimizer::SOFT); // TODO(rossberg): we should be able to get rid of non-continuous // defaults. left_type = handle(Type::Any(), isolate()); - } else if (left_type->IsConstant()) { - HConstant* c_left = Add(left_type->AsConstant()); - IfBuilder if_same(this); - if (c_left->HasDoubleValue()) { - if_same.If(left, c_left, Token::EQ); - } else { - if_same.If(left, c_left); - } - if_same.Then(); - if_same.ElseDeopt("Unexpected LHS of binary operation"); - left = c_left; + } else { + if (!maybe_string_add) left = TruncateToNumber(left, &left_type); + left_rep = Representation::FromType(left_type); } if (right_type->Is(Type::None())) { Add("Insufficient type feedback for RHS of binary operation", Deoptimizer::SOFT); right_type = handle(Type::Any(), isolate()); - } else if (right_type->IsConstant()) { - HConstant* c_right = Add(right_type->AsConstant()); - IfBuilder if_same(this); - if (c_right->HasDoubleValue()) { - if_same.If(right, c_right, Token::EQ); - } else { - if_same.If(right, c_right); - } - if_same.Then(); - if_same.ElseDeopt("Unexpected RHS of binary operation"); - right = c_right; + } else { + if (!maybe_string_add) right = TruncateToNumber(right, &right_type); + right_rep = Representation::FromType(right_type); } // Special case for string addition here. @@ -8834,11 +8825,6 @@ HValue* HGraphBuilder::BuildBinaryOperation( return AddUncasted(left, right, STRING_ADD_CHECK_NONE); } - left = TruncateToNumber(left, &left_type); - left_rep = Representation::FromType(left_type); - right = TruncateToNumber(right, &right_type); - right_rep = Representation::FromType(right_type); - if (graph()->info()->IsStub()) { left = EnforceNumberType(left, left_type); right = EnforceNumberType(right, right_type); @@ -8870,6 +8856,22 @@ HValue* HGraphBuilder::BuildBinaryOperation( instr = AddUncasted(left, right); break; case Token::MOD: { + if (fixed_right_arg.has_value) { + if (right->IsConstant()) { + HConstant* c_right = HConstant::cast(right); + if (c_right->HasInteger32Value()) { + ASSERT_EQ(fixed_right_arg.value, c_right->Integer32Value()); + } + } else { + HConstant* fixed_right = Add( + static_cast(fixed_right_arg.value)); + IfBuilder if_same(this); + if_same.If(right, fixed_right, Token::EQ); + if_same.Then(); + if_same.ElseDeopt("Unexpected RHS of binary operation"); + right = fixed_right; + } + } instr = AddUncasted(left, right); break; } diff --git a/src/hydrogen.h b/src/hydrogen.h index 409935d38b..61e98b2b0c 100644 --- a/src/hydrogen.h +++ b/src/hydrogen.h @@ -1351,7 +1351,8 @@ class HGraphBuilder { HValue* right, Handle left_type, Handle right_type, - Handle result_type); + Handle result_type, + Maybe fixed_right_arg); HLoadNamedField* AddLoadFixedArrayLength(HValue *object); diff --git a/src/ic.cc b/src/ic.cc index b92f219dc4..08df2261fd 100644 --- a/src/ic.cc +++ b/src/ic.cc @@ -2329,10 +2329,7 @@ BinaryOpIC::State::State(ExtraICState extra_ic_state) { 1 << FixedRightArgValueField::decode(extra_ic_state)); left_kind_ = LeftKindField::decode(extra_ic_state); if (fixed_right_arg_.has_value) { - // We have only 4 bits to encode the log2 of the fixed right arg, so the - // max value is 2^(2^4), which is always a SMI. - ASSERT(Smi::IsValid(fixed_right_arg_.value)); - right_kind_ = SMI; + right_kind_ = Smi::IsValid(fixed_right_arg_.value) ? SMI : INT32; } else { right_kind_ = RightKindField::decode(extra_ic_state); } @@ -2585,17 +2582,6 @@ void BinaryOpIC::State::GenerateAheadOfTime( } -Handle BinaryOpIC::State::GetRightType(Isolate* isolate) const { - if (fixed_right_arg_.has_value) { - Handle value = handle(Smi::FromInt(fixed_right_arg_.value), isolate); - Handle type = handle(Type::Constant(value, isolate), isolate); - ASSERT(type->Is(KindToType(right_kind_, isolate))); - return type; - } - return KindToType(right_kind_, isolate); -} - - Handle BinaryOpIC::State::GetResultType(Isolate* isolate) const { Kind result_kind = result_kind_; if (HasSideEffects()) { diff --git a/src/ic.h b/src/ic.h index 3d8b7fd33e..fa7ed6dbc1 100644 --- a/src/ic.h +++ b/src/ic.h @@ -866,11 +866,14 @@ class BinaryOpIC: public IC { Token::Value op() const { return op_; } OverwriteMode mode() const { return mode_; } + Maybe fixed_right_arg() const { return fixed_right_arg_; } Handle GetLeftType(Isolate* isolate) const { return KindToType(left_kind_, isolate); } - Handle GetRightType(Isolate* isolate) const; + Handle GetRightType(Isolate* isolate) const { + return KindToType(right_kind_, isolate); + } Handle GetResultType(Isolate* isolate) const; void Print(StringStream* stream) const; diff --git a/src/type-info.cc b/src/type-info.cc index f445544a6d..6e3a4f6b7a 100644 --- a/src/type-info.cc +++ b/src/type-info.cc @@ -407,6 +407,7 @@ void TypeFeedbackOracle::BinaryType(TypeFeedbackId id, Handle* left, Handle* right, Handle* result, + Maybe* fixed_right_arg, Token::Value op) { Handle object = GetInfo(id); if (!object->IsCode()) { @@ -415,6 +416,7 @@ void TypeFeedbackOracle::BinaryType(TypeFeedbackId id, ASSERT(op < BinaryOpIC::State::FIRST_TOKEN || op > BinaryOpIC::State::LAST_TOKEN); *left = *right = *result = handle(Type::None(), isolate_); + *fixed_right_arg = Maybe(); return; } Handle code = Handle::cast(object); @@ -425,6 +427,7 @@ void TypeFeedbackOracle::BinaryType(TypeFeedbackId id, *left = state.GetLeftType(isolate()); *right = state.GetRightType(isolate()); *result = state.GetResultType(isolate()); + *fixed_right_arg = state.fixed_right_arg(); } diff --git a/src/type-info.h b/src/type-info.h index 5a275b313f..a0d3215844 100644 --- a/src/type-info.h +++ b/src/type-info.h @@ -312,6 +312,7 @@ class TypeFeedbackOracle: public ZoneObject { Handle* left, Handle* right, Handle* result, + Maybe* fixed_right_arg, Token::Value operation); void CompareType(TypeFeedbackId id, diff --git a/src/typing.cc b/src/typing.cc index 582b404a29..8487c05eb4 100644 --- a/src/typing.cc +++ b/src/typing.cc @@ -571,11 +571,13 @@ void AstTyper::VisitCountOperation(CountOperation* expr) { void AstTyper::VisitBinaryOperation(BinaryOperation* expr) { // Collect type feedback. Handle type, left_type, right_type; + Maybe fixed_right_arg; oracle()->BinaryType(expr->BinaryOperationFeedbackId(), - &left_type, &right_type, &type, expr->op()); + &left_type, &right_type, &type, &fixed_right_arg, expr->op()); NarrowLowerType(expr, type); NarrowLowerType(expr->left(), left_type); NarrowLowerType(expr->right(), right_type); + expr->set_fixed_right_arg(fixed_right_arg); if (expr->op() == Token::OR || expr->op() == Token::AND) { expr->left()->RecordToBooleanTypeFeedback(oracle()); }