From c9c7f8834e3eb0ad5e4417c4f81ab7963348f985 Mon Sep 17 00:00:00 2001 From: "ager@chromium.org" Date: Wed, 21 Jul 2010 06:59:34 +0000 Subject: [PATCH] Inline in-object property stores on ia32 when in loop and not in top-level code. Review URL: http://codereview.chromium.org/3046006 git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@5105 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- src/arm/ic-arm.cc | 30 ++----------- src/ia32/codegen-ia32.cc | 97 +++++++++++++++++++++++++++++++++++++++- src/ia32/ic-ia32.cc | 83 +++++++++++++++++++++------------- src/ia32/macro-assembler-ia32.cc | 6 ++- src/ia32/virtual-frame-ia32.cc | 2 +- src/ic.cc | 93 +++++++++++++++++++++++++++++++++++++- src/ic.h | 12 +++++ src/x64/ic-x64.cc | 36 +++------------ 8 files changed, 266 insertions(+), 93 deletions(-) diff --git a/src/arm/ic-arm.cc b/src/arm/ic-arm.cc index 97e6148..57d9376 100644 --- a/src/arm/ic-arm.cc +++ b/src/arm/ic-arm.cc @@ -958,14 +958,6 @@ static inline bool IsInlinedICSite(Address address, } -void LoadIC::ClearInlinedVersion(Address address) { - // Reset the map check of the inlined in-object property load (if present) to - // guarantee failure by holding an invalid map (the null value). The offset - // can be patched to anything. - PatchInlinedLoad(address, Heap::null_value(), 0); -} - - bool LoadIC::PatchInlinedLoad(Address address, Object* map, int offset) { // Find the end of the inlined code for handling the load if this is an // inlined IC call site. @@ -996,10 +988,9 @@ bool LoadIC::PatchInlinedLoad(Address address, Object* map, int offset) { } -void KeyedLoadIC::ClearInlinedVersion(Address address) { - // Reset the map check of the inlined keyed load (if present) to - // guarantee failure by holding an invalid map (the null value). - PatchInlinedLoad(address, Heap::null_value()); +bool StoreIC::PatchInlinedStore(Address address, Object* map, int offset) { + // TODO(787): Implement inline stores on arm. + return false; } @@ -1018,21 +1009,6 @@ bool KeyedLoadIC::PatchInlinedLoad(Address address, Object* map) { } -void KeyedStoreIC::ClearInlinedVersion(Address address) { - // Insert null as the elements map to check for. This will make - // sure that the elements fast-case map check fails so that control - // flows to the IC instead of the inlined version. - PatchInlinedStore(address, Heap::null_value()); -} - - -void KeyedStoreIC::RestoreInlinedVersion(Address address) { - // Restore the fast-case elements map check so that the inlined - // version can be used again. - PatchInlinedStore(address, Heap::fixed_array_map()); -} - - bool KeyedStoreIC::PatchInlinedStore(Address address, Object* map) { // Find the end of the inlined code for handling the store if this is an // inlined IC call site. diff --git a/src/ia32/codegen-ia32.cc b/src/ia32/codegen-ia32.cc index 0ac5ce9..b19cf80 100644 --- a/src/ia32/codegen-ia32.cc +++ b/src/ia32/codegen-ia32.cc @@ -5340,6 +5340,11 @@ void CodeGenerator::VisitObjectLiteral(ObjectLiteral* node) { frame_->Dup(); Load(property->value()); Result dummy = frame_->CallStoreIC(Handle::cast(key), false); + // A test eax instruction following the store IC call would + // indicate the presence of an inlined version of the + // store. Add a nop to indicate that there is no such + // inlined version. + __ nop(); dummy.Unuse(); break; } @@ -8861,7 +8866,97 @@ Result CodeGenerator::EmitNamedStore(Handle name, bool is_contextual) { #ifdef DEBUG int expected_height = frame()->height() - (is_contextual ? 1 : 2); #endif - Result result = frame()->CallStoreIC(name, is_contextual); + + Result result; + if (is_contextual || scope()->is_global_scope() || loop_nesting() == 0) { + result = frame()->CallStoreIC(name, is_contextual); + // A test eax instruction following the call signals that the inobject + // property case was inlined. Ensure that there is not a test eax + // instruction here. + __ nop(); + } else { + // Inline the in-object property case. + JumpTarget slow, done; + Label patch_site; + + // Get the value and receiver from the stack. + Result value = frame()->Pop(); + value.ToRegister(); + Result receiver = frame()->Pop(); + receiver.ToRegister(); + + // Allocate result register. + result = allocator()->Allocate(); + ASSERT(result.is_valid() && receiver.is_valid() && value.is_valid()); + + // Check that the receiver is a heap object. + __ test(receiver.reg(), Immediate(kSmiTagMask)); + slow.Branch(zero, &value, &receiver); + + // This is the map check instruction that will be patched (so we can't + // use the double underscore macro that may insert instructions). + // Initially use an invalid map to force a failure. + __ bind(&patch_site); + masm()->cmp(FieldOperand(receiver.reg(), HeapObject::kMapOffset), + Immediate(Factory::null_value())); + // This branch is always a forwards branch so it's always a fixed size + // which allows the assert below to succeed and patching to work. + slow.Branch(not_equal, &value, &receiver); + + // The delta from the patch label to the store offset must be + // statically known. + ASSERT(masm()->SizeOfCodeGeneratedSince(&patch_site) == + StoreIC::kOffsetToStoreInstruction); + + // The initial (invalid) offset has to be large enough to force a 32-bit + // instruction encoding to allow patching with an arbitrary offset. Use + // kMaxInt (minus kHeapObjectTag). + int offset = kMaxInt; + __ mov(FieldOperand(receiver.reg(), offset), value.reg()); + __ mov(result.reg(), Operand(value.reg())); + + // Allocate scratch register for write barrier. + Result scratch = allocator()->Allocate(); + ASSERT(scratch.is_valid() && + result.is_valid() && + receiver.is_valid() && + value.is_valid()); + + // The write barrier clobbers all input registers, so spill the + // receiver and the value. + frame_->Spill(receiver.reg()); + frame_->Spill(value.reg()); + + // Update the write barrier. To save instructions in the inlined + // version we do not filter smis. + Label skip_write_barrier; + __ InNewSpace(receiver.reg(), value.reg(), equal, &skip_write_barrier); + int delta_to_record_write = masm_->SizeOfCodeGeneratedSince(&patch_site); + __ lea(scratch.reg(), Operand(receiver.reg(), offset)); + __ RecordWriteHelper(receiver.reg(), scratch.reg(), value.reg()); + if (FLAG_debug_code) { + __ mov(receiver.reg(), Immediate(BitCast(kZapValue))); + __ mov(value.reg(), Immediate(BitCast(kZapValue))); + __ mov(scratch.reg(), Immediate(BitCast(kZapValue))); + } + __ bind(&skip_write_barrier); + value.Unuse(); + scratch.Unuse(); + receiver.Unuse(); + done.Jump(&result); + + slow.Bind(&value, &receiver); + frame()->Push(&receiver); + frame()->Push(&value); + result = frame()->CallStoreIC(name, is_contextual); + // Encode the offset to the map check instruction and the offset + // to the write barrier store address computation in a test eax + // instruction. + int delta_to_patch_site = masm_->SizeOfCodeGeneratedSince(&patch_site); + __ test(eax, + Immediate((delta_to_record_write << 16) | delta_to_patch_site)); + done.Bind(&result); + } ASSERT_EQ(expected_height, frame()->height()); return result; diff --git a/src/ia32/ic-ia32.cc b/src/ia32/ic-ia32.cc index 062f0f2..cbf710c 100644 --- a/src/ia32/ic-ia32.cc +++ b/src/ia32/ic-ia32.cc @@ -1644,37 +1644,6 @@ void LoadIC::GenerateMiss(MacroAssembler* masm) { // One byte opcode for test eax,0xXXXXXXXX. static const byte kTestEaxByte = 0xA9; - -void LoadIC::ClearInlinedVersion(Address address) { - // Reset the map check of the inlined inobject property load (if - // present) to guarantee failure by holding an invalid map (the null - // value). The offset can be patched to anything. - PatchInlinedLoad(address, Heap::null_value(), kMaxInt); -} - - -void KeyedLoadIC::ClearInlinedVersion(Address address) { - // Insert null as the map to check for to make sure the map check fails - // sending control flow to the IC instead of the inlined version. - PatchInlinedLoad(address, Heap::null_value()); -} - - -void KeyedStoreIC::ClearInlinedVersion(Address address) { - // Insert null as the elements map to check for. This will make - // sure that the elements fast-case map check fails so that control - // flows to the IC instead of the inlined version. - PatchInlinedStore(address, Heap::null_value()); -} - - -void KeyedStoreIC::RestoreInlinedVersion(Address address) { - // Restore the fast-case elements map check so that the inlined - // version can be used again. - PatchInlinedStore(address, Heap::fixed_array_map()); -} - - bool LoadIC::PatchInlinedLoad(Address address, Object* map, int offset) { // The address of the instruction following the call. Address test_instruction_address = @@ -1703,6 +1672,52 @@ bool LoadIC::PatchInlinedLoad(Address address, Object* map, int offset) { } +bool StoreIC::PatchInlinedStore(Address address, Object* map, int offset) { + // The address of the instruction following the call. + Address test_instruction_address = + address + Assembler::kCallTargetAddressOffset; + + // If the instruction following the call is not a test eax, nothing + // was inlined. + if (*test_instruction_address != kTestEaxByte) return false; + + // Extract the encoded deltas from the test eax instruction. + Address encoded_offsets_address = test_instruction_address + 1; + int encoded_offsets = *reinterpret_cast(encoded_offsets_address); + int delta_to_map_check = -(encoded_offsets & 0xFFFF); + int delta_to_record_write = encoded_offsets >> 16; + + // Patch the map to check. The map address is the last 4 bytes of + // the 7-byte operand-immediate compare instruction. + Address map_check_address = test_instruction_address + delta_to_map_check; + Address map_address = map_check_address + 3; + *(reinterpret_cast(map_address)) = map; + + // Patch the offset in the store instruction. The offset is in the + // last 4 bytes of a six byte register-to-memory move instruction. + Address offset_address = + map_check_address + StoreIC::kOffsetToStoreInstruction + 2; + // The offset should have initial value (kMaxInt - 1), cleared value + // (-1) or we should be clearing the inlined version. + ASSERT(*reinterpret_cast(offset_address) == kMaxInt - 1 || + *reinterpret_cast(offset_address) == -1 || + (offset == 0 && map == Heap::null_value())); + *reinterpret_cast(offset_address) = offset - kHeapObjectTag; + + // Patch the offset in the write-barrier code. The offset is the + // last 4 bytes of a six byte lea instruction. + offset_address = map_check_address + delta_to_record_write + 2; + // The offset should have initial value (kMaxInt), cleared value + // (-1) or we should be clearing the inlined version. + ASSERT(*reinterpret_cast(offset_address) == kMaxInt || + *reinterpret_cast(offset_address) == -1 || + (offset == 0 && map == Heap::null_value())); + *reinterpret_cast(offset_address) = offset - kHeapObjectTag; + + return true; +} + + static bool PatchInlinedMapCheck(Address address, Object* map) { Address test_instruction_address = address + Assembler::kCallTargetAddressOffset; @@ -1814,6 +1829,12 @@ void StoreIC::GenerateMiss(MacroAssembler* masm) { } +// The offset from the inlined patch site to the start of the inlined +// store instruction. It is 7 bytes (test reg, imm) plus 6 bytes (jne +// slow_label). +const int StoreIC::kOffsetToStoreInstruction = 13; + + void StoreIC::GenerateArrayLength(MacroAssembler* masm) { // ----------- S t a t e ------------- // -- eax : value diff --git a/src/ia32/macro-assembler-ia32.cc b/src/ia32/macro-assembler-ia32.cc index b3f7c21..bb613ed 100644 --- a/src/ia32/macro-assembler-ia32.cc +++ b/src/ia32/macro-assembler-ia32.cc @@ -98,8 +98,10 @@ void MacroAssembler::InNewSpace(Register object, } -void MacroAssembler::RecordWrite(Register object, int offset, - Register value, Register scratch) { +void MacroAssembler::RecordWrite(Register object, + int offset, + Register value, + Register scratch) { // The compiled code assumes that record write doesn't change the // context register, so we check that none of the clobbered // registers are esi. diff --git a/src/ia32/virtual-frame-ia32.cc b/src/ia32/virtual-frame-ia32.cc index 36774da..ff9132c 100644 --- a/src/ia32/virtual-frame-ia32.cc +++ b/src/ia32/virtual-frame-ia32.cc @@ -1034,7 +1034,7 @@ Result VirtualFrame::CallKeyedLoadIC(RelocInfo::Mode mode) { Result VirtualFrame::CallStoreIC(Handle name, bool is_contextual) { // Value and (if not contextual) receiver are on top of the frame. - // The IC expects name in ecx, value in eax, and receiver in edx. + // The IC expects name in ecx, value in eax, and receiver in edx. Handle ic(Builtins::builtin(Builtins::StoreIC_Initialize)); Result value = Pop(); if (is_contextual) { diff --git a/src/ic.cc b/src/ic.cc index 9bb18f7..a5370a6 100644 --- a/src/ic.cc +++ b/src/ic.cc @@ -277,6 +277,13 @@ void CallICBase::Clear(Address address, Code* target) { } +void KeyedLoadIC::ClearInlinedVersion(Address address) { + // Insert null as the map to check for to make sure the map check fails + // sending control flow to the IC instead of the inlined version. + PatchInlinedLoad(address, Heap::null_value()); +} + + void KeyedLoadIC::Clear(Address address, Code* target) { if (target->ic_state() == UNINITIALIZED) return; // Make sure to also clear the map used in inline fast cases. If we @@ -287,6 +294,14 @@ void KeyedLoadIC::Clear(Address address, Code* target) { } +void LoadIC::ClearInlinedVersion(Address address) { + // Reset the map check of the inlined inobject property load (if + // present) to guarantee failure by holding an invalid map (the null + // value). The offset can be patched to anything. + PatchInlinedLoad(address, Heap::null_value(), 0); +} + + void LoadIC::Clear(Address address, Code* target) { if (target->ic_state() == UNINITIALIZED) return; ClearInlinedVersion(address); @@ -294,12 +309,36 @@ void LoadIC::Clear(Address address, Code* target) { } +void StoreIC::ClearInlinedVersion(Address address) { + // Reset the map check of the inlined inobject property store (if + // present) to guarantee failure by holding an invalid map (the null + // value). The offset can be patched to anything. + PatchInlinedStore(address, Heap::null_value(), 0); +} + + void StoreIC::Clear(Address address, Code* target) { if (target->ic_state() == UNINITIALIZED) return; + ClearInlinedVersion(address); SetTargetAtAddress(address, initialize_stub()); } +void KeyedStoreIC::ClearInlinedVersion(Address address) { + // Insert null as the elements map to check for. This will make + // sure that the elements fast-case map check fails so that control + // flows to the IC instead of the inlined version. + PatchInlinedStore(address, Heap::null_value()); +} + + +void KeyedStoreIC::RestoreInlinedVersion(Address address) { + // Restore the fast-case elements map check so that the inlined + // version can be used again. + PatchInlinedStore(address, Heap::fixed_array_map()); +} + + void KeyedStoreIC::Clear(Address address, Code* target) { if (target->ic_state() == UNINITIALIZED) return; SetTargetAtAddress(address, initialize_stub()); @@ -777,11 +816,13 @@ Object* LoadIC::Load(State state, Handle object, Handle name) { int offset = map->instance_size() + (index * kPointerSize); if (PatchInlinedLoad(address(), map, offset)) { set_target(megamorphic_stub()); - return lookup.holder()->FastPropertyAt(lookup.GetFieldIndex()); #ifdef DEBUG if (FLAG_trace_ic) { PrintF("[LoadIC : inline patch %s]\n", *name->ToCString()); } +#endif + return lookup.holder()->FastPropertyAt(lookup.GetFieldIndex()); +#ifdef DEBUG } else { if (FLAG_trace_ic) { PrintF("[LoadIC : no inline patch %s (patching failed)]\n", @@ -1205,7 +1246,57 @@ Object* StoreIC::Store(State state, // Lookup the property locally in the receiver. if (FLAG_use_ic && !receiver->IsJSGlobalProxy()) { LookupResult lookup; + if (LookupForWrite(*receiver, *name, &lookup)) { + bool can_be_inlined = + state == UNINITIALIZED && + lookup.IsProperty() && + lookup.holder() == *receiver && + lookup.type() == FIELD && + !receiver->IsAccessCheckNeeded(); + + if (can_be_inlined) { + Map* map = lookup.holder()->map(); + // Property's index in the properties array. If negative we have + // an inobject property. + int index = lookup.GetFieldIndex() - map->inobject_properties(); + if (index < 0) { + // Index is an offset from the end of the object. + int offset = map->instance_size() + (index * kPointerSize); + if (PatchInlinedStore(address(), map, offset)) { + set_target(megamorphic_stub()); +#ifdef DEBUG + if (FLAG_trace_ic) { + PrintF("[StoreIC : inline patch %s]\n", *name->ToCString()); + } +#endif + return receiver->SetProperty(*name, *value, NONE); +#ifdef DEBUG + + } else { + if (FLAG_trace_ic) { + PrintF("[StoreIC : no inline patch %s (patching failed)]\n", + *name->ToCString()); + } + } + } else { + if (FLAG_trace_ic) { + PrintF("[StoreIC : no inline patch %s (not inobject)]\n", + *name->ToCString()); + } + } + } else { + if (state == PREMONOMORPHIC) { + if (FLAG_trace_ic) { + PrintF("[StoreIC : no inline patch %s (not inlinable)]\n", + *name->ToCString()); +#endif + } + } + } + + // If no inlined store ic was patched, generate a stub for this + // store. UpdateCaches(&lookup, state, receiver, name, value); } } diff --git a/src/ic.h b/src/ic.h index 0d5df96..a02f272 100644 --- a/src/ic.h +++ b/src/ic.h @@ -391,6 +391,13 @@ class StoreIC: public IC { static void GenerateArrayLength(MacroAssembler* masm); static void GenerateNormal(MacroAssembler* masm); + // Clear the use of an inlined version. + static void ClearInlinedVersion(Address address); + + // The offset from the inlined patch site to the start of the + // inlined store instruction. + static const int kOffsetToStoreInstruction; + private: // Update the inline cache and the global stub cache based on the // lookup result. @@ -408,6 +415,11 @@ class StoreIC: public IC { } static void Clear(Address address, Code* target); + + // Support for patching the index and the map that is checked in an + // inlined version of the named store. + static bool PatchInlinedStore(Address address, Object* map, int index); + friend class IC; }; diff --git a/src/x64/ic-x64.cc b/src/x64/ic-x64.cc index d04a7dc..2af887c 100644 --- a/src/x64/ic-x64.cc +++ b/src/x64/ic-x64.cc @@ -418,28 +418,6 @@ bool KeyedStoreIC::PatchInlinedStore(Address address, Object* map) { } -void KeyedLoadIC::ClearInlinedVersion(Address address) { - // Insert null as the map to check for to make sure the map check fails - // sending control flow to the IC instead of the inlined version. - PatchInlinedLoad(address, Heap::null_value()); -} - - -void KeyedStoreIC::ClearInlinedVersion(Address address) { - // Insert null as the elements map to check for. This will make - // sure that the elements fast-case map check fails so that control - // flows to the IC instead of the inlined version. - PatchInlinedStore(address, Heap::null_value()); -} - - -void KeyedStoreIC::RestoreInlinedVersion(Address address) { - // Restore the fast-case elements map check so that the inlined - // version can be used again. - PatchInlinedStore(address, Heap::fixed_array_map()); -} - - void KeyedLoadIC::GenerateMiss(MacroAssembler* masm) { // ----------- S t a t e ------------- // -- rax : key @@ -1630,14 +1608,6 @@ void KeyedCallIC::GenerateNormal(MacroAssembler* masm, int argc) { const int LoadIC::kOffsetToLoadInstruction = 20; -void LoadIC::ClearInlinedVersion(Address address) { - // Reset the map check of the inlined inobject property load (if - // present) to guarantee failure by holding an invalid map (the null - // value). The offset can be patched to anything. - PatchInlinedLoad(address, Heap::null_value(), kMaxInt); -} - - void LoadIC::GenerateMiss(MacroAssembler* masm) { // ----------- S t a t e ------------- // -- rax : receiver @@ -1767,6 +1737,12 @@ bool LoadIC::PatchInlinedLoad(Address address, Object* map, int offset) { } +bool StoreIC::PatchInlinedStore(Address address, Object* map, int offset) { + // TODO(787): Implement inline stores on x64. + return false; +} + + void StoreIC::GenerateMiss(MacroAssembler* masm) { // ----------- S t a t e ------------- // -- rax : value -- 2.7.4