From 59c334ebf45e150b47856cbc7a15e90c2a087aa0 Mon Sep 17 00:00:00 2001 From: "jkummerow@chromium.org" Date: Wed, 18 Dec 2013 11:58:58 +0000 Subject: [PATCH] Hydrogen: Re-use regular comparisons infrastructure for switch statements R=rossberg@chromium.org Review URL: https://codereview.chromium.org/111573003 git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@18348 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- src/ast.h | 6 -- src/code-stubs.h | 4 -- src/hydrogen.cc | 157 ++++++++++++++++++++--------------------------- src/hydrogen.h | 9 +++ src/type-info.cc | 12 ---- src/type-info.h | 2 - src/typing.cc | 31 +++------- 7 files changed, 83 insertions(+), 138 deletions(-) diff --git a/src/ast.h b/src/ast.h index db7e259bbe..cf3ef92cc5 100644 --- a/src/ast.h +++ b/src/ast.h @@ -1144,16 +1144,11 @@ class SwitchStatement V8_FINAL : public BreakableStatement { void Initialize(Expression* tag, ZoneList* cases) { tag_ = tag; cases_ = cases; - switch_type_ = UNKNOWN_SWITCH; } Expression* tag() const { return tag_; } ZoneList* 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) : BreakableStatement(isolate, labels, TARGET_FOR_ANONYMOUS, pos), @@ -1163,7 +1158,6 @@ class SwitchStatement V8_FINAL : public BreakableStatement { private: Expression* tag_; ZoneList* cases_; - SwitchType switch_type_; }; diff --git a/src/code-stubs.h b/src/code-stubs.h index 989163cc93..2ab62b0cdc 100644 --- a/src/code-stubs.h +++ b/src/code-stubs.h @@ -1176,10 +1176,6 @@ class ICCompareStub: public PlatformCodeStub { CompareIC::State* handler_state, Token::Value* op); - static CompareIC::State CompareState(int minor_key) { - return static_cast(HandlerStateField::decode(minor_key)); - } - virtual InlineCacheState GetICState(); private: diff --git a/src/hydrogen.cc b/src/hydrogen.cc index abefcb1d46..b3549f527e 100644 --- a/src/hydrogen.cc +++ b/src/hydrogen.cc @@ -4136,8 +4136,7 @@ void HOptimizedGraphBuilder::VisitSwitchStatement(SwitchStatement* stmt) { 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* clauses = stmt->cases(); int clause_count = clauses->length(); @@ -4146,28 +4145,10 @@ void HOptimizedGraphBuilder::VisitSwitchStatement(SwitchStatement* stmt) { return Bailout(kSwitchStatementTooManyClauses); } - ASSERT(stmt->switch_type() != SwitchStatement::UNKNOWN_SWITCH); - CHECK_ALIVE(VisitForValue(stmt->tag())); Add(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( - 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 tag_type = stmt->tag()->bounds().lower; // 1. Build all the tests, with dangling true branches BailoutId default_id = BailoutId::None(); @@ -4183,35 +4164,12 @@ void HOptimizedGraphBuilder::VisitSwitchStatement(SwitchStatement* stmt) { 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("Non-smi switch type", Deoptimizer::SOFT); - } - - HCompareNumericAndBranch* compare_ = - New(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(tag_value, - label_value, - Token::EQ_STRICT); - } else { - HValue* test = Add(tag_value, - label_value, - Token::EQ_STRICT); - if (test->HasObservableSideEffects()) { - Push(test); - Add(clause->id(), REMOVABLE_SIMULATE); - Drop(1); - } - compare = New(test); - } + Handle label_type = clause->label()->bounds().lower; + Handle 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 @@ void HOptimizedGraphBuilder::VisitSwitchStatement(SwitchStatement* stmt) { 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 @@ void HOptimizedGraphBuilder::VisitCompareOperation(CompareOperation* expr) { Handle left_type = expr->left()->bounds().lower; Handle right_type = expr->right()->bounds().lower; Handle 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())); @@ -9218,34 +9168,54 @@ void HOptimizedGraphBuilder::VisitCompareOperation(CompareOperation* expr) { 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 left_type, + Handle right_type, + Handle 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 = combined_type->AsClass(); - AddCheckMap(left, map); - AddCheckMap(right, map); - HCompareObjectEqAndBranch* result = - New(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(left, HCheckInstanceType::IS_SPEC_OBJECT); - BuildCheckHeapObject(right); - Add(right, HCheckInstanceType::IS_SPEC_OBJECT); - HCompareObjectEqAndBranch* result = - New(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 = combined_type->AsClass(); + AddCheckMap(left, map); + AddCheckMap(right, map); + HCompareObjectEqAndBranch* result = + New(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(left, HCheckInstanceType::IS_SPEC_OBJECT); + BuildCheckHeapObject(right); + Add(right, HCheckInstanceType::IS_SPEC_OBJECT); + HCompareObjectEqAndBranch* result = + New(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 @@ void HOptimizedGraphBuilder::VisitCompareOperation(CompareOperation* expr) { Add(right, HCheckInstanceType::IS_INTERNALIZED_STRING); HCompareObjectEqAndBranch* result = New(left, right); - return ast_context()->ReturnControl(result, expr->id()); + return result; } else if (combined_type->Is(Type::String())) { BuildCheckHeapObject(left); Add(left, HCheckInstanceType::IS_STRING); @@ -9263,23 +9233,28 @@ void HOptimizedGraphBuilder::VisitCompareOperation(CompareOperation* expr) { Add(right, HCheckInstanceType::IS_STRING); HStringCompareAndBranch* result = New(left, right, op); - return ast_context()->ReturnControl(result, expr->id()); + return result; } else { if (combined_rep.IsTagged() || combined_rep.IsNone()) { - HCompareGeneric* result = New(left, right, op); + HCompareGeneric* result = Add(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(result); + return branch; } else { HCompareNumericAndBranch* result = New(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; } } } diff --git a/src/hydrogen.h b/src/hydrogen.h index 61e98b2b0c..b2fd14c3ab 100644 --- a/src/hydrogen.h +++ b/src/hydrogen.h @@ -2323,6 +2323,15 @@ class HOptimizedGraphBuilder : public HGraphBuilder, public AstVisitor { void HandleLiteralCompareNil(CompareOperation* expr, Expression* sub_expr, NilValue nil); + HControlInstruction* BuildCompareInstruction(Token::Value op, + HValue* left, + HValue* right, + Handle left_type, + Handle right_type, + Handle combined_type, + int left_position, + int right_position, + BailoutId bailout_id); HInstruction* BuildStringCharCodeAt(HValue* string, HValue* index); diff --git a/src/type-info.cc b/src/type-info.cc index eed54ce2bc..73d8c756a1 100644 --- a/src/type-info.cc +++ b/src/type-info.cc @@ -320,18 +320,6 @@ void TypeFeedbackOracle::BinaryType(TypeFeedbackId id, } -Handle TypeFeedbackOracle::ClauseType(TypeFeedbackId id) { - Handle info = GetInfo(id); - Handle result(Type::None(), isolate_); - if (info->IsCode() && Handle::cast(info)->is_compare_ic_stub()) { - Handle code = Handle::cast(info); - CompareIC::State state = ICCompareStub::CompareState(code->stub_info()); - result = CompareIC::StateToType(isolate_, state); - } - return result; -} - - Handle TypeFeedbackOracle::CountType(TypeFeedbackId id) { Handle object = GetInfo(id); if (!object->IsCode()) return handle(Type::None(), isolate_); diff --git a/src/type-info.h b/src/type-info.h index 0ff99e994d..8bad5c0571 100644 --- a/src/type-info.h +++ b/src/type-info.h @@ -303,8 +303,6 @@ class TypeFeedbackOracle: public ZoneObject { Handle CountType(TypeFeedbackId id); - Handle ClauseType(TypeFeedbackId id); - Zone* zone() const { return zone_; } Isolate* isolate() const { return isolate_; } diff --git a/src/typing.cc b/src/typing.cc index cec5f863de..de9f4041e7 100644 --- a/src/typing.cc +++ b/src/typing.cc @@ -222,24 +222,23 @@ void AstTyper::VisitSwitchStatement(SwitchStatement* stmt) { RECURSE(Visit(stmt->tag())); ZoneList* 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 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 @@ void AstTyper::VisitSwitchStatement(SwitchStatement* stmt) { } 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())); - } - } }