Minor cleanup of CompareIC state

src/ic.cc

Index: src/ic.cc
diff --git a/src/ic.cc b/src/ic.cc
index e88ba7d75e10b455c95ea88c86143a1d7080e314..b1f4dc3ae484b3ad140df3a5011ecb059f7345cb 100644
--- a/src/ic.cc
+++ b/src/ic.cc
@@ -399,7 +399,7 @@ void CompareIC::Clear(Address address, Code* target) {
   ICCompareStub::DecodeMinorKey(target->stub_info(), NULL, NULL,
                                 &handler_state, &op);
   // Only clear CompareICs that can retain objects.
-  if (handler_state != KNOWN_OBJECTS) return;
+  if (handler_state != KNOWN_OBJECT) return;
   SetTargetAtAddress(address, GetRawUninitialized(op));
   PatchInlinedSmiCode(address, DISABLE_INLINED_SMI_CHECK);
 }
@@ -2373,11 +2373,11 @@ const char* CompareIC::GetStateName(State state) {
   switch (state) {
     case UNINITIALIZED: return "UNINITIALIZED";
     case SMI: return "SMI";
-    case HEAP_NUMBER: return "HEAP_NUMBER";
-    case OBJECT: return "OBJECTS";
-    case KNOWN_OBJECTS: return "KNOWN_OBJECTS";
+    case NUMBER: return "NUMBER";
     case SYMBOL: return "SYMBOL";
     case STRING: return "STRING";
+    case OBJECT: return "OBJECT";
+    case KNOWN_OBJECT: return "KNOWN_OBJECT";
     case GENERIC: return "GENERIC";
     default:
       UNREACHABLE();
@@ -2391,17 +2391,17 @@ static CompareIC::State InputState(CompareIC::State old_state,
   switch (old_state) {
     case CompareIC::UNINITIALIZED:
       if (value->IsSmi()) return CompareIC::SMI;
-      if (value->IsHeapNumber()) return CompareIC::HEAP_NUMBER;
+      if (value->IsHeapNumber()) return CompareIC::NUMBER;
       if (value->IsSymbol()) return CompareIC::SYMBOL;
       if (value->IsString()) return CompareIC::STRING;
       if (value->IsJSObject()) return CompareIC::OBJECT;
       break;
     case CompareIC::SMI:
       if (value->IsSmi()) return CompareIC::SMI;
-      if (value->IsHeapNumber()) return CompareIC::HEAP_NUMBER;
+      if (value->IsHeapNumber()) return CompareIC::NUMBER;
       break;
-    case CompareIC::HEAP_NUMBER:
-      if (value->IsNumber()) return CompareIC::HEAP_NUMBER;
+    case CompareIC::NUMBER:
+      if (value->IsNumber()) return CompareIC::NUMBER;
       break;
     case CompareIC::SYMBOL:
       if (value->IsSymbol()) return CompareIC::SYMBOL;
@@ -2415,7 +2415,7 @@ static CompareIC::State InputState(CompareIC::State old_state,
       break;
     case CompareIC::GENERIC:
       break;
-    case CompareIC::KNOWN_OBJECTS:
+    case CompareIC::KNOWN_OBJECT:
       UNREACHABLE();
       break;
   }
@@ -2432,13 +2432,13 @@ CompareIC::State CompareIC::TargetState(State old_state,
   switch (old_state) {
     case UNINITIALIZED:
       if (x->IsSmi() && y->IsSmi()) return SMI;
-      if (x->IsNumber() && y->IsNumber()) return HEAP_NUMBER;
+      if (x->IsNumber() && y->IsNumber()) return NUMBER;
       if (Token::IsOrderedRelationalCompareOp(op_)) {
         // Ordered comparisons treat undefined as NaN, so the
         // HEAP_NUMBER stub will do the right thing.

[Jakob Kummerow, 2013/02/15 09:12:53]

s/HEAP_//

[rossberg, 2013/02/19 11:32:13]

Done.

         if ((x->IsNumber() && y->IsUndefined()) ||
             (y->IsNumber() && x->IsUndefined())) {
-          return HEAP_NUMBER;
+          return NUMBER;
         }
       }
       if (x->IsSymbol() && y->IsSymbol()) {
@@ -2450,27 +2450,38 @@ CompareIC::State CompareIC::TargetState(State old_state,
       if (!Token::IsEqualityOp(op_)) return GENERIC;
       if (x->IsJSObject() && y->IsJSObject()) {
         if (Handle<JSObject>::cast(x)->map() ==
-            Handle<JSObject>::cast(y)->map() &&
-            Token::IsEqualityOp(op_)) {
-          return KNOWN_OBJECTS;
+            Handle<JSObject>::cast(y)->map()) {
+          return KNOWN_OBJECT;
         } else {
           return OBJECT;
         }
       }
       return GENERIC;
     case SMI:
-      return x->IsNumber() && y->IsNumber()
-          ? HEAP_NUMBER
-          : GENERIC;
+      return x->IsNumber() && y->IsNumber() ? NUMBER : GENERIC;
     case SYMBOL:
       ASSERT(Token::IsEqualityOp(op_));
       return x->IsString() && y->IsString() ? STRING : GENERIC;
-    case HEAP_NUMBER:
-      if (old_left == SMI && x->IsHeapNumber()) return HEAP_NUMBER;
-      if (old_right == SMI && y->IsHeapNumber()) return HEAP_NUMBER;
+    case NUMBER:
+      // If the failure was due to one side changing from smi to heap number,
+      // then keep the state (if other changed at the same time, we will get
+      // a second miss and then go to generic).
+      if (old_left == SMI && x->IsHeapNumber()) return NUMBER;
+      if (old_right == SMI && y->IsHeapNumber()) return NUMBER;
+      return GENERIC;
+    case KNOWN_OBJECT:
+      ASSERT(Token::IsEqualityOp(op_));
+      if (x->IsJSObject() && y->IsJSObject()) {
+        if (Handle<JSObject>::cast(x)->map() ==
+            Handle<JSObject>::cast(y)->map()) {
+          return KNOWN_OBJECT;

[Jakob Kummerow, 2013/02/15 09:12:53]

We should not have a KNOWN_OBJECT -> KNOWN_OBJECT transition. It could lead to
endless IC patching cycles. When KNOWN_OBJECT misses, we should go to OBJECT or
GENERIC.

[rossberg, 2013/02/19 11:32:13]

Done.

+        } else {
+          return OBJECT;
+        }
+      }
+      return GENERIC;
     case STRING:
     case OBJECT:
-    case KNOWN_OBJECTS:
     case GENERIC:
       return GENERIC;
   }
@@ -2489,7 +2500,7 @@ void CompareIC::UpdateCaches(Handle<Object> x, Handle<Object> y) {
   State state = TargetState(previous_state, previous_left, previous_right,
                             HasInlinedSmiCode(address()), x, y);
   ICCompareStub stub(op_, new_left, new_right, state);
-  if (state == KNOWN_OBJECTS) {
+  if (state == KNOWN_OBJECT) {
     stub.set_known_map(Handle<Map>(Handle<JSObject>::cast(x)->map()));
   }
   set_target(*stub.GetCode());