Optimize the equality check case of ICCompare stubs.

This includes specialcasing the generation when we know that the maps
of the two objects are the same. In addition, a new specialized
compare ic known objects cache is created.

The reason for the cache is that we need to have access to the stub
code from the roots; if we do not, the GC will collect the stub. In
this specialized case we use the map pointer as key in the cache, and
we always do a lookup before generating code. Actually hitting
something in the cache will happen very rarely, but we could
potentially overwrite an existing stub, which again will lead to the
GC collecting this old stub (even if it is referenced from other code
objects)
Review URL: http://codereview.chromium.org/8520006

git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@10216 ce2b1a6d-e550-0410-aec6-3dcde31c8c00

src/code-stubs.cc

@@ -101,7 +101,14 @@ Handle<Code> CodeStub::GetCode() {
   Factory* factory = isolate->factory();
   Heap* heap = isolate->heap();
   Code* code;
-  if (!FindCodeInCache(&code)) {
+  if (UseSpecialCache()
+      ? FindCodeInSpecialCache(&code)
+      : FindCodeInCache(&code)) {
+    ASSERT(IsPregenerated() == code->is_pregenerated());
+    return Handle<Code>(code);
+  }
+
+  {
     HandleScope scope(isolate);
 
     // Generate the new code.
@@ -121,19 +128,21 @@ Handle<Code> CodeStub::GetCode() {
     RecordCodeGeneration(*new_object, &masm);
     FinishCode(new_object);
 
-    // Update the dictionary and the root in Heap.
-    Handle<NumberDictionary> dict =
-        factory->DictionaryAtNumberPut(
-            Handle<NumberDictionary>(heap->code_stubs()),
-            GetKey(),
-            new_object);
-    heap->public_set_code_stubs(*dict);
+    if (UseSpecialCache()) {
+      AddToSpecialCache(new_object);
+    } else {
+      // Update the dictionary and the root in Heap.
+      Handle<NumberDictionary> dict =
+          factory->DictionaryAtNumberPut(
+              Handle<NumberDictionary>(heap->code_stubs()),
+              GetKey(),
+              new_object);
+      heap->public_set_code_stubs(*dict);
+    }
     code = *new_object;
-    Activate(code);
-  } else {
-    CHECK(IsPregenerated() == code->is_pregenerated());
   }
 
+  Activate(code);
   ASSERT(!NeedsImmovableCode() || heap->lo_space()->Contains(code));
   return Handle<Code>(code, isolate);
 }
@@ -159,6 +168,32 @@ void CodeStub::PrintName(StringStream* stream) {
 }
 
 
+void ICCompareStub::AddToSpecialCache(Handle<Code> new_object) {
+  ASSERT(*known_map_ != NULL);
+  Isolate* isolate = new_object->GetIsolate();
+  Factory* factory = isolate->factory();
+  return Map::UpdateCodeCache(known_map_,
+                              factory->compare_ic_symbol(),
+                              new_object);
+}
+
+
+bool ICCompareStub::FindCodeInSpecialCache(Code** code_out) {
+  Isolate* isolate = known_map_->GetIsolate();
+  Factory* factory = isolate->factory();
+  Code::Flags flags = Code::ComputeFlags(
+      static_cast<Code::Kind>(GetCodeKind()),
+      UNINITIALIZED);
+  Handle<Object> probe(
+      known_map_->FindInCodeCache(*factory->compare_ic_symbol(), flags));
+  if (probe->IsCode()) {
+    *code_out = Code::cast(*probe);
+    return true;
+  }
+  return false;
+}
+
+
 int ICCompareStub::MinorKey() {
   return OpField::encode(op_ - Token::EQ) | StateField::encode(state_);
 }
@@ -184,6 +219,10 @@ void ICCompareStub::Generate(MacroAssembler* masm) {
     case CompareIC::OBJECTS:
       GenerateObjects(masm);
       break;
+    case CompareIC::KNOWN_OBJECTS:
+      ASSERT(*known_map_ != NULL);
+      GenerateKnownObjects(masm);
+      break;
     default:
       UNREACHABLE();
   }

src/code-stubs.h

@@ -194,6 +194,17 @@ class CodeStub BASE_EMBEDDED {
     return UNINITIALIZED;
   }
 
+  // Add the code to a specialized cache, specific to an individual
+  // stub type. Please note, this method must add the code object to a
+  // roots object, otherwise we will remove the code during GC.
+  virtual void AddToSpecialCache(Handle<Code> new_object) { }
+
+  // Find code in a specialized cache, work is delegated to the specific stub.
+  virtual bool FindCodeInSpecialCache(Code** code_out) { return false; }
+
+  // If a stub uses a special cache override this.
+  virtual bool UseSpecialCache() { return false; }
+
   // Returns a name for logging/debugging purposes.
   SmartArrayPointer<const char> GetName();
   virtual void PrintName(StringStream* stream);
@@ -465,6 +476,8 @@ class ICCompareStub: public CodeStub {
 
   virtual void Generate(MacroAssembler* masm);
 
+  void set_known_map(Handle<Map> map) { known_map_ = map; }
+
  private:
   class OpField: public BitField<int, 0, 3> { };
   class StateField: public BitField<int, 3, 5> { };
@@ -484,12 +497,18 @@ class ICCompareStub: public CodeStub {
   void GenerateStrings(MacroAssembler* masm);
   void GenerateObjects(MacroAssembler* masm);
   void GenerateMiss(MacroAssembler* masm);
+  void GenerateKnownObjects(MacroAssembler* masm);
 
   bool strict() const { return op_ == Token::EQ_STRICT; }
   Condition GetCondition() const { return CompareIC::ComputeCondition(op_); }
 
+  virtual void AddToSpecialCache(Handle<Code> new_object);
+  virtual bool FindCodeInSpecialCache(Code** code_out);
+  virtual bool UseSpecialCache() { return state_ == CompareIC::KNOWN_OBJECTS; }
+
   Token::Value op_;
   CompareIC::State state_;
+  Handle<Map> known_map_;
 };
 
 

src/heap.cc

@@ -2417,6 +2417,7 @@ bool Heap::CreateInitialObjects() {
   }
   set_code_stubs(NumberDictionary::cast(obj));
 
+
   // Allocate the non_monomorphic_cache used in stub-cache.cc. The initial size
   // is set to avoid expanding the dictionary during bootstrapping.
   { MaybeObject* maybe_obj = NumberDictionary::Allocate(64);

src/heap.h

@@ -245,6 +245,7 @@ inline Heap* _inline_get_heap_();
   V(use_strict, "use strict")                                            \
   V(dot_symbol, ".")                                                     \
   V(anonymous_function_symbol, "(anonymous function)")                   \
+  V(compare_ic_symbol, ".compare_ic")                                   \
   V(infinity_symbol, "Infinity")                                         \
   V(minus_infinity_symbol, "-Infinity")
 

src/hydrogen.cc

@@ -6141,14 +6141,27 @@ void HGraphBuilder::VisitCompareOperation(CompareOperation* expr) {
     switch (op) {
       case Token::EQ:
       case Token::EQ_STRICT: {
-        AddInstruction(new(zone()) HCheckNonSmi(left));
-        AddInstruction(HCheckInstanceType::NewIsSpecObject(left));
-        AddInstruction(new(zone()) HCheckNonSmi(right));
-        AddInstruction(HCheckInstanceType::NewIsSpecObject(right));
-        HCompareObjectEqAndBranch* result =
-            new(zone()) HCompareObjectEqAndBranch(left, right);
-        result->set_position(expr->position());
-        return ast_context()->ReturnControl(result, expr->id());
+        // Can we get away with map check and not instance type check?
+        Handle<Map> map = oracle()->GetCompareMap(expr);
+        if (!map.is_null()) {
+          AddInstruction(new(zone()) HCheckNonSmi(left));
+          AddInstruction(new(zone()) HCheckMap(left, map));
+          AddInstruction(new(zone()) HCheckNonSmi(right));
+          AddInstruction(new(zone()) HCheckMap(right, map));
+          HCompareObjectEqAndBranch* result =
+              new(zone()) HCompareObjectEqAndBranch(left, right);
+          result->set_position(expr->position());
+          return ast_context()->ReturnControl(result, expr->id());
+        } else {
+          AddInstruction(new(zone()) HCheckNonSmi(left));
+          AddInstruction(HCheckInstanceType::NewIsSpecObject(left));
+          AddInstruction(new(zone()) HCheckNonSmi(right));
+          AddInstruction(HCheckInstanceType::NewIsSpecObject(right));
+          HCompareObjectEqAndBranch* result =
+              new(zone()) HCompareObjectEqAndBranch(left, right);
+          result->set_position(expr->position());
+          return ast_context()->ReturnControl(result, expr->id());
+        }
       }
       default:
         return Bailout("Unsupported non-primitive compare");

src/ia32/code-stubs-ia32.cc

@@ -6670,33 +6670,45 @@ void ICCompareStub::GenerateObjects(MacroAssembler* masm) {
 }
 
 
-void ICCompareStub::GenerateMiss(MacroAssembler* masm) {
-  // Save the registers.
-  __ pop(ecx);
-  __ push(edx);
-  __ push(eax);
-  __ push(ecx);
+void ICCompareStub::GenerateKnownObjects(MacroAssembler* masm) {
+  Label miss;
+  __ mov(ecx, edx);
+  __ and_(ecx, eax);
+  __ JumpIfSmi(ecx, &miss, Label::kNear);
 
+  __ mov(ecx, FieldOperand(eax, HeapObject::kMapOffset));
+  __ mov(ebx, FieldOperand(edx, HeapObject::kMapOffset));
+  __ cmp(ecx, known_map_);
+  __ j(not_equal, &miss, Label::kNear);
+  __ cmp(ebx, known_map_);
+  __ j(not_equal, &miss, Label::kNear);
+
+  __ sub(eax, edx);
+  __ ret(0);
+
+  __ bind(&miss);
+  GenerateMiss(masm);
+}
+
+
+void ICCompareStub::GenerateMiss(MacroAssembler* masm) {
   {
     // Call the runtime system in a fresh internal frame.
     ExternalReference miss = ExternalReference(IC_Utility(IC::kCompareIC_Miss),
                                                masm->isolate());
     FrameScope scope(masm, StackFrame::INTERNAL);
-    __ push(edx);
+    __ push(edx);  // Preserve edx and eax.
+    __ push(eax);
+    __ push(edx);  // And also use them as the arguments.
     __ push(eax);
     __ push(Immediate(Smi::FromInt(op_)));
     __ CallExternalReference(miss, 3);
+    // Compute the entry point of the rewritten stub.
+    __ lea(edi, FieldOperand(eax, Code::kHeaderSize));
+    __ pop(eax);
+    __ pop(edx);
   }
 
-  // Compute the entry point of the rewritten stub.
-  __ lea(edi, FieldOperand(eax, Code::kHeaderSize));
-
-  // Restore registers.
-  __ pop(ecx);
-  __ pop(eax);
-  __ pop(edx);
-  __ push(ecx);
-
   // Do a tail call to the rewritten stub.
   __ jmp(edi);
 }

src/ia32/ic-ia32.cc

@@ -1625,6 +1625,9 @@ void CompareIC::UpdateCaches(Handle<Object> x, Handle<Object> y) {
     rewritten = stub.GetCode();
   } else {
     ICCompareStub stub(op_, state);
+    if (state == KNOWN_OBJECTS) {
+      stub.set_known_map(Handle<Map>(Handle<JSObject>::cast(x)->map()));
+    }
     rewritten = stub.GetCode();
   }
   set_target(*rewritten);

src/ic.cc

@@ -2320,6 +2320,7 @@ const char* CompareIC::GetStateName(State state) {
     case SMIS: return "SMIS";
     case HEAP_NUMBERS: return "HEAP_NUMBERS";
     case OBJECTS: return "OBJECTS";
+    case KNOWN_OBJECTS: return "OBJECTS";
     case SYMBOLS: return "SYMBOLS";
     case STRINGS: return "STRINGS";
     case GENERIC: return "GENERIC";
@@ -2334,20 +2335,37 @@ CompareIC::State CompareIC::TargetState(State state,
                                         bool has_inlined_smi_code,
                                         Handle<Object> x,
                                         Handle<Object> y) {
-  if (!has_inlined_smi_code && state != UNINITIALIZED && state != SYMBOLS) {
-    return GENERIC;
+  switch (state) {
+    case UNINITIALIZED:
+      if (x->IsSmi() && y->IsSmi()) return SMIS;
+      if (x->IsNumber() && y->IsNumber()) return HEAP_NUMBERS;
+      if (!Token::IsEqualityOp(op_)) return GENERIC;
+      if (x->IsSymbol() && y->IsSymbol()) return SYMBOLS;
+      if (x->IsString() && y->IsString()) return STRINGS;
+      if (x->IsJSObject() && y->IsJSObject()) {
+        if (Handle<JSObject>::cast(x)->map() ==
+            Handle<JSObject>::cast(y)->map() &&
+            Token::IsEqualityOp(op_)) {
+          return KNOWN_OBJECTS;
+        } else {
+          return OBJECTS;
+        }
+      }
+      return GENERIC;
+    case SMIS:
+      return has_inlined_smi_code && x->IsNumber() && y->IsNumber()
+          ? HEAP_NUMBERS
+          : GENERIC;
+    case SYMBOLS:
+      ASSERT(Token::IsEqualityOp(op_));
+      return x->IsString() && y->IsString() ? STRINGS : GENERIC;
+    case HEAP_NUMBERS:
+    case STRINGS:
+    case OBJECTS:
+    case KNOWN_OBJECTS:
+    case GENERIC:
+      return GENERIC;
   }
-  if (state == UNINITIALIZED && x->IsSmi() && y->IsSmi()) return SMIS;
-  if ((state == UNINITIALIZED || (state == SMIS && has_inlined_smi_code)) &&
-      x->IsNumber() && y->IsNumber()) return HEAP_NUMBERS;
-  if (op_ != Token::EQ && op_ != Token::EQ_STRICT) return GENERIC;
-  if (state == UNINITIALIZED &&
-      x->IsSymbol() && y->IsSymbol()) return SYMBOLS;
-  if ((state == UNINITIALIZED || state == SYMBOLS) &&
-      x->IsString() && y->IsString()) return STRINGS;
-  if (state == UNINITIALIZED &&
-      x->IsJSObject() && y->IsJSObject()) return OBJECTS;
-  return GENERIC;
 }
 
 

src/ic.h

@@ -724,6 +724,7 @@ class CompareIC: public IC {
     SYMBOLS,
     STRINGS,
     OBJECTS,
+    KNOWN_OBJECTS,
     GENERIC
   };
 

src/mark-compact.cc

@@ -883,8 +883,6 @@ class StaticMarkingVisitor : public StaticVisitorBase {
     Code* target = Code::GetCodeFromTargetAddress(rinfo->target_address());
     if (FLAG_cleanup_code_caches_at_gc && target->is_inline_cache_stub()) {
       IC::Clear(rinfo->pc());
-      // Please note targets for cleared inline cached do not have to be
-      // marked since they are contained in HEAP->non_monomorphic_cache().
       target = Code::GetCodeFromTargetAddress(rinfo->target_address());
     } else {
       if (FLAG_cleanup_code_caches_at_gc &&
@@ -893,9 +891,10 @@ class StaticMarkingVisitor : public StaticVisitorBase {
           target->has_function_cache()) {
         CallFunctionStub::Clear(heap, rinfo->pc());
       }
-      MarkBit code_mark = Marking::MarkBitFrom(target);
-      heap->mark_compact_collector()->MarkObject(target, code_mark);
     }
+    MarkBit code_mark = Marking::MarkBitFrom(target);
+    heap->mark_compact_collector()->MarkObject(target, code_mark);
+
     heap->mark_compact_collector()->RecordRelocSlot(rinfo, target);
   }
 

src/type-info.cc

@@ -259,6 +259,7 @@ TypeInfo TypeFeedbackOracle::CompareType(CompareOperation* expr) {
     case CompareIC::STRINGS:
       return TypeInfo::String();
     case CompareIC::OBJECTS:
+    case CompareIC::KNOWN_OBJECTS:
       // TODO(kasperl): We really need a type for JS objects here.
       return TypeInfo::NonPrimitive();
     case CompareIC::GENERIC:
@@ -278,6 +279,19 @@ bool TypeFeedbackOracle::IsSymbolCompare(CompareOperation* expr) {
 }
 
 
+Handle<Map> TypeFeedbackOracle::GetCompareMap(CompareOperation* expr) {
+  Handle<Object> object = GetInfo(expr->id());
+  if (!object->IsCode()) return Handle<Map>::null();
+  Handle<Code> code = Handle<Code>::cast(object);
+  if (!code->is_compare_ic_stub()) return Handle<Map>::null();
+  CompareIC::State state = static_cast<CompareIC::State>(code->compare_state());
+  if (state != CompareIC::KNOWN_OBJECTS) {
+    return Handle<Map>::null();
+  }
+  return Handle<Map>(code->FindFirstMap());
+}
+
+
 TypeInfo TypeFeedbackOracle::UnaryType(UnaryOperation* expr) {
   Handle<Object> object = GetInfo(expr->id());
   TypeInfo unknown = TypeInfo::Unknown();
@@ -367,6 +381,7 @@ TypeInfo TypeFeedbackOracle::SwitchType(CaseClause* clause) {
     case CompareIC::HEAP_NUMBERS:
       return TypeInfo::Number();
     case CompareIC::OBJECTS:
+    case CompareIC::KNOWN_OBJECTS:
       // TODO(kasperl): We really need a type for JS objects here.
       return TypeInfo::NonPrimitive();
     case CompareIC::GENERIC:

src/type-info.h

@@ -273,6 +273,7 @@ class TypeFeedbackOracle BASE_EMBEDDED {
   TypeInfo BinaryType(BinaryOperation* expr);
   TypeInfo CompareType(CompareOperation* expr);
   bool IsSymbolCompare(CompareOperation* expr);
+  Handle<Map> GetCompareMap(CompareOperation* expr);
   TypeInfo SwitchType(CaseClause* clause);
   TypeInfo IncrementType(CountOperation* expr);