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

diff --git a/src/ast.h b/src/ast.h
index db7e259bbedf83ee33a40922439983e770058e44..cf3ef92cc50f7b9daf979abe15bd9d60e42cbe3d 100644 (file)
--- a/src/ast.h
+++ b/src/ast.h
@@ -1144,16 +1144,11 @@ class SwitchStatement V8_FINAL : public BreakableStatement {
   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)
       : BreakableStatement(isolate, labels, TARGET_FOR_ANONYMOUS, pos),
@@ -1163,7 +1158,6 @@ class SwitchStatement V8_FINAL : public BreakableStatement {
  private:
   Expression* tag_;
   ZoneList<CaseClause*>* cases_;
-  SwitchType switch_type_;
 };
 
 

diff --git a/src/code-stubs.h b/src/code-stubs.h
index 989163cc93074f9b817d0527a9d9b8ddf6d77e9e..2ab62b0cdc2a620e68cae5a1a18aace9d8b40842 100644 (file)
--- 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<CompareIC::State>(HandlerStateField::decode(minor_key));
-  }
-
   virtual InlineCacheState GetICState();
 
  private:

diff --git a/src/hydrogen.cc b/src/hydrogen.cc
index abefcb1d46ed1a2e0fc00d4bfa5d070ff1637f98..b3549f527e31f00a15f451f7d36733e8985bec70 100644 (file)
--- 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<CaseClause*>* 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<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 @@ 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<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 @@ 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<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()));
@@ -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<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 @@ void HOptimizedGraphBuilder::VisitCompareOperation(CompareOperation* expr) {
     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 @@ void HOptimizedGraphBuilder::VisitCompareOperation(CompareOperation* expr) {
     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;
     }
   }
 }

diff --git a/src/hydrogen.h b/src/hydrogen.h
index 61e98b2b0ce9c0053c131098bd74d30a4cc15c2f..b2fd14c3ab9f32d65a52d05fdbb3913bee628072 100644 (file)
--- 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<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);

diff --git a/src/type-info.cc b/src/type-info.cc
index eed54ce2bcdcd61aa7ec7e44b9664332597c7722..73d8c756a1e518417b901eec867ecee2da518a1b 100644 (file)
--- a/src/type-info.cc
+++ b/src/type-info.cc
@@ -320,18 +320,6 @@ void TypeFeedbackOracle::BinaryType(TypeFeedbackId id,
 }
 
 
-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) {
   Handle<Object> object = GetInfo(id);
   if (!object->IsCode()) return handle(Type::None(), isolate_);

diff --git a/src/type-info.h b/src/type-info.h
index 0ff99e994d913ddf4d1af2d08f3adff3130a82c2..8bad5c0571e36e72abbf05e8c85c496940a02d81 100644 (file)
--- a/src/type-info.h
+++ b/src/type-info.h
@@ -303,8 +303,6 @@ class TypeFeedbackOracle: public ZoneObject {
 
   Handle<Type> CountType(TypeFeedbackId id);
 
-  Handle<Type> ClauseType(TypeFeedbackId id);
-
   Zone* zone() const { return zone_; }
   Isolate* isolate() const { return isolate_; }
 

diff --git a/src/typing.cc b/src/typing.cc
index cec5f863de11be36cc5c523a728db3ff47fced4c..de9f4041e7755cb915af24739d912cc94583aa7c 100644 (file)
--- a/src/typing.cc
+++ b/src/typing.cc
@@ -222,24 +222,23 @@ void AstTyper::VisitSwitchStatement(SwitchStatement* stmt) {
   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 @@ 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()));
-    }
-  }
 }