From 174532d23f8875c06e4880dfc3091a71c3a0aafe Mon Sep 17 00:00:00 2001 From: "ricow@chromium.org" Date: Thu, 8 Dec 2011 17:28:44 +0000 Subject: [PATCH] Revert 10216 Optimize the equality check case of ICCompare stubs. Missing arm and x64 implementations Review URL: http://codereview.chromium.org/8883023 git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@10217 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- src/code-stubs.cc | 61 +++++++------------------------------ src/code-stubs.h | 19 ------------ src/heap.cc | 1 - src/heap.h | 1 - src/hydrogen.cc | 29 +++++------------- src/ia32/code-stubs-ia32.cc | 44 ++++++++++---------------- src/ia32/ic-ia32.cc | 3 -- src/ic.cc | 44 ++++++++------------------ src/ic.h | 1 - src/mark-compact.cc | 7 +++-- src/type-info.cc | 15 --------- src/type-info.h | 1 - 12 files changed, 52 insertions(+), 174 deletions(-) diff --git a/src/code-stubs.cc b/src/code-stubs.cc index 85410c3cc8..ba7df802fa 100644 --- a/src/code-stubs.cc +++ b/src/code-stubs.cc @@ -101,14 +101,7 @@ Handle CodeStub::GetCode() { Factory* factory = isolate->factory(); Heap* heap = isolate->heap(); Code* code; - if (UseSpecialCache() - ? FindCodeInSpecialCache(&code) - : FindCodeInCache(&code)) { - ASSERT(IsPregenerated() == code->is_pregenerated()); - return Handle(code); - } - - { + if (!FindCodeInCache(&code)) { HandleScope scope(isolate); // Generate the new code. @@ -128,21 +121,19 @@ Handle CodeStub::GetCode() { RecordCodeGeneration(*new_object, &masm); FinishCode(new_object); - if (UseSpecialCache()) { - AddToSpecialCache(new_object); - } else { - // Update the dictionary and the root in Heap. - Handle dict = - factory->DictionaryAtNumberPut( - Handle(heap->code_stubs()), - GetKey(), - new_object); - heap->public_set_code_stubs(*dict); - } + // Update the dictionary and the root in Heap. + Handle dict = + factory->DictionaryAtNumberPut( + Handle(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, isolate); } @@ -168,32 +159,6 @@ void CodeStub::PrintName(StringStream* stream) { } -void ICCompareStub::AddToSpecialCache(Handle 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(GetCodeKind()), - UNINITIALIZED); - Handle 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_); } @@ -219,10 +184,6 @@ void ICCompareStub::Generate(MacroAssembler* masm) { case CompareIC::OBJECTS: GenerateObjects(masm); break; - case CompareIC::KNOWN_OBJECTS: - ASSERT(*known_map_ != NULL); - GenerateKnownObjects(masm); - break; default: UNREACHABLE(); } diff --git a/src/code-stubs.h b/src/code-stubs.h index 34da148e5d..dc07e5d67d 100644 --- a/src/code-stubs.h +++ b/src/code-stubs.h @@ -194,17 +194,6 @@ 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 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 GetName(); virtual void PrintName(StringStream* stream); @@ -476,8 +465,6 @@ class ICCompareStub: public CodeStub { virtual void Generate(MacroAssembler* masm); - void set_known_map(Handle map) { known_map_ = map; } - private: class OpField: public BitField { }; class StateField: public BitField { }; @@ -497,18 +484,12 @@ 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 new_object); - virtual bool FindCodeInSpecialCache(Code** code_out); - virtual bool UseSpecialCache() { return state_ == CompareIC::KNOWN_OBJECTS; } - Token::Value op_; CompareIC::State state_; - Handle known_map_; }; diff --git a/src/heap.cc b/src/heap.cc index bc7550ed9a..8d3ed539a9 100644 --- a/src/heap.cc +++ b/src/heap.cc @@ -2417,7 +2417,6 @@ 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); diff --git a/src/heap.h b/src/heap.h index d92a4fb148..741e3d9779 100644 --- a/src/heap.h +++ b/src/heap.h @@ -245,7 +245,6 @@ 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") diff --git a/src/hydrogen.cc b/src/hydrogen.cc index b465e97688..519b4b84ba 100644 --- a/src/hydrogen.cc +++ b/src/hydrogen.cc @@ -6141,27 +6141,14 @@ void HGraphBuilder::VisitCompareOperation(CompareOperation* expr) { switch (op) { case Token::EQ: case Token::EQ_STRICT: { - // Can we get away with map check and not instance type check? - Handle 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"); diff --git a/src/ia32/code-stubs-ia32.cc b/src/ia32/code-stubs-ia32.cc index 32ed1f9bfa..fa706fbf50 100644 --- a/src/ia32/code-stubs-ia32.cc +++ b/src/ia32/code-stubs-ia32.cc @@ -6670,45 +6670,33 @@ void ICCompareStub::GenerateObjects(MacroAssembler* 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); } diff --git a/src/ia32/ic-ia32.cc b/src/ia32/ic-ia32.cc index a83db129a1..e93353e5dd 100644 --- a/src/ia32/ic-ia32.cc +++ b/src/ia32/ic-ia32.cc @@ -1625,9 +1625,6 @@ void CompareIC::UpdateCaches(Handle x, Handle y) { rewritten = stub.GetCode(); } else { ICCompareStub stub(op_, state); - if (state == KNOWN_OBJECTS) { - stub.set_known_map(Handle(Handle::cast(x)->map())); - } rewritten = stub.GetCode(); } set_target(*rewritten); diff --git a/src/ic.cc b/src/ic.cc index ee66a43511..d816e7765e 100644 --- a/src/ic.cc +++ b/src/ic.cc @@ -2320,7 +2320,6 @@ 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"; @@ -2335,37 +2334,20 @@ CompareIC::State CompareIC::TargetState(State state, bool has_inlined_smi_code, Handle x, Handle 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::cast(x)->map() == - Handle::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; } diff --git a/src/ic.h b/src/ic.h index 94e83dc519..65cade4242 100644 --- a/src/ic.h +++ b/src/ic.h @@ -724,7 +724,6 @@ class CompareIC: public IC { SYMBOLS, STRINGS, OBJECTS, - KNOWN_OBJECTS, GENERIC }; diff --git a/src/mark-compact.cc b/src/mark-compact.cc index a864c34674..a4b8349d48 100644 --- a/src/mark-compact.cc +++ b/src/mark-compact.cc @@ -883,6 +883,8 @@ 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 && @@ -891,10 +893,9 @@ 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); } diff --git a/src/type-info.cc b/src/type-info.cc index e722d14527..c781c615a7 100644 --- a/src/type-info.cc +++ b/src/type-info.cc @@ -259,7 +259,6 @@ 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: @@ -279,19 +278,6 @@ bool TypeFeedbackOracle::IsSymbolCompare(CompareOperation* expr) { } -Handle TypeFeedbackOracle::GetCompareMap(CompareOperation* expr) { - Handle object = GetInfo(expr->id()); - if (!object->IsCode()) return Handle::null(); - Handle code = Handle::cast(object); - if (!code->is_compare_ic_stub()) return Handle::null(); - CompareIC::State state = static_cast(code->compare_state()); - if (state != CompareIC::KNOWN_OBJECTS) { - return Handle::null(); - } - return Handle(code->FindFirstMap()); -} - - TypeInfo TypeFeedbackOracle::UnaryType(UnaryOperation* expr) { Handle object = GetInfo(expr->id()); TypeInfo unknown = TypeInfo::Unknown(); @@ -381,7 +367,6 @@ 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: diff --git a/src/type-info.h b/src/type-info.h index eba098737d..7c9c05ef08 100644 --- a/src/type-info.h +++ b/src/type-info.h @@ -273,7 +273,6 @@ class TypeFeedbackOracle BASE_EMBEDDED { TypeInfo BinaryType(BinaryOperation* expr); TypeInfo CompareType(CompareOperation* expr); bool IsSymbolCompare(CompareOperation* expr); - Handle GetCompareMap(CompareOperation* expr); TypeInfo SwitchType(CaseClause* clause); TypeInfo IncrementType(CountOperation* expr);