From 541840f8c16cb68d2290322fd994886236cbd909 Mon Sep 17 00:00:00 2001 From: "mstarzinger@chromium.org" Date: Thu, 12 Sep 2013 09:09:39 +0000 Subject: [PATCH] Refactoring PropertyCallbackInfo & FunctionCallbackInfo, step 1. MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit The goal is to unify PropertyCallbackInfo and FunctionCallbackInfo so that they contain the same fields. The field order will be: holder isolate return value default value return value data this This step 1 reorders the PropertyCallbackInfo fields. BUG= R=dcarney@chromium.org, mstarzinger@chromium.org Review URL: https://codereview.chromium.org/23620036 Patch from Marja Hölttä . git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@16673 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- include/v8.h | 10 +++++----- src/arguments.h | 4 ++++ src/arm/stub-cache-arm.cc | 27 +++++++++++++++++---------- src/ia32/stub-cache-ia32.cc | 24 +++++++++++++++++------- src/stub-cache.cc | 41 ++++++++++++++++------------------------- src/stub-cache.h | 10 ++++++++++ src/x64/stub-cache-x64.cc | 26 +++++++++++++++++++------- 7 files changed, 88 insertions(+), 54 deletions(-) diff --git a/include/v8.h b/include/v8.h index 7ffddea..de27338 100644 --- a/include/v8.h +++ b/include/v8.h @@ -2407,11 +2407,11 @@ class PropertyCallbackInfo { friend class internal::PropertyCallbackArguments; friend class internal::CustomArguments; static const int kThisIndex = 0; - static const int kHolderIndex = -1; - static const int kDataIndex = -2; - static const int kReturnValueIndex = -3; - static const int kReturnValueDefaultValueIndex = -4; - static const int kIsolateIndex = -5; + static const int kDataIndex = -1; + static const int kReturnValueIndex = -2; + static const int kReturnValueDefaultValueIndex = -3; + static const int kIsolateIndex = -4; + static const int kHolderIndex = -5; V8_INLINE PropertyCallbackInfo(internal::Object** args) : args_(args) {} internal::Object** args_; diff --git a/src/arguments.h b/src/arguments.h index 169528b..c1db98b 100644 --- a/src/arguments.h +++ b/src/arguments.h @@ -175,6 +175,10 @@ class PropertyCallbackArguments static const int kArgsLength = T::kArgsLength; static const int kThisIndex = T::kThisIndex; static const int kHolderIndex = T::kHolderIndex; + static const int kDataIndex = T::kDataIndex; + static const int kReturnValueDefaultValueIndex = + T::kReturnValueDefaultValueIndex; + static const int kIsolateIndex = T::kIsolateIndex; PropertyCallbackArguments(Isolate* isolate, Object* data, diff --git a/src/arm/stub-cache-arm.cc b/src/arm/stub-cache-arm.cc index 5bab086..a73b9f7 100644 --- a/src/arm/stub-cache-arm.cc +++ b/src/arm/stub-cache-arm.cc @@ -785,6 +785,11 @@ static void PushInterceptorArguments(MacroAssembler* masm, Register holder, Register name, Handle holder_obj) { + STATIC_ASSERT(StubCache::kInterceptorArgsNameIndex == 0); + STATIC_ASSERT(StubCache::kInterceptorArgsInfoIndex == 1); + STATIC_ASSERT(StubCache::kInterceptorArgsThisIndex == 2); + STATIC_ASSERT(StubCache::kInterceptorArgsHolderIndex == 3); + STATIC_ASSERT(StubCache::kInterceptorArgsLength == 4); __ push(name); Handle interceptor(holder_obj->GetNamedInterceptor()); ASSERT(!masm->isolate()->heap()->InNewSpace(*interceptor)); @@ -793,10 +798,6 @@ static void PushInterceptorArguments(MacroAssembler* masm, __ push(scratch); __ push(receiver); __ push(holder); - __ ldr(scratch, FieldMemOperand(scratch, InterceptorInfo::kDataOffset)); - __ push(scratch); - __ mov(scratch, Operand(ExternalReference::isolate_address(masm->isolate()))); - __ push(scratch); } @@ -811,7 +812,7 @@ static void CompileCallLoadPropertyWithInterceptor( ExternalReference ref = ExternalReference(IC_Utility(IC::kLoadPropertyWithInterceptorOnly), masm->isolate()); - __ mov(r0, Operand(6)); + __ mov(r0, Operand(StubCache::kInterceptorArgsLength)); __ mov(r1, Operand(ref)); CEntryStub stub(1); @@ -1110,7 +1111,7 @@ class CallInterceptorCompiler BASE_EMBEDDED { __ CallExternalReference( ExternalReference(IC_Utility(IC::kLoadPropertyWithInterceptorForCall), masm->isolate()), - 6); + StubCache::kInterceptorArgsLength); // Restore the name_ register. __ pop(name_); // Leave the internal frame. @@ -1420,6 +1421,12 @@ void BaseLoadStubCompiler::GenerateLoadCallback( Handle callback) { // Build AccessorInfo::args_ list on the stack and push property name below // the exit frame to make GC aware of them and store pointers to them. + STATIC_ASSERT(PropertyCallbackArguments::kThisIndex == 0); + STATIC_ASSERT(PropertyCallbackArguments::kDataIndex == -1); + STATIC_ASSERT(PropertyCallbackArguments::kReturnValueOffset == -2); + STATIC_ASSERT(PropertyCallbackArguments::kReturnValueDefaultValueIndex == -3); + STATIC_ASSERT(PropertyCallbackArguments::kIsolateIndex == -4); + STATIC_ASSERT(PropertyCallbackArguments::kHolderIndex == -5); __ push(receiver()); __ mov(scratch2(), sp); // scratch2 = AccessorInfo::args_ if (heap()->InNewSpace(callback->data())) { @@ -1429,13 +1436,13 @@ void BaseLoadStubCompiler::GenerateLoadCallback( } else { __ Move(scratch3(), Handle(callback->data(), isolate())); } - __ Push(reg, scratch3()); + __ push(scratch3()); __ LoadRoot(scratch3(), Heap::kUndefinedValueRootIndex); __ mov(scratch4(), scratch3()); __ Push(scratch3(), scratch4()); __ mov(scratch4(), Operand(ExternalReference::isolate_address(isolate()))); - __ Push(scratch4(), name()); + __ Push(scratch4(), reg, name()); __ mov(r0, sp); // r0 = Handle const int kApiStackSpace = 1; @@ -1465,7 +1472,7 @@ void BaseLoadStubCompiler::GenerateLoadCallback( thunk_ref, r2, kStackUnwindSpace, - 5); + 6); } @@ -1553,7 +1560,7 @@ void BaseLoadStubCompiler::GenerateLoadInterceptor( ExternalReference ref = ExternalReference(IC_Utility(IC::kLoadPropertyWithInterceptorForLoad), isolate()); - __ TailCallExternalReference(ref, 6, 1); + __ TailCallExternalReference(ref, StubCache::kInterceptorArgsLength, 1); } } diff --git a/src/ia32/stub-cache-ia32.cc b/src/ia32/stub-cache-ia32.cc index d2da43e..6520fc5 100644 --- a/src/ia32/stub-cache-ia32.cc +++ b/src/ia32/stub-cache-ia32.cc @@ -392,6 +392,11 @@ static void PushInterceptorArguments(MacroAssembler* masm, Register holder, Register name, Handle holder_obj) { + STATIC_ASSERT(StubCache::kInterceptorArgsNameIndex == 0); + STATIC_ASSERT(StubCache::kInterceptorArgsInfoIndex == 1); + STATIC_ASSERT(StubCache::kInterceptorArgsThisIndex == 2); + STATIC_ASSERT(StubCache::kInterceptorArgsHolderIndex == 3); + STATIC_ASSERT(StubCache::kInterceptorArgsLength == 4); __ push(name); Handle interceptor(holder_obj->GetNamedInterceptor()); ASSERT(!masm->isolate()->heap()->InNewSpace(*interceptor)); @@ -400,8 +405,6 @@ static void PushInterceptorArguments(MacroAssembler* masm, __ push(scratch); __ push(receiver); __ push(holder); - __ push(FieldOperand(scratch, InterceptorInfo::kDataOffset)); - __ push(Immediate(reinterpret_cast(masm->isolate()))); } @@ -415,7 +418,7 @@ static void CompileCallLoadPropertyWithInterceptor( __ CallExternalReference( ExternalReference(IC_Utility(IC::kLoadPropertyWithInterceptorOnly), masm->isolate()), - 6); + StubCache::kInterceptorArgsLength); } @@ -733,7 +736,7 @@ class CallInterceptorCompiler BASE_EMBEDDED { __ CallExternalReference( ExternalReference(IC_Utility(IC::kLoadPropertyWithInterceptorForCall), masm->isolate()), - 6); + StubCache::kInterceptorArgsLength); // Restore the name_ register. __ pop(name_); @@ -1401,12 +1404,18 @@ void BaseLoadStubCompiler::GenerateLoadCallback( ASSERT(!scratch3().is(reg)); __ pop(scratch3()); // Get return address to place it below. + STATIC_ASSERT(PropertyCallbackArguments::kThisIndex == 0); + STATIC_ASSERT(PropertyCallbackArguments::kDataIndex == -1); + STATIC_ASSERT(PropertyCallbackArguments::kReturnValueOffset == -2); + STATIC_ASSERT(PropertyCallbackArguments::kReturnValueDefaultValueIndex == -3); + STATIC_ASSERT(PropertyCallbackArguments::kIsolateIndex == -4); + STATIC_ASSERT(PropertyCallbackArguments::kHolderIndex == -5); __ push(receiver()); // receiver __ mov(scratch2(), esp); ASSERT(!scratch2().is(reg)); - __ push(reg); // holder // Push data from ExecutableAccessorInfo. if (isolate()->heap()->InNewSpace(callback->data())) { + ASSERT(!scratch1().is(reg)); __ mov(scratch1(), Immediate(callback)); __ push(FieldOperand(scratch1(), ExecutableAccessorInfo::kDataOffset)); } else { @@ -1416,6 +1425,7 @@ void BaseLoadStubCompiler::GenerateLoadCallback( // ReturnValue default value __ push(Immediate(isolate()->factory()->undefined_value())); __ push(Immediate(reinterpret_cast(isolate()))); + __ push(reg); // holder // Save a pointer to where we pushed the arguments pointer. This will be // passed as the const ExecutableAccessorInfo& to the C++ callback. @@ -1450,7 +1460,7 @@ void BaseLoadStubCompiler::GenerateLoadCallback( thunk_address, ApiParameterOperand(2), kStackSpace, - 6); + 7); } @@ -1557,7 +1567,7 @@ void BaseLoadStubCompiler::GenerateLoadInterceptor( ExternalReference ref = ExternalReference(IC_Utility(IC::kLoadPropertyWithInterceptorForLoad), isolate()); - __ TailCallExternalReference(ref, 6, 1); + __ TailCallExternalReference(ref, StubCache::kInterceptorArgsLength, 1); } } diff --git a/src/stub-cache.cc b/src/stub-cache.cc index bdfb32f..fd2160e 100644 --- a/src/stub-cache.cc +++ b/src/stub-cache.cc @@ -1249,9 +1249,6 @@ RUNTIME_FUNCTION(MaybeObject*, StoreCallbackProperty) { } -static const int kAccessorInfoOffsetInInterceptorArgs = 2; - - /** * Attempts to load a property with an interceptor (which must be present), * but doesn't search the prototype chain. @@ -1260,13 +1257,11 @@ static const int kAccessorInfoOffsetInInterceptorArgs = 2; * provide any value for the given name. */ RUNTIME_FUNCTION(MaybeObject*, LoadPropertyWithInterceptorOnly) { - typedef PropertyCallbackArguments PCA; - static const int kArgsOffset = kAccessorInfoOffsetInInterceptorArgs; - Handle name_handle = args.at(0); - Handle interceptor_info = args.at(1); - ASSERT(kArgsOffset == 2); - // No ReturnValue in interceptors. - ASSERT_EQ(kArgsOffset + PCA::kArgsLength - 2, args.length()); + ASSERT(args.length() == StubCache::kInterceptorArgsLength); + Handle name_handle = + args.at(StubCache::kInterceptorArgsNameIndex); + Handle interceptor_info = + args.at(StubCache::kInterceptorArgsInfoIndex); // TODO(rossberg): Support symbols in the API. if (name_handle->IsSymbol()) @@ -1279,13 +1274,11 @@ RUNTIME_FUNCTION(MaybeObject*, LoadPropertyWithInterceptorOnly) { ASSERT(getter != NULL); Handle receiver = - args.at(kArgsOffset - PCA::kThisIndex); + args.at(StubCache::kInterceptorArgsThisIndex); Handle holder = - args.at(kArgsOffset - PCA::kHolderIndex); - PropertyCallbackArguments callback_args(isolate, - interceptor_info->data(), - *receiver, - *holder); + args.at(StubCache::kInterceptorArgsHolderIndex); + PropertyCallbackArguments callback_args( + isolate, interceptor_info->data(), *receiver, *holder); { // Use the interceptor getter. HandleScope scope(isolate); @@ -1323,17 +1316,15 @@ static MaybeObject* ThrowReferenceError(Isolate* isolate, Name* name) { static MaybeObject* LoadWithInterceptor(Arguments* args, PropertyAttributes* attrs) { - typedef PropertyCallbackArguments PCA; - static const int kArgsOffset = kAccessorInfoOffsetInInterceptorArgs; - Handle name_handle = args->at(0); - Handle interceptor_info = args->at(1); - ASSERT(kArgsOffset == 2); - // No ReturnValue in interceptors. - ASSERT_EQ(kArgsOffset + PCA::kArgsLength - 2, args->length()); + ASSERT(args->length() == StubCache::kInterceptorArgsLength); + Handle name_handle = + args->at(StubCache::kInterceptorArgsNameIndex); + Handle interceptor_info = + args->at(StubCache::kInterceptorArgsInfoIndex); Handle receiver_handle = - args->at(kArgsOffset - PCA::kThisIndex); + args->at(StubCache::kInterceptorArgsThisIndex); Handle holder_handle = - args->at(kArgsOffset - PCA::kHolderIndex); + args->at(StubCache::kInterceptorArgsHolderIndex); Isolate* isolate = receiver_handle->GetIsolate(); diff --git a/src/stub-cache.h b/src/stub-cache.h index 141e567..c2ff18a 100644 --- a/src/stub-cache.h +++ b/src/stub-cache.h @@ -408,6 +408,16 @@ class StubCache { Heap* heap() { return isolate()->heap(); } Factory* factory() { return isolate()->factory(); } + // These constants describe the structure of the interceptor arguments on the + // stack. The arguments are pushed by the (platform-specific) + // PushInterceptorArguments and read by LoadPropertyWithInterceptorOnly and + // LoadWithInterceptor. + static const int kInterceptorArgsNameIndex = 0; + static const int kInterceptorArgsInfoIndex = 1; + static const int kInterceptorArgsThisIndex = 2; + static const int kInterceptorArgsHolderIndex = 3; + static const int kInterceptorArgsLength = 4; + private: explicit StubCache(Isolate* isolate); diff --git a/src/x64/stub-cache-x64.cc b/src/x64/stub-cache-x64.cc index b2cfa86..9b8a04a 100644 --- a/src/x64/stub-cache-x64.cc +++ b/src/x64/stub-cache-x64.cc @@ -29,6 +29,7 @@ #if V8_TARGET_ARCH_X64 +#include "arguments.h" #include "ic-inl.h" #include "codegen.h" #include "stub-cache.h" @@ -366,6 +367,11 @@ static void PushInterceptorArguments(MacroAssembler* masm, Register holder, Register name, Handle holder_obj) { + STATIC_ASSERT(StubCache::kInterceptorArgsNameIndex == 0); + STATIC_ASSERT(StubCache::kInterceptorArgsInfoIndex == 1); + STATIC_ASSERT(StubCache::kInterceptorArgsThisIndex == 2); + STATIC_ASSERT(StubCache::kInterceptorArgsHolderIndex == 3); + STATIC_ASSERT(StubCache::kInterceptorArgsLength == 4); __ push(name); Handle interceptor(holder_obj->GetNamedInterceptor()); ASSERT(!masm->isolate()->heap()->InNewSpace(*interceptor)); @@ -373,8 +379,6 @@ static void PushInterceptorArguments(MacroAssembler* masm, __ push(kScratchRegister); __ push(receiver); __ push(holder); - __ push(FieldOperand(kScratchRegister, InterceptorInfo::kDataOffset)); - __ PushAddress(ExternalReference::isolate_address(masm->isolate())); } @@ -389,7 +393,7 @@ static void CompileCallLoadPropertyWithInterceptor( ExternalReference ref = ExternalReference(IC_Utility(IC::kLoadPropertyWithInterceptorOnly), masm->isolate()); - __ Set(rax, 6); + __ Set(rax, StubCache::kInterceptorArgsLength); __ LoadAddress(rbx, ref); CEntryStub stub(1); @@ -719,7 +723,7 @@ class CallInterceptorCompiler BASE_EMBEDDED { __ CallExternalReference( ExternalReference(IC_Utility(IC::kLoadPropertyWithInterceptorForCall), masm->isolate()), - 6); + StubCache::kInterceptorArgsLength); // Restore the name_ register. __ pop(name_); @@ -1322,19 +1326,27 @@ void BaseLoadStubCompiler::GenerateLoadCallback( ASSERT(!scratch4().is(reg)); __ PopReturnAddressTo(scratch4()); + STATIC_ASSERT(PropertyCallbackArguments::kThisIndex == 0); + STATIC_ASSERT(PropertyCallbackArguments::kDataIndex == -1); + STATIC_ASSERT(PropertyCallbackArguments::kReturnValueOffset == -2); + STATIC_ASSERT(PropertyCallbackArguments::kReturnValueDefaultValueIndex == -3); + STATIC_ASSERT(PropertyCallbackArguments::kIsolateIndex == -4); + STATIC_ASSERT(PropertyCallbackArguments::kHolderIndex == -5); __ push(receiver()); // receiver - __ push(reg); // holder if (heap()->InNewSpace(callback->data())) { + ASSERT(!scratch1().is(reg)); __ Move(scratch1(), callback); __ push(FieldOperand(scratch1(), ExecutableAccessorInfo::kDataOffset)); // data } else { __ Push(Handle(callback->data(), isolate())); } + ASSERT(!kScratchRegister.is(reg)); __ LoadRoot(kScratchRegister, Heap::kUndefinedValueRootIndex); __ push(kScratchRegister); // return value __ push(kScratchRegister); // return value default __ PushAddress(ExternalReference::isolate_address(isolate())); + __ push(reg); // holder __ push(name()); // name // Save a pointer to where we pushed the arguments pointer. This will be // passed as the const ExecutableAccessorInfo& to the C++ callback. @@ -1378,7 +1390,7 @@ void BaseLoadStubCompiler::GenerateLoadCallback( thunk_address, getter_arg, kStackSpace, - 5); + 6); } @@ -1477,7 +1489,7 @@ void BaseLoadStubCompiler::GenerateLoadInterceptor( ExternalReference ref = ExternalReference( IC_Utility(IC::kLoadPropertyWithInterceptorForLoad), isolate()); - __ TailCallExternalReference(ref, 6, 1); + __ TailCallExternalReference(ref, StubCache::kInterceptorArgsLength, 1); } } -- 2.7.4