Revision: 18348
Author: [email protected]
Date: Wed Dec 18 11:58:58 2013 UTC
Log: Hydrogen: Re-use regular comparisons infrastructure for switch
statements
[email protected]
Review URL: https://codereview.chromium.org/111573003
http://code.google.com/p/v8/source/detail?r=18348
Modified:
/branches/bleeding_edge/src/ast.h
/branches/bleeding_edge/src/code-stubs.h
/branches/bleeding_edge/src/hydrogen.cc
/branches/bleeding_edge/src/hydrogen.h
/branches/bleeding_edge/src/type-info.cc
/branches/bleeding_edge/src/type-info.h
/branches/bleeding_edge/src/typing.cc
=======================================
--- /branches/bleeding_edge/src/ast.h Wed Dec 18 11:44:38 2013 UTC
+++ /branches/bleeding_edge/src/ast.h Wed Dec 18 11:58:58 2013 UTC
@@ -1144,15 +1144,10 @@
void Initialize(Expression* tag, ZoneList<CaseClause*>* cases) {
tag_ = tag;
cases_ = cases;
- switch_type_ = UNKNOWN_SWITCH;
}
Expression* tag() const { return tag_; }
ZoneList<CaseClause*>* cases() const { return cases_; }
-
- enum SwitchType { UNKNOWN_SWITCH, SMI_SWITCH, STRING_SWITCH,
GENERIC_SWITCH };
- SwitchType switch_type() const { return switch_type_; }
- void set_switch_type(SwitchType switch_type) { switch_type_ =
switch_type; }
protected:
SwitchStatement(Isolate* isolate, ZoneStringList* labels, int pos)
@@ -1163,7 +1158,6 @@
private:
Expression* tag_;
ZoneList<CaseClause*>* cases_;
- SwitchType switch_type_;
};
=======================================
--- /branches/bleeding_edge/src/code-stubs.h Wed Dec 18 10:40:26 2013 UTC
+++ /branches/bleeding_edge/src/code-stubs.h Wed Dec 18 11:58:58 2013 UTC
@@ -1176,10 +1176,6 @@
CompareIC::State* handler_state,
Token::Value* op);
- static CompareIC::State CompareState(int minor_key) {
- return
static_cast<CompareIC::State>(HandlerStateField::decode(minor_key));
- }
-
virtual InlineCacheState GetICState();
private:
=======================================
--- /branches/bleeding_edge/src/hydrogen.cc Wed Dec 18 11:44:38 2013 UTC
+++ /branches/bleeding_edge/src/hydrogen.cc Wed Dec 18 11:58:58 2013 UTC
@@ -4136,8 +4136,7 @@
ASSERT(current_block() != NULL);
ASSERT(current_block()->HasPredecessor());
- // We only optimize switch statements with smi-literal smi comparisons,
- // with a bounded number of clauses.
+ // We only optimize switch statements with a bounded number of clauses.
const int kCaseClauseLimit = 128;
ZoneList<CaseClause*>* clauses = stmt->cases();
int clause_count = clauses->length();
@@ -4145,29 +4144,11 @@
if (clause_count > kCaseClauseLimit) {
return Bailout(kSwitchStatementTooManyClauses);
}
-
- ASSERT(stmt->switch_type() != SwitchStatement::UNKNOWN_SWITCH);
CHECK_ALIVE(VisitForValue(stmt->tag()));
Add<HSimulate>(stmt->EntryId());
HValue* tag_value = Top();
-
- HUnaryControlInstruction* string_check = NULL;
- HBasicBlock* not_string_block = NULL;
-
- // Test switch's tag value if all clauses are string literals
- if (stmt->switch_type() == SwitchStatement::STRING_SWITCH) {
- HBasicBlock* first_test_block = graph()->CreateBasicBlock();
- not_string_block = graph()->CreateBasicBlock();
- string_check = New<HIsStringAndBranch>(
- tag_value, first_test_block, not_string_block);
- FinishCurrentBlock(string_check);
-
- set_current_block(not_string_block);
- Drop(1); // tag_value
-
- set_current_block(first_test_block);
- }
+ Handle<Type> tag_type = stmt->tag()->bounds().lower;
// 1. Build all the tests, with dangling true branches
BailoutId default_id = BailoutId::None();
@@ -4183,35 +4164,12 @@
CHECK_ALIVE(VisitForValue(clause->label()));
HValue* label_value = Pop();
- HControlInstruction* compare;
-
- if (stmt->switch_type() == SwitchStatement::SMI_SWITCH) {
- if (!clause->compare_type()->Is(Type::Smi())) {
- Add<HDeoptimize>("Non-smi switch type", Deoptimizer::SOFT);
- }
-
- HCompareNumericAndBranch* compare_ =
- New<HCompareNumericAndBranch>(tag_value,
- label_value,
- Token::EQ_STRICT);
- compare_->set_observed_input_representation(
- Representation::Smi(), Representation::Smi());
- compare = compare_;
- } else if (stmt->switch_type() == SwitchStatement::STRING_SWITCH) {
- compare = New<HStringCompareAndBranch>(tag_value,
- label_value,
- Token::EQ_STRICT);
- } else {
- HValue* test = Add<HCompareGeneric>(tag_value,
- label_value,
- Token::EQ_STRICT);
- if (test->HasObservableSideEffects()) {
- Push(test);
- Add<HSimulate>(clause->id(), REMOVABLE_SIMULATE);
- Drop(1);
- }
- compare = New<HBranch>(test);
- }
+ Handle<Type> label_type = clause->label()->bounds().lower;
+ Handle<Type> combined_type = clause->compare_type();
+ HControlInstruction* compare = BuildCompareInstruction(
+ Token::EQ_STRICT, tag_value, label_value, tag_type, label_type,
+ combined_type, stmt->tag()->position(),
clause->label()->position(),
+ clause->id());
HBasicBlock* next_test_block = graph()->CreateBasicBlock();
HBasicBlock* body_block = graph()->CreateBasicBlock();
@@ -4231,11 +4189,6 @@
HBasicBlock* last_block = current_block();
Drop(1); // tag_value
- if (not_string_block != NULL) {
- BailoutId join_id = !default_id.IsNone() ? default_id : stmt->ExitId();
- last_block = CreateJoin(last_block, not_string_block, join_id);
- }
-
// 2. Loop over the clauses and the linked list of tests in lockstep,
// translating the clause bodies.
HBasicBlock* fall_through_block = NULL;
@@ -9142,9 +9095,6 @@
Handle<Type> left_type = expr->left()->bounds().lower;
Handle<Type> right_type = expr->right()->bounds().lower;
Handle<Type> combined_type = expr->combined_type();
- Representation combined_rep = Representation::FromType(combined_type);
- Representation left_rep = Representation::FromType(left_type);
- Representation right_rep = Representation::FromType(right_type);
CHECK_ALIVE(VisitForValue(expr->left()));
CHECK_ALIVE(VisitForValue(expr->right()));
@@ -9217,35 +9167,55 @@
Deoptimizer::SOFT);
combined_type = left_type = right_type = handle(Type::Any(),
isolate());
}
+
+ HControlInstruction* compare = BuildCompareInstruction(
+ op, left, right, left_type, right_type, combined_type,
+ expr->left()->position(), expr->right()->position(), expr->id());
+ if (compare == NULL) return; // Bailed out.
+ return ast_context()->ReturnControl(compare, expr->id());
+}
+
+
+HControlInstruction* HOptimizedGraphBuilder::BuildCompareInstruction(
+ Token::Value op,
+ HValue* left,
+ HValue* right,
+ Handle<Type> left_type,
+ Handle<Type> right_type,
+ Handle<Type> combined_type,
+ int left_position,
+ int right_position,
+ BailoutId bailout_id) {
+ Representation left_rep = Representation::FromType(left_type);
+ Representation right_rep = Representation::FromType(right_type);
+ Representation combined_rep = Representation::FromType(combined_type);
if (combined_type->Is(Type::Receiver())) {
- switch (op) {
- case Token::EQ:
- case Token::EQ_STRICT: {
- // Can we get away with map check and not instance type check?
- if (combined_type->IsClass()) {
- Handle<Map> map = combined_type->AsClass();
- AddCheckMap(left, map);
- AddCheckMap(right, map);
- HCompareObjectEqAndBranch* result =
- New<HCompareObjectEqAndBranch>(left, right);
- if (FLAG_emit_opt_code_positions) {
- result->set_operand_position(zone(), 0,
expr->left()->position());
- result->set_operand_position(zone(), 1,
expr->right()->position());
- }
- return ast_context()->ReturnControl(result, expr->id());
- } else {
- BuildCheckHeapObject(left);
- Add<HCheckInstanceType>(left,
HCheckInstanceType::IS_SPEC_OBJECT);
- BuildCheckHeapObject(right);
- Add<HCheckInstanceType>(right,
HCheckInstanceType::IS_SPEC_OBJECT);
- HCompareObjectEqAndBranch* result =
- New<HCompareObjectEqAndBranch>(left, right);
- return ast_context()->ReturnControl(result, expr->id());
+ if (Token::IsEqualityOp(op)) {
+ // Can we get away with map check and not instance type check?
+ if (combined_type->IsClass()) {
+ Handle<Map> map = combined_type->AsClass();
+ AddCheckMap(left, map);
+ AddCheckMap(right, map);
+ HCompareObjectEqAndBranch* result =
+ New<HCompareObjectEqAndBranch>(left, right);
+ if (FLAG_emit_opt_code_positions) {
+ result->set_operand_position(zone(), 0, left_position);
+ result->set_operand_position(zone(), 1, right_position);
}
+ return result;
+ } else {
+ BuildCheckHeapObject(left);
+ Add<HCheckInstanceType>(left, HCheckInstanceType::IS_SPEC_OBJECT);
+ BuildCheckHeapObject(right);
+ Add<HCheckInstanceType>(right, HCheckInstanceType::IS_SPEC_OBJECT);
+ HCompareObjectEqAndBranch* result =
+ New<HCompareObjectEqAndBranch>(left, right);
+ return result;
}
- default:
- return Bailout(kUnsupportedNonPrimitiveCompare);
+ } else {
+ Bailout(kUnsupportedNonPrimitiveCompare);
+ return NULL;
}
} else if (combined_type->Is(Type::InternalizedString()) &&
Token::IsEqualityOp(op)) {
@@ -9255,7 +9225,7 @@
Add<HCheckInstanceType>(right,
HCheckInstanceType::IS_INTERNALIZED_STRING);
HCompareObjectEqAndBranch* result =
New<HCompareObjectEqAndBranch>(left, right);
- return ast_context()->ReturnControl(result, expr->id());
+ return result;
} else if (combined_type->Is(Type::String())) {
BuildCheckHeapObject(left);
Add<HCheckInstanceType>(left, HCheckInstanceType::IS_STRING);
@@ -9263,23 +9233,28 @@
Add<HCheckInstanceType>(right, HCheckInstanceType::IS_STRING);
HStringCompareAndBranch* result =
New<HStringCompareAndBranch>(left, right, op);
- return ast_context()->ReturnControl(result, expr->id());
+ return result;
} else {
if (combined_rep.IsTagged() || combined_rep.IsNone()) {
- HCompareGeneric* result = New<HCompareGeneric>(left, right, op);
+ HCompareGeneric* result = Add<HCompareGeneric>(left, right, op);
result->set_observed_input_representation(1, left_rep);
result->set_observed_input_representation(2, right_rep);
- return ast_context()->ReturnInstruction(result, expr->id());
+ if (result->HasObservableSideEffects()) {
+ Push(result);
+ AddSimulate(bailout_id, REMOVABLE_SIMULATE);
+ Drop(1);
+ }
+ // TODO(jkummerow): Can we make this more efficient?
+ HBranch* branch = New<HBranch>(result);
+ return branch;
} else {
HCompareNumericAndBranch* result =
New<HCompareNumericAndBranch>(left, right, op);
result->set_observed_input_representation(left_rep, right_rep);
if (FLAG_emit_opt_code_positions) {
- result->SetOperandPositions(zone(),
- expr->left()->position(),
- expr->right()->position());
+ result->SetOperandPositions(zone(), left_position, right_position);
}
- return ast_context()->ReturnControl(result, expr->id());
+ return result;
}
}
}
=======================================
--- /branches/bleeding_edge/src/hydrogen.h Wed Dec 4 09:27:48 2013 UTC
+++ /branches/bleeding_edge/src/hydrogen.h Wed Dec 18 11:58:58 2013 UTC
@@ -2323,6 +2323,15 @@
void HandleLiteralCompareNil(CompareOperation* expr,
Expression* sub_expr,
NilValue nil);
+ HControlInstruction* BuildCompareInstruction(Token::Value op,
+ HValue* left,
+ HValue* right,
+ Handle<Type> left_type,
+ Handle<Type> right_type,
+ Handle<Type> combined_type,
+ int left_position,
+ int right_position,
+ BailoutId bailout_id);
HInstruction* BuildStringCharCodeAt(HValue* string,
HValue* index);
=======================================
--- /branches/bleeding_edge/src/type-info.cc Thu Dec 12 14:57:00 2013 UTC
+++ /branches/bleeding_edge/src/type-info.cc Wed Dec 18 11:58:58 2013 UTC
@@ -318,18 +318,6 @@
*result = state.GetResultType(isolate());
*fixed_right_arg = state.fixed_right_arg();
}
-
-
-Handle<Type> TypeFeedbackOracle::ClauseType(TypeFeedbackId id) {
- Handle<Object> info = GetInfo(id);
- Handle<Type> result(Type::None(), isolate_);
- if (info->IsCode() && Handle<Code>::cast(info)->is_compare_ic_stub()) {
- Handle<Code> code = Handle<Code>::cast(info);
- CompareIC::State state =
ICCompareStub::CompareState(code->stub_info());
- result = CompareIC::StateToType(isolate_, state);
- }
- return result;
-}
Handle<Type> TypeFeedbackOracle::CountType(TypeFeedbackId id) {
=======================================
--- /branches/bleeding_edge/src/type-info.h Thu Dec 12 14:57:00 2013 UTC
+++ /branches/bleeding_edge/src/type-info.h Wed Dec 18 11:58:58 2013 UTC
@@ -303,8 +303,6 @@
Handle<Type> CountType(TypeFeedbackId id);
- Handle<Type> ClauseType(TypeFeedbackId id);
-
Zone* zone() const { return zone_; }
Isolate* isolate() const { return isolate_; }
=======================================
--- /branches/bleeding_edge/src/typing.cc Wed Dec 18 10:38:58 2013 UTC
+++ /branches/bleeding_edge/src/typing.cc Wed Dec 18 11:58:58 2013 UTC
@@ -222,24 +222,23 @@
RECURSE(Visit(stmt->tag()));
ZoneList<CaseClause*>* clauses = stmt->cases();
- SwitchStatement::SwitchType switch_type = stmt->switch_type();
Effects local_effects(zone());
bool complex_effects = false; // True for label effects or fall-through.
for (int i = 0; i < clauses->length(); ++i) {
CaseClause* clause = clauses->at(i);
+
Effects clause_effects = EnterEffects();
if (!clause->is_default()) {
Expression* label = clause->label();
- SwitchStatement::SwitchType label_switch_type =
- label->IsSmiLiteral() ? SwitchStatement::SMI_SWITCH :
- label->IsStringLiteral() ? SwitchStatement::STRING_SWITCH :
- SwitchStatement::GENERIC_SWITCH;
- if (switch_type == SwitchStatement::UNKNOWN_SWITCH)
- switch_type = label_switch_type;
- else if (switch_type != label_switch_type)
- switch_type = SwitchStatement::GENERIC_SWITCH;
+ // Collect type feedback.
+ Handle<Type> tag_type, label_type, combined_type;
+ oracle()->CompareType(clause->CompareId(),
+ &tag_type, &label_type, &combined_type);
+ NarrowLowerType(stmt->tag(), tag_type);
+ NarrowLowerType(label, label_type);
+ clause->set_compare_type(combined_type);
RECURSE(Visit(label));
if (!clause_effects.IsEmpty()) complex_effects = true;
@@ -260,20 +259,6 @@
} else {
store_.Seq(local_effects);
}
-
- if (switch_type == SwitchStatement::UNKNOWN_SWITCH)
- switch_type = SwitchStatement::GENERIC_SWITCH;
- stmt->set_switch_type(switch_type);
-
- // Collect type feedback.
- // TODO(rossberg): can we eliminate this special case and extra loop?
- if (switch_type == SwitchStatement::SMI_SWITCH) {
- for (int i = 0; i < clauses->length(); ++i) {
- CaseClause* clause = clauses->at(i);
- if (!clause->is_default())
-
clause->set_compare_type(oracle()->ClauseType(clause->CompareId()));
- }
- }
}
--
--
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/groups/opt_out.
