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
This commit is contained in:
bmeurer@chromium.org 2013-12-03 11:24:56 +00:00
parent 5c6b9500aa
commit 7e540e6b8a
9 changed files with 59 additions and 52 deletions

View File

@ -1952,6 +1952,8 @@ class BinaryOperation V8_FINAL : public Expression {
BailoutId RightId() const { return right_id_; } BailoutId RightId() const { return right_id_; }
TypeFeedbackId BinaryOperationFeedbackId() const { return reuse(id()); } TypeFeedbackId BinaryOperationFeedbackId() const { return reuse(id()); }
Maybe<int> fixed_right_arg() const { return fixed_right_arg_; }
void set_fixed_right_arg(Maybe<int> arg) { fixed_right_arg_ = arg; }
virtual void RecordToBooleanTypeFeedback( virtual void RecordToBooleanTypeFeedback(
TypeFeedbackOracle* oracle) V8_OVERRIDE; TypeFeedbackOracle* oracle) V8_OVERRIDE;
@ -1975,6 +1977,10 @@ class BinaryOperation V8_FINAL : public Expression {
Expression* left_; Expression* left_;
Expression* right_; Expression* right_;
// TODO(rossberg): the fixed arg should probably be represented as a Constant
// type for the RHS.
Maybe<int> fixed_right_arg_;
// The short-circuit logical operations need an AST ID for their // The short-circuit logical operations need an AST ID for their
// right-hand subexpression. // right-hand subexpression.
const BailoutId right_id_; const BailoutId right_id_;

View File

@ -922,13 +922,14 @@ HValue* CodeStubGraphBuilder<BinaryOpICStub>::BuildCodeInitializedStub() {
Push(BuildBinaryOperation( Push(BuildBinaryOperation(
state.op(), left, right, state.op(), left, right,
handle(Type::String(), isolate()), right_type, handle(Type::String(), isolate()), right_type,
result_type)); result_type, state.fixed_right_arg()));
} }
if_leftisstring.Else(); if_leftisstring.Else();
{ {
Push(BuildBinaryOperation( Push(BuildBinaryOperation(
state.op(), left, right, state.op(), left, right,
left_type, right_type, result_type)); left_type, right_type, result_type,
state.fixed_right_arg()));
} }
if_leftisstring.End(); if_leftisstring.End();
result = Pop(); result = Pop();
@ -940,13 +941,14 @@ HValue* CodeStubGraphBuilder<BinaryOpICStub>::BuildCodeInitializedStub() {
Push(BuildBinaryOperation( Push(BuildBinaryOperation(
state.op(), left, right, state.op(), left, right,
left_type, handle(Type::String(), isolate()), left_type, handle(Type::String(), isolate()),
result_type)); result_type, state.fixed_right_arg()));
} }
if_rightisstring.Else(); if_rightisstring.Else();
{ {
Push(BuildBinaryOperation( Push(BuildBinaryOperation(
state.op(), left, right, state.op(), left, right,
left_type, right_type, result_type)); left_type, right_type, result_type,
state.fixed_right_arg()));
} }
if_rightisstring.End(); if_rightisstring.End();
result = Pop(); result = Pop();
@ -954,7 +956,8 @@ HValue* CodeStubGraphBuilder<BinaryOpICStub>::BuildCodeInitializedStub() {
} else { } else {
result = BuildBinaryOperation( result = BuildBinaryOperation(
state.op(), left, right, 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 // If we encounter a generic argument, the number conversion is

View File

@ -8732,9 +8732,11 @@ HValue* HOptimizedGraphBuilder::BuildBinaryOperation(
Handle<Type> left_type = expr->left()->bounds().lower; Handle<Type> left_type = expr->left()->bounds().lower;
Handle<Type> right_type = expr->right()->bounds().lower; Handle<Type> right_type = expr->right()->bounds().lower;
Handle<Type> result_type = expr->bounds().lower; Handle<Type> result_type = expr->bounds().lower;
Maybe<int> fixed_right_arg = expr->fixed_right_arg();
HValue* result = HGraphBuilder::BuildBinaryOperation( 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 // Add a simulate after instructions with observable side effects, and
// after phis, which are the result of BuildBinaryOperation when we // after phis, which are the result of BuildBinaryOperation when we
// inlined some complex subgraph. // inlined some complex subgraph.
@ -8753,45 +8755,34 @@ HValue* HGraphBuilder::BuildBinaryOperation(
HValue* right, HValue* right,
Handle<Type> left_type, Handle<Type> left_type,
Handle<Type> right_type, Handle<Type> right_type,
Handle<Type> result_type) { Handle<Type> result_type,
Maybe<int> fixed_right_arg) {
Representation left_rep = Representation::FromType(left_type); Representation left_rep = Representation::FromType(left_type);
Representation right_rep = Representation::FromType(right_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())) { if (left_type->Is(Type::None())) {
Add<HDeoptimize>("Insufficient type feedback for LHS of binary operation", Add<HDeoptimize>("Insufficient type feedback for LHS of binary operation",
Deoptimizer::SOFT); Deoptimizer::SOFT);
// TODO(rossberg): we should be able to get rid of non-continuous // TODO(rossberg): we should be able to get rid of non-continuous
// defaults. // defaults.
left_type = handle(Type::Any(), isolate()); left_type = handle(Type::Any(), isolate());
} else if (left_type->IsConstant()) { } else {
HConstant* c_left = Add<HConstant>(left_type->AsConstant()); if (!maybe_string_add) left = TruncateToNumber(left, &left_type);
IfBuilder if_same(this); left_rep = Representation::FromType(left_type);
if (c_left->HasDoubleValue()) {
if_same.If<HCompareNumericAndBranch>(left, c_left, Token::EQ);
} else {
if_same.If<HCompareObjectEqAndBranch>(left, c_left);
}
if_same.Then();
if_same.ElseDeopt("Unexpected LHS of binary operation");
left = c_left;
} }
if (right_type->Is(Type::None())) { if (right_type->Is(Type::None())) {
Add<HDeoptimize>("Insufficient type feedback for RHS of binary operation", Add<HDeoptimize>("Insufficient type feedback for RHS of binary operation",
Deoptimizer::SOFT); Deoptimizer::SOFT);
right_type = handle(Type::Any(), isolate()); right_type = handle(Type::Any(), isolate());
} else if (right_type->IsConstant()) { } else {
HConstant* c_right = Add<HConstant>(right_type->AsConstant()); if (!maybe_string_add) right = TruncateToNumber(right, &right_type);
IfBuilder if_same(this); right_rep = Representation::FromType(right_type);
if (c_right->HasDoubleValue()) {
if_same.If<HCompareNumericAndBranch>(right, c_right, Token::EQ);
} else {
if_same.If<HCompareObjectEqAndBranch>(right, c_right);
}
if_same.Then();
if_same.ElseDeopt("Unexpected RHS of binary operation");
right = c_right;
} }
// Special case for string addition here. // Special case for string addition here.
@ -8834,11 +8825,6 @@ HValue* HGraphBuilder::BuildBinaryOperation(
return AddUncasted<HStringAdd>(left, right, STRING_ADD_CHECK_NONE); return AddUncasted<HStringAdd>(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()) { if (graph()->info()->IsStub()) {
left = EnforceNumberType(left, left_type); left = EnforceNumberType(left, left_type);
right = EnforceNumberType(right, right_type); right = EnforceNumberType(right, right_type);
@ -8870,6 +8856,22 @@ HValue* HGraphBuilder::BuildBinaryOperation(
instr = AddUncasted<HMul>(left, right); instr = AddUncasted<HMul>(left, right);
break; break;
case Token::MOD: { 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<HConstant>(
static_cast<int>(fixed_right_arg.value));
IfBuilder if_same(this);
if_same.If<HCompareNumericAndBranch>(right, fixed_right, Token::EQ);
if_same.Then();
if_same.ElseDeopt("Unexpected RHS of binary operation");
right = fixed_right;
}
}
instr = AddUncasted<HMod>(left, right); instr = AddUncasted<HMod>(left, right);
break; break;
} }

View File

@ -1351,7 +1351,8 @@ class HGraphBuilder {
HValue* right, HValue* right,
Handle<Type> left_type, Handle<Type> left_type,
Handle<Type> right_type, Handle<Type> right_type,
Handle<Type> result_type); Handle<Type> result_type,
Maybe<int> fixed_right_arg);
HLoadNamedField* AddLoadFixedArrayLength(HValue *object); HLoadNamedField* AddLoadFixedArrayLength(HValue *object);

View File

@ -2329,10 +2329,7 @@ BinaryOpIC::State::State(ExtraICState extra_ic_state) {
1 << FixedRightArgValueField::decode(extra_ic_state)); 1 << FixedRightArgValueField::decode(extra_ic_state));
left_kind_ = LeftKindField::decode(extra_ic_state); left_kind_ = LeftKindField::decode(extra_ic_state);
if (fixed_right_arg_.has_value) { if (fixed_right_arg_.has_value) {
// We have only 4 bits to encode the log2 of the fixed right arg, so the right_kind_ = Smi::IsValid(fixed_right_arg_.value) ? SMI : INT32;
// max value is 2^(2^4), which is always a SMI.
ASSERT(Smi::IsValid(fixed_right_arg_.value));
right_kind_ = SMI;
} else { } else {
right_kind_ = RightKindField::decode(extra_ic_state); right_kind_ = RightKindField::decode(extra_ic_state);
} }
@ -2585,17 +2582,6 @@ void BinaryOpIC::State::GenerateAheadOfTime(
} }
Handle<Type> BinaryOpIC::State::GetRightType(Isolate* isolate) const {
if (fixed_right_arg_.has_value) {
Handle<Smi> value = handle(Smi::FromInt(fixed_right_arg_.value), isolate);
Handle<Type> type = handle(Type::Constant(value, isolate), isolate);
ASSERT(type->Is(KindToType(right_kind_, isolate)));
return type;
}
return KindToType(right_kind_, isolate);
}
Handle<Type> BinaryOpIC::State::GetResultType(Isolate* isolate) const { Handle<Type> BinaryOpIC::State::GetResultType(Isolate* isolate) const {
Kind result_kind = result_kind_; Kind result_kind = result_kind_;
if (HasSideEffects()) { if (HasSideEffects()) {

View File

@ -866,11 +866,14 @@ class BinaryOpIC: public IC {
Token::Value op() const { return op_; } Token::Value op() const { return op_; }
OverwriteMode mode() const { return mode_; } OverwriteMode mode() const { return mode_; }
Maybe<int> fixed_right_arg() const { return fixed_right_arg_; }
Handle<Type> GetLeftType(Isolate* isolate) const { Handle<Type> GetLeftType(Isolate* isolate) const {
return KindToType(left_kind_, isolate); return KindToType(left_kind_, isolate);
} }
Handle<Type> GetRightType(Isolate* isolate) const; Handle<Type> GetRightType(Isolate* isolate) const {
return KindToType(right_kind_, isolate);
}
Handle<Type> GetResultType(Isolate* isolate) const; Handle<Type> GetResultType(Isolate* isolate) const;
void Print(StringStream* stream) const; void Print(StringStream* stream) const;

View File

@ -407,6 +407,7 @@ void TypeFeedbackOracle::BinaryType(TypeFeedbackId id,
Handle<Type>* left, Handle<Type>* left,
Handle<Type>* right, Handle<Type>* right,
Handle<Type>* result, Handle<Type>* result,
Maybe<int>* fixed_right_arg,
Token::Value op) { Token::Value op) {
Handle<Object> object = GetInfo(id); Handle<Object> object = GetInfo(id);
if (!object->IsCode()) { if (!object->IsCode()) {
@ -415,6 +416,7 @@ void TypeFeedbackOracle::BinaryType(TypeFeedbackId id,
ASSERT(op < BinaryOpIC::State::FIRST_TOKEN || ASSERT(op < BinaryOpIC::State::FIRST_TOKEN ||
op > BinaryOpIC::State::LAST_TOKEN); op > BinaryOpIC::State::LAST_TOKEN);
*left = *right = *result = handle(Type::None(), isolate_); *left = *right = *result = handle(Type::None(), isolate_);
*fixed_right_arg = Maybe<int>();
return; return;
} }
Handle<Code> code = Handle<Code>::cast(object); Handle<Code> code = Handle<Code>::cast(object);
@ -425,6 +427,7 @@ void TypeFeedbackOracle::BinaryType(TypeFeedbackId id,
*left = state.GetLeftType(isolate()); *left = state.GetLeftType(isolate());
*right = state.GetRightType(isolate()); *right = state.GetRightType(isolate());
*result = state.GetResultType(isolate()); *result = state.GetResultType(isolate());
*fixed_right_arg = state.fixed_right_arg();
} }

View File

@ -312,6 +312,7 @@ class TypeFeedbackOracle: public ZoneObject {
Handle<Type>* left, Handle<Type>* left,
Handle<Type>* right, Handle<Type>* right,
Handle<Type>* result, Handle<Type>* result,
Maybe<int>* fixed_right_arg,
Token::Value operation); Token::Value operation);
void CompareType(TypeFeedbackId id, void CompareType(TypeFeedbackId id,

View File

@ -571,11 +571,13 @@ void AstTyper::VisitCountOperation(CountOperation* expr) {
void AstTyper::VisitBinaryOperation(BinaryOperation* expr) { void AstTyper::VisitBinaryOperation(BinaryOperation* expr) {
// Collect type feedback. // Collect type feedback.
Handle<Type> type, left_type, right_type; Handle<Type> type, left_type, right_type;
Maybe<int> fixed_right_arg;
oracle()->BinaryType(expr->BinaryOperationFeedbackId(), 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, type);
NarrowLowerType(expr->left(), left_type); NarrowLowerType(expr->left(), left_type);
NarrowLowerType(expr->right(), right_type); NarrowLowerType(expr->right(), right_type);
expr->set_fixed_right_arg(fixed_right_arg);
if (expr->op() == Token::OR || expr->op() == Token::AND) { if (expr->op() == Token::OR || expr->op() == Token::AND) {
expr->left()->RecordToBooleanTypeFeedback(oracle()); expr->left()->RecordToBooleanTypeFeedback(oracle());
} }