Revision: 10217
Author:   [email protected]
Date:     Thu Dec  8 09:28:44 2011
Log:      Revert 10216 Optimize the equality check case of ICCompare stubs.

Missing arm and x64 implementations
Review URL: http://codereview.chromium.org/8883023
http://code.google.com/p/v8/source/detail?r=10217

Modified:
 /branches/bleeding_edge/src/code-stubs.cc
 /branches/bleeding_edge/src/code-stubs.h
 /branches/bleeding_edge/src/heap.cc
 /branches/bleeding_edge/src/heap.h
 /branches/bleeding_edge/src/hydrogen.cc
 /branches/bleeding_edge/src/ia32/code-stubs-ia32.cc
 /branches/bleeding_edge/src/ia32/ic-ia32.cc
 /branches/bleeding_edge/src/ic.cc
 /branches/bleeding_edge/src/ic.h
 /branches/bleeding_edge/src/mark-compact.cc
 /branches/bleeding_edge/src/type-info.cc
 /branches/bleeding_edge/src/type-info.h

=======================================
--- /branches/bleeding_edge/src/code-stubs.cc   Thu Dec  8 09:17:21 2011
+++ /branches/bleeding_edge/src/code-stubs.cc   Thu Dec  8 09:28:44 2011
@@ -101,14 +101,7 @@
   Factory* factory = isolate->factory();
   Heap* heap = isolate->heap();
   Code* code;
-  if (UseSpecialCache()
-      ? FindCodeInSpecialCache(&code)
-      : FindCodeInCache(&code)) {
-    ASSERT(IsPregenerated() == code->is_pregenerated());
-    return Handle<Code>(code);
-  }
-
-  {
+  if (!FindCodeInCache(&code)) {
     HandleScope scope(isolate);

     // Generate the new code.
@@ -128,21 +121,19 @@
     RecordCodeGeneration(*new_object, &masm);
     FinishCode(new_object);

-    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);
-    }
+    // 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);
 }
@@ -166,32 +157,6 @@
 void CodeStub::PrintName(StringStream* stream) {
   stream->Add("%s", MajorName(MajorKey(), false));
 }
-
-
-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() {
@@ -219,10 +184,6 @@
     case CompareIC::OBJECTS:
       GenerateObjects(masm);
       break;
-    case CompareIC::KNOWN_OBJECTS:
-      ASSERT(*known_map_ != NULL);
-      GenerateKnownObjects(masm);
-      break;
     default:
       UNREACHABLE();
   }
=======================================
--- /branches/bleeding_edge/src/code-stubs.h    Thu Dec  8 09:17:21 2011
+++ /branches/bleeding_edge/src/code-stubs.h    Thu Dec  8 09:28:44 2011
@@ -193,17 +193,6 @@
   virtual InlineCacheState GetICState() {
     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();
@@ -475,8 +464,6 @@
   }

   virtual void Generate(MacroAssembler* masm);
-
-  void set_known_map(Handle<Map> map) { known_map_ = map; }

  private:
   class OpField: public BitField<int, 0, 3> { };
@@ -497,18 +484,12 @@
   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_;
 };


=======================================
--- /branches/bleeding_edge/src/heap.cc Thu Dec  8 09:17:21 2011
+++ /branches/bleeding_edge/src/heap.cc Thu Dec  8 09:28:44 2011
@@ -2417,7 +2417,6 @@
   }
   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);
=======================================
--- /branches/bleeding_edge/src/heap.h  Thu Dec  8 09:17:21 2011
+++ /branches/bleeding_edge/src/heap.h  Thu Dec  8 09:28:44 2011
@@ -245,7 +245,6 @@
   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")

=======================================
--- /branches/bleeding_edge/src/hydrogen.cc     Thu Dec  8 09:17:21 2011
+++ /branches/bleeding_edge/src/hydrogen.cc     Thu Dec  8 09:28:44 2011
@@ -6141,27 +6141,14 @@
     switch (op) {
       case Token::EQ:
       case Token::EQ_STRICT: {
-        // 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());
-        }
+        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");
=======================================
--- /branches/bleeding_edge/src/ia32/code-stubs-ia32.cc Thu Dec 8 09:17:21 2011 +++ /branches/bleeding_edge/src/ia32/code-stubs-ia32.cc Thu Dec 8 09:28:44 2011
@@ -6668,46 +6668,34 @@
   __ bind(&miss);
   GenerateMiss(masm);
 }
-
-
-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) {
+  // Save the registers.
+  __ pop(ecx);
+  __ push(edx);
+  __ push(eax);
+  __ push(ecx);
+
   {
     // 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);  // Preserve edx and eax.
-    __ push(eax);
-    __ push(edx);  // And also use them as the arguments.
+    __ push(edx);
     __ 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);
=======================================
--- /branches/bleeding_edge/src/ia32/ic-ia32.cc Thu Dec  8 09:17:21 2011
+++ /branches/bleeding_edge/src/ia32/ic-ia32.cc Thu Dec  8 09:28:44 2011
@@ -1625,9 +1625,6 @@
     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);
=======================================
--- /branches/bleeding_edge/src/ic.cc   Thu Dec  8 09:17:21 2011
+++ /branches/bleeding_edge/src/ic.cc   Thu Dec  8 09:28:44 2011
@@ -2320,7 +2320,6 @@
     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";
@@ -2335,37 +2334,20 @@
                                         bool has_inlined_smi_code,
                                         Handle<Object> x,
                                         Handle<Object> y) {
-  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 (!has_inlined_smi_code && state != UNINITIALIZED && state != SYMBOLS) {
+    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;
 }


=======================================
--- /branches/bleeding_edge/src/ic.h    Thu Dec  8 09:17:21 2011
+++ /branches/bleeding_edge/src/ic.h    Thu Dec  8 09:28:44 2011
@@ -724,7 +724,6 @@
     SYMBOLS,
     STRINGS,
     OBJECTS,
-    KNOWN_OBJECTS,
     GENERIC
   };

=======================================
--- /branches/bleeding_edge/src/mark-compact.cc Thu Dec  8 09:17:21 2011
+++ /branches/bleeding_edge/src/mark-compact.cc Thu Dec  8 09:28:44 2011
@@ -883,6 +883,8 @@
     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 &&
@@ -891,10 +893,9 @@
           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);
   }

=======================================
--- /branches/bleeding_edge/src/type-info.cc    Thu Dec  8 09:17:21 2011
+++ /branches/bleeding_edge/src/type-info.cc    Thu Dec  8 09:28:44 2011
@@ -259,7 +259,6 @@
     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:
@@ -277,19 +276,6 @@
CompareIC::State state = static_cast<CompareIC::State>(code->compare_state());
   return state == CompareIC::SYMBOLS;
 }
-
-
-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) {
@@ -381,7 +367,6 @@
     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:
=======================================
--- /branches/bleeding_edge/src/type-info.h     Thu Dec  8 09:17:21 2011
+++ /branches/bleeding_edge/src/type-info.h     Thu Dec  8 09:28:44 2011
@@ -273,7 +273,6 @@
   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);

--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev

Reply via email to