From 019b8c4511de0fa4aa92ba7dd051f0e0531e75fd Mon Sep 17 00:00:00 2001 From: "whesse@chromium.org" Date: Mon, 31 May 2010 13:26:12 +0000 Subject: [PATCH] Change the interface of LoadIC on the x64 platform to take its arguments in registers. Review URL: http://codereview.chromium.org/2330003 git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@4759 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- src/arm/ic-arm.cc | 9 ++++---- src/ia32/ic-ia32.cc | 4 ++-- src/x64/codegen-x64.cc | 16 +++++++------ src/x64/debug-x64.cc | 3 ++- src/x64/full-codegen-x64.cc | 20 ++++++++-------- src/x64/ic-x64.cc | 55 +++++++++++++++++++------------------------- src/x64/stub-cache-x64.cc | 31 +++++++++---------------- src/x64/virtual-frame-x64.cc | 12 +++++----- test/mjsunit/delete.js | 27 ++++++++++++++++++---- 9 files changed, 90 insertions(+), 87 deletions(-) diff --git a/src/arm/ic-arm.cc b/src/arm/ic-arm.cc index 80c8765..0510440 100644 --- a/src/arm/ic-arm.cc +++ b/src/arm/ic-arm.cc @@ -56,15 +56,14 @@ static void GenerateDictionaryLoad(MacroAssembler* masm, // // t0 - used to hold the property dictionary. // - // t1 - initially the receiver - // - used for the index into the property dictionary + // t1 - initially the receiver. // - holds the result on exit. // // r3 - used as temporary and to hold the capacity of the property // dictionary. // - // r2 - holds the name of the property and is unchanged. - // r4 - used as temporary. + // r2 - initially holds the name of the property and is unchanged. + // r4 - used to hold the index into the property dictionary. Label done; @@ -542,7 +541,7 @@ void LoadIC::GenerateNormal(MacroAssembler* masm) { __ CheckAccessGlobalProxy(r0, r1, &miss); __ b(&probe); - // Cache miss: Restore receiver from stack and jump to runtime. + // Cache miss: Jump to runtime. __ bind(&miss); GenerateMiss(masm); } diff --git a/src/ia32/ic-ia32.cc b/src/ia32/ic-ia32.cc index 2ba64dc..0efb5d9 100644 --- a/src/ia32/ic-ia32.cc +++ b/src/ia32/ic-ia32.cc @@ -1,4 +1,4 @@ -// Copyright 2006-2008 the V8 project authors. All rights reserved. +// Copyright 2010 the V8 project authors. All rights reserved. // Redistribution and use in source and binary forms, with or without // modification, are permitted provided that the following conditions are // met: @@ -1322,7 +1322,7 @@ void LoadIC::GenerateNormal(MacroAssembler* masm) { __ CheckAccessGlobalProxy(eax, edx, &miss); __ jmp(&probe); - // Cache miss: Restore receiver from stack and jump to runtime. + // Cache miss: Jump to runtime. __ bind(&miss); GenerateMiss(masm); } diff --git a/src/x64/codegen-x64.cc b/src/x64/codegen-x64.cc index 182e94b..c15b08b 100644 --- a/src/x64/codegen-x64.cc +++ b/src/x64/codegen-x64.cc @@ -794,6 +794,7 @@ void CodeGenerator::CallApplyLazy(Expression* applicand, // Load applicand.apply onto the stack. This will usually // give us a megamorphic load site. Not super, but it works. Load(applicand); + frame()->Dup(); Handle name = Factory::LookupAsciiSymbol("apply"); frame()->Push(name); Result answer = frame()->CallLoadIC(RelocInfo::CODE_TARGET); @@ -5791,8 +5792,6 @@ Result CodeGenerator::LoadFromGlobalSlotCheckExtensions( // property case was inlined. Ensure that there is not a test rax // instruction here. masm_->nop(); - // Discard the global object. The result is in answer. - frame_->Drop(); return answer; } @@ -6740,7 +6739,9 @@ class DeferredReferenceGetNamedValue: public DeferredCode { void DeferredReferenceGetNamedValue::Generate() { - __ push(receiver_); + if (!receiver_.is(rax)) { + __ movq(rax, receiver_); + } __ Move(rcx, name_); Handle ic(Builtins::builtin(Builtins::LoadIC_Initialize)); __ Call(ic, RelocInfo::CODE_TARGET); @@ -6757,7 +6758,6 @@ void DeferredReferenceGetNamedValue::Generate() { __ IncrementCounter(&Counters::named_load_inline_miss, 1); if (!dst_.is(rax)) __ movq(dst_, rax); - __ pop(receiver_); } @@ -7418,9 +7418,8 @@ Result CodeGenerator::EmitNamedLoad(Handle name, bool is_contextual) { __ IncrementCounter(&Counters::named_load_inline, 1); deferred->BindExit(); - frame()->Push(&receiver); } - ASSERT(frame()->height() == original_height); + ASSERT(frame()->height() == original_height - 1); return result; } @@ -7569,10 +7568,13 @@ void Reference::GetValue() { Variable* var = expression_->AsVariableProxy()->AsVariable(); bool is_global = var != NULL; ASSERT(!is_global || var->is_global()); + if (persist_after_get_) { + cgen_->frame()->Dup(); + } Result result = cgen_->EmitNamedLoad(GetName(), is_global); cgen_->frame()->Push(&result); if (!persist_after_get_) { - cgen_->UnloadReference(this); + set_unloaded(); } break; } diff --git a/src/x64/debug-x64.cc b/src/x64/debug-x64.cc index 89b98f1..b9d062d 100644 --- a/src/x64/debug-x64.cc +++ b/src/x64/debug-x64.cc @@ -144,9 +144,10 @@ void Debug::GenerateKeyedStoreICDebugBreak(MacroAssembler* masm) { void Debug::GenerateLoadICDebugBreak(MacroAssembler* masm) { // Register state for IC load call (from ic-x64.cc). // ----------- S t a t e ------------- + // -- rax : receiver // -- rcx : name // ----------------------------------- - Generate_DebugBreakCallHelper(masm, rcx.bit(), false); + Generate_DebugBreakCallHelper(masm, rax.bit() | rcx.bit(), false); } diff --git a/src/x64/full-codegen-x64.cc b/src/x64/full-codegen-x64.cc index e4e6a0b..5007b8e 100644 --- a/src/x64/full-codegen-x64.cc +++ b/src/x64/full-codegen-x64.cc @@ -1127,15 +1127,15 @@ void FullCodeGenerator::EmitVariableLoad(Variable* var, Comment cmnt(masm_, "Global variable"); // Use inline caching. Variable name is passed in rcx and the global // object on the stack. - __ push(CodeGenerator::GlobalObject()); __ Move(rcx, var->name()); + __ movq(rax, CodeGenerator::GlobalObject()); Handle ic(Builtins::builtin(Builtins::LoadIC_Initialize)); __ Call(ic, RelocInfo::CODE_TARGET_CONTEXT); // A test rax instruction following the call is used by the IC to // indicate that the inobject property case was inlined. Ensure there // is no test rax instruction here. __ nop(); - DropAndApply(1, context, rax); + Apply(context, rax); } else if (slot != NULL && slot->type() == Slot::LOOKUP) { Comment cmnt(masm_, "Lookup slot"); @@ -1693,14 +1693,12 @@ void FullCodeGenerator::VisitProperty(Property* expr) { Comment cmnt(masm_, "[ Property"); Expression* key = expr->key(); - // Evaluate receiver. - VisitForValue(expr->obj(), kStack); - if (key->IsPropertyName()) { + VisitForValue(expr->obj(), kAccumulator); EmitNamedPropertyLoad(expr); - // Drop receiver left on the stack by IC. - DropAndApply(1, context_, rax); + Apply(context_, rax); } else { + VisitForValue(expr->obj(), kStack); VisitForValue(expr->key(), kStack); EmitKeyedPropertyLoad(expr); // Drop key and receiver left on the stack by IC. @@ -2745,13 +2743,13 @@ void FullCodeGenerator::VisitUnaryOperation(UnaryOperation* expr) { !proxy->var()->is_this() && proxy->var()->is_global()) { Comment cmnt(masm_, "Global variable"); - __ push(CodeGenerator::GlobalObject()); __ Move(rcx, proxy->name()); + __ movq(rax, CodeGenerator::GlobalObject()); Handle ic(Builtins::builtin(Builtins::LoadIC_Initialize)); // Use a regular load, not a contextual load, to avoid a reference // error. __ Call(ic, RelocInfo::CODE_TARGET); - __ movq(Operand(rsp, 0), rax); + __ push(rax); } else if (proxy != NULL && proxy->var()->slot() != NULL && proxy->var()->slot()->type() == Slot::LOOKUP) { @@ -2861,10 +2859,12 @@ void FullCodeGenerator::VisitCountOperation(CountOperation* expr) { if (expr->is_postfix() && context_ != Expression::kEffect) { __ Push(Smi::FromInt(0)); } - VisitForValue(prop->obj(), kStack); if (assign_type == NAMED_PROPERTY) { + VisitForValue(prop->obj(), kAccumulator); + __ push(rax); // Copy of receiver, needed for later store. EmitNamedPropertyLoad(prop); } else { + VisitForValue(prop->obj(), kStack); VisitForValue(prop->key(), kStack); EmitKeyedPropertyLoad(prop); } diff --git a/src/x64/ic-x64.cc b/src/x64/ic-x64.cc index 773d262..d32f364 100644 --- a/src/x64/ic-x64.cc +++ b/src/x64/ic-x64.cc @@ -56,18 +56,20 @@ static void GenerateDictionaryLoad(MacroAssembler* masm, Register r1, Register r2, Register name, + Register r4, DictionaryCheck check_dictionary) { // Register use: // // r0 - used to hold the property dictionary. // - // r1 - initially the receiver - // - used for the index into the property dictionary + // r1 - initially the receiver. + // - unchanged on any jump to miss_label. // - holds the result on exit. // // r2 - used to hold the capacity of the property dictionary. // // name - holds the name of the property and is unchanged. + // r4 - used to hold the index into the property dictionary. Label done; @@ -116,19 +118,19 @@ static void GenerateDictionaryLoad(MacroAssembler* masm, StringDictionary::kElementsStartIndex * kPointerSize; for (int i = 0; i < kProbes; i++) { // Compute the masked index: (hash + i + i * i) & mask. - __ movl(r1, FieldOperand(name, String::kHashFieldOffset)); - __ shrl(r1, Immediate(String::kHashShift)); + __ movl(r4, FieldOperand(name, String::kHashFieldOffset)); + __ shrl(r4, Immediate(String::kHashShift)); if (i > 0) { - __ addl(r1, Immediate(StringDictionary::GetProbeOffset(i))); + __ addl(r4, Immediate(StringDictionary::GetProbeOffset(i))); } - __ and_(r1, r2); + __ and_(r4, r2); // Scale the index by multiplying by the entry size. ASSERT(StringDictionary::kEntrySize == 3); - __ lea(r1, Operand(r1, r1, times_2, 0)); // r1 = r1 * 3 + __ lea(r4, Operand(r4, r4, times_2, 0)); // r4 = r4 * 3 // Check if the key is identical to the name. - __ cmpq(name, Operand(r0, r1, times_pointer_size, + __ cmpq(name, Operand(r0, r4, times_pointer_size, kElementsStartOffset - kHeapObjectTag)); if (i != kProbes - 1) { __ j(equal, &done); @@ -140,14 +142,14 @@ static void GenerateDictionaryLoad(MacroAssembler* masm, // Check that the value is a normal property. __ bind(&done); const int kDetailsOffset = kElementsStartOffset + 2 * kPointerSize; - __ Test(Operand(r0, r1, times_pointer_size, kDetailsOffset - kHeapObjectTag), + __ Test(Operand(r0, r4, times_pointer_size, kDetailsOffset - kHeapObjectTag), Smi::FromInt(PropertyDetails::TypeField::mask())); __ j(not_zero, miss_label); // Get the value at the masked, scaled index. const int kValueOffset = kElementsStartOffset + kPointerSize; __ movq(r1, - Operand(r0, r1, times_pointer_size, kValueOffset - kHeapObjectTag)); + Operand(r0, r4, times_pointer_size, kValueOffset - kHeapObjectTag)); } @@ -501,6 +503,7 @@ void KeyedLoadIC::GenerateGeneric(MacroAssembler* masm) { rcx, rdx, rax, + rdi, DICTIONARY_CHECK_DONE); __ movq(rax, rcx); __ IncrementCounter(&Counters::keyed_load_generic_symbol, 1); @@ -1228,7 +1231,7 @@ static void GenerateNormalHelper(MacroAssembler* masm, // rsp[(argc + 1) * 8] : argument 0 = receiver // ----------------------------------- // Search dictionary - put result in register rdx. - GenerateDictionaryLoad(masm, miss, rax, rdx, rbx, rcx, CHECK_DICTIONARY); + GenerateDictionaryLoad(masm, miss, rax, rdx, rbx, rcx, rdi, CHECK_DICTIONARY); // Move the result to register rdi and check that it isn't a smi. __ movq(rdi, rdx); @@ -1333,13 +1336,13 @@ void LoadIC::ClearInlinedVersion(Address address) { void LoadIC::GenerateMiss(MacroAssembler* masm) { // ----------- S t a t e ------------- + // -- rax : receiver // -- rcx : name // -- rsp[0] : return address - // -- rsp[8] : receiver // ----------------------------------- __ pop(rbx); - __ push(Operand(rsp, 0)); // receiver + __ push(rax); // receiver __ push(rcx); // name __ push(rbx); // return address @@ -1351,14 +1354,12 @@ void LoadIC::GenerateMiss(MacroAssembler* masm) { void LoadIC::GenerateArrayLength(MacroAssembler* masm) { // ----------- S t a t e ------------- + // -- rax : receiver // -- rcx : name // -- rsp[0] : return address - // -- rsp[8] : receiver // ----------------------------------- Label miss; - __ movq(rax, Operand(rsp, kPointerSize)); - StubCompiler::GenerateLoadArrayLength(masm, rax, rdx, &miss); __ bind(&miss); StubCompiler::GenerateLoadMiss(masm, Code::LOAD_IC); @@ -1367,14 +1368,12 @@ void LoadIC::GenerateArrayLength(MacroAssembler* masm) { void LoadIC::GenerateFunctionPrototype(MacroAssembler* masm) { // ----------- S t a t e ------------- + // -- rax : receiver // -- rcx : name // -- rsp[0] : return address - // -- rsp[8] : receiver // ----------------------------------- Label miss; - __ movq(rax, Operand(rsp, kPointerSize)); - StubCompiler::GenerateLoadFunctionPrototype(masm, rax, rdx, rbx, &miss); __ bind(&miss); StubCompiler::GenerateLoadMiss(masm, Code::LOAD_IC); @@ -1383,13 +1382,11 @@ void LoadIC::GenerateFunctionPrototype(MacroAssembler* masm) { void LoadIC::GenerateMegamorphic(MacroAssembler* masm) { // ----------- S t a t e ------------- + // -- rax : receiver // -- rcx : name // -- rsp[0] : return address - // -- rsp[8] : receiver // ----------------------------------- - __ movq(rax, Operand(rsp, kPointerSize)); - // Probe the stub cache. Code::Flags flags = Code::ComputeFlags(Code::LOAD_IC, NOT_IN_LOOP, @@ -1403,14 +1400,12 @@ void LoadIC::GenerateMegamorphic(MacroAssembler* masm) { void LoadIC::GenerateNormal(MacroAssembler* masm) { // ----------- S t a t e ------------- + // -- rax : receiver // -- rcx : name // -- rsp[0] : return address - // -- rsp[8] : receiver // ----------------------------------- Label miss, probe, global; - __ movq(rax, Operand(rsp, kPointerSize)); - // Check that the receiver isn't a smi. __ JumpIfSmi(rax, &miss); @@ -1432,7 +1427,8 @@ void LoadIC::GenerateNormal(MacroAssembler* masm) { // Search the dictionary placing the result in rax. __ bind(&probe); - GenerateDictionaryLoad(masm, &miss, rdx, rax, rbx, rcx, CHECK_DICTIONARY); + GenerateDictionaryLoad(masm, &miss, rdx, rax, rbx, + rcx, rdi, CHECK_DICTIONARY); __ ret(0); // Global object access: Check access rights. @@ -1440,23 +1436,20 @@ void LoadIC::GenerateNormal(MacroAssembler* masm) { __ CheckAccessGlobalProxy(rax, rdx, &miss); __ jmp(&probe); - // Cache miss: Restore receiver from stack and jump to runtime. + // Cache miss: Jump to runtime. __ bind(&miss); - __ movq(rax, Operand(rsp, 1 * kPointerSize)); GenerateMiss(masm); } void LoadIC::GenerateStringLength(MacroAssembler* masm) { // ----------- S t a t e ------------- + // -- rax : receiver // -- rcx : name // -- rsp[0] : return address - // -- rsp[8] : receiver // ----------------------------------- Label miss; - __ movq(rax, Operand(rsp, kPointerSize)); - StubCompiler::GenerateLoadStringLength(masm, rax, rdx, rbx, &miss); __ bind(&miss); StubCompiler::GenerateLoadMiss(masm, Code::LOAD_IC); diff --git a/src/x64/stub-cache-x64.cc b/src/x64/stub-cache-x64.cc index fbd95d9..0340d8f 100644 --- a/src/x64/stub-cache-x64.cc +++ b/src/x64/stub-cache-x64.cc @@ -1318,13 +1318,12 @@ Object* LoadStubCompiler::CompileLoadCallback(String* name, JSObject* holder, AccessorInfo* callback) { // ----------- S t a t e ------------- + // -- rax : receiver // -- rcx : name // -- rsp[0] : return address - // -- rsp[8] : receiver // ----------------------------------- Label miss; - __ movq(rax, Operand(rsp, kPointerSize)); Failure* failure = Failure::InternalError(); bool success = GenerateLoadCallback(object, holder, rax, rcx, rbx, rdx, callback, name, &miss, &failure); @@ -1343,13 +1342,12 @@ Object* LoadStubCompiler::CompileLoadConstant(JSObject* object, Object* value, String* name) { // ----------- S t a t e ------------- + // -- rax : receiver // -- rcx : name // -- rsp[0] : return address - // -- rsp[8] : receiver // ----------------------------------- Label miss; - __ movq(rax, Operand(rsp, kPointerSize)); GenerateLoadConstant(object, holder, rax, rbx, rdx, value, name, &miss); __ bind(&miss); GenerateLoadMiss(masm(), Code::LOAD_IC); @@ -1363,15 +1361,12 @@ Object* LoadStubCompiler::CompileLoadNonexistent(String* name, JSObject* object, JSObject* last) { // ----------- S t a t e ------------- + // -- rax : receiver // -- rcx : name // -- rsp[0] : return address - // -- rsp[8] : receiver // ----------------------------------- Label miss; - // Load receiver. - __ movq(rax, Operand(rsp, kPointerSize)); - // Chech that receiver is not a smi. __ JumpIfSmi(rax, &miss); @@ -1409,13 +1404,12 @@ Object* LoadStubCompiler::CompileLoadField(JSObject* object, int index, String* name) { // ----------- S t a t e ------------- + // -- rax : receiver // -- rcx : name // -- rsp[0] : return address - // -- rsp[8] : receiver // ----------------------------------- Label miss; - __ movq(rax, Operand(rsp, kPointerSize)); GenerateLoadField(object, holder, rax, rbx, rdx, index, name, &miss); __ bind(&miss); GenerateLoadMiss(masm(), Code::LOAD_IC); @@ -1429,16 +1423,15 @@ Object* LoadStubCompiler::CompileLoadInterceptor(JSObject* receiver, JSObject* holder, String* name) { // ----------- S t a t e ------------- + // -- rax : receiver // -- rcx : name // -- rsp[0] : return address - // -- rsp[8] : receiver // ----------------------------------- Label miss; LookupResult lookup; LookupPostInterceptor(holder, name, &lookup); - __ movq(rax, Operand(rsp, kPointerSize)); // TODO(368): Compile in the whole chain: all the interceptors in // prototypes and ultimate answer. GenerateLoadInterceptor(receiver, @@ -1465,15 +1458,12 @@ Object* LoadStubCompiler::CompileLoadGlobal(JSObject* object, String* name, bool is_dont_delete) { // ----------- S t a t e ------------- + // -- rax : receiver // -- rcx : name // -- rsp[0] : return address - // -- rsp[8] : receiver // ----------------------------------- Label miss; - // Get the receiver from the stack. - __ movq(rax, Operand(rsp, kPointerSize)); - // If the object is the holder then we know that it's a global // object which can only happen for contextual loads. In this case, // the receiver cannot be a smi. @@ -1485,19 +1475,20 @@ Object* LoadStubCompiler::CompileLoadGlobal(JSObject* object, CheckPrototypes(object, rax, holder, rbx, rdx, name, &miss); // Get the value from the cell. - __ Move(rax, Handle(cell)); - __ movq(rax, FieldOperand(rax, JSGlobalPropertyCell::kValueOffset)); + __ Move(rbx, Handle(cell)); + __ movq(rbx, FieldOperand(rbx, JSGlobalPropertyCell::kValueOffset)); // Check for deleted property if property can actually be deleted. if (!is_dont_delete) { - __ CompareRoot(rax, Heap::kTheHoleValueRootIndex); + __ CompareRoot(rbx, Heap::kTheHoleValueRootIndex); __ j(equal, &miss); } else if (FLAG_debug_code) { - __ CompareRoot(rax, Heap::kTheHoleValueRootIndex); + __ CompareRoot(rbx, Heap::kTheHoleValueRootIndex); __ Check(not_equal, "DontDelete cells can't contain the hole"); } __ IncrementCounter(&Counters::named_load_global_inline, 1); + __ movq(rax, rbx); __ ret(0); __ bind(&miss); diff --git a/src/x64/virtual-frame-x64.cc b/src/x64/virtual-frame-x64.cc index db316bb..0846604 100644 --- a/src/x64/virtual-frame-x64.cc +++ b/src/x64/virtual-frame-x64.cc @@ -1072,14 +1072,14 @@ void VirtualFrame::MoveResultsToRegisters(Result* a, Result VirtualFrame::CallLoadIC(RelocInfo::Mode mode) { - // Name and receiver are on the top of the frame. The IC expects - // name in rcx and receiver on the stack. It does not drop the - // receiver. + // Name and receiver are on the top of the frame. Both are dropped. + // The IC expects name in rcx and receiver in rax. Handle ic(Builtins::builtin(Builtins::LoadIC_Initialize)); Result name = Pop(); - PrepareForCall(1, 0); // One stack arg, not callee-dropped. - name.ToRegister(rcx); - name.Unuse(); + Result receiver = Pop(); + PrepareForCall(0, 0); // One stack arg, not callee-dropped. + MoveResultsToRegisters(&name, &receiver, rcx, rax); + return RawCallCodeObject(ic, mode); } diff --git a/test/mjsunit/delete.js b/test/mjsunit/delete.js index 6fc15e9..8d4636a 100644 --- a/test/mjsunit/delete.js +++ b/test/mjsunit/delete.js @@ -44,16 +44,11 @@ assertEquals(42, x); assertTrue(delete x); assertTrue(typeof x === 'undefined', "x is gone"); -/**** - * This test relies on DontDelete attributes. This is not - * working yet. - var y = 87; // should have DontDelete attribute assertEquals(87, y); assertFalse(delete y, "don't delete"); assertFalse(typeof y === 'undefined'); assertEquals(87, y); -*/ var o = { x: 42, y: 87 }; assertTrue(has(o, 'x')); @@ -161,3 +156,25 @@ assertFalse(has(a, 1), "delete 1"); assertFalse(has(a, Math.pow(2,30)-1), "delete 2^30-1"); assertFalse(has(a, Math.pow(2,31)-1), "delete 2^31-1"); assertEquals(Math.pow(2,31), a.length); + +// Check that a LoadIC for a dictionary field works, even +// when the dictionary probe misses. +function load_deleted_property_using_IC() { + var x = new Object(); + x.a = 3; + x.b = 4; + x.c = 5; + + delete x.c; + assertEquals(3, load_a(x)); + assertEquals(3, load_a(x)); + delete x.a; + assertTrue(typeof load_a(x) === 'undefined', "x.a is gone"); + assertTrue(typeof load_a(x) === 'undefined', "x.a is gone"); +} + +function load_a(x) { + return x.a; +} + +load_deleted_property_using_IC(); -- 2.7.4