Revision: 4732
Author: [email protected]
Date: Wed May 26 07:04:37 2010
Log: Various refactorings in interceptor calling and loading.
1) do not push receiver early---that simplifies tail call preparation
on ia32/x64 and renders special cleanup unnecessary;
2) do not do second map check if interceptor's and cached holder
are the same;
3) do not push/pop receiver if receiver and holder registers are the same
(means that receiver is interceptor's holder);
4) do batch pushes on arm;
5) minor cosmetic improvements.
Review URL: http://codereview.chromium.org/2282001
http://code.google.com/p/v8/source/detail?r=4732
Modified:
/branches/bleeding_edge/src/arm/stub-cache-arm.cc
/branches/bleeding_edge/src/ia32/stub-cache-ia32.cc
/branches/bleeding_edge/src/x64/stub-cache-x64.cc
/branches/bleeding_edge/test/cctest/test-api.cc
=======================================
--- /branches/bleeding_edge/src/arm/stub-cache-arm.cc Tue May 25 07:59:11
2010
+++ /branches/bleeding_edge/src/arm/stub-cache-arm.cc Wed May 26 07:04:37
2010
@@ -461,11 +461,16 @@
return;
}
- // Note: starting a frame here makes GC aware of pointers pushed below.
+ // Save necessary data before invoking an interceptor.
+ // Requires a frame to make GC aware of pushed pointers.
__ EnterInternalFrame();
- __ push(receiver);
- __ Push(holder, name_);
+ if (lookup->type() == CALLBACKS && !receiver.is(holder)) {
+ // CALLBACKS case needs a receiver to be passed into C++ callback.
+ __ Push(receiver, holder, name_);
+ } else {
+ __ Push(holder, name_);
+ }
// Invoke an interceptor. Note: map checks from receiver to
// interceptor's holder has been compiled before (see a caller
@@ -488,22 +493,25 @@
__ bind(&interceptor_failed);
__ pop(name_);
__ pop(holder);
- __ pop(receiver);
+ if (lookup->type() == CALLBACKS && !receiver.is(holder)) {
+ __ pop(receiver);
+ }
__ LeaveInternalFrame();
- if (lookup->type() == FIELD) {
- // We found FIELD property in prototype chain of interceptor's
holder.
- // Check that the maps from interceptor's holder to field's holder
- // haven't changed...
- holder = stub_compiler->CheckPrototypes(interceptor_holder,
- holder,
- lookup->holder(),
- scratch1,
+ // Check that the maps from interceptor's holder to lookup's holder
+ // haven't changed. And load lookup's holder into |holder| register.
+ if (interceptor_holder != lookup->holder()) {
+ holder = stub_compiler->CheckPrototypes(interceptor_holder, holder,
+ lookup->holder(), scratch1,
scratch2,
name,
miss_label);
- // ... and retrieve a field from field's holder.
+ }
+
+ if (lookup->type() == FIELD) {
+ // We found FIELD property in prototype chain of interceptor's
holder.
+ // Retrieve a field from field's holder.
stub_compiler->GenerateFastPropertyLoad(masm,
r0,
holder,
@@ -518,33 +526,25 @@
ASSERT(callback != NULL);
ASSERT(callback->getter() != NULL);
- // Prepare for tail call: push receiver to stack.
- Label cleanup;
- __ push(receiver);
-
- // Check that the maps from interceptor's holder to callback's holder
- // haven't changed.
- holder = stub_compiler->CheckPrototypes(interceptor_holder, holder,
- lookup->holder(), scratch1,
- scratch2,
- name,
- &cleanup);
-
- // Continue tail call preparation: push remaining parameters.
- __ push(holder);
- __ Move(holder, Handle<AccessorInfo>(callback));
- __ push(holder);
- __ ldr(scratch1, FieldMemOperand(holder, AccessorInfo::kDataOffset));
- __ Push(scratch1, name_);
-
// Tail call to runtime.
+ // Important invariant in CALLBACKS case: the code above must be
+ // structured to never clobber |receiver| register.
+ __ Move(scratch2, Handle<AccessorInfo>(callback));
+ // holder is either receiver or scratch1.
+ if (!receiver.is(holder)) {
+ ASSERT(scratch1.is(holder));
+ __ Push(receiver, holder, scratch2);
+ __ ldr(scratch1, FieldMemOperand(holder,
AccessorInfo::kDataOffset));
+ __ Push(scratch1, name_);
+ } else {
+ __ push(receiver);
+ __ ldr(scratch1, FieldMemOperand(holder,
AccessorInfo::kDataOffset));
+ __ Push(holder, scratch2, scratch1, name_);
+ }
+
ExternalReference ref =
ExternalReference(IC_Utility(IC::kLoadCallbackProperty));
__ TailCallExternalReference(ref, 5, 1);
-
- // Clean up code: we pushed receiver and need to remove it.
- __ bind(&cleanup);
- __ pop(scratch2);
}
}
@@ -770,9 +770,9 @@
Label miss_cleanup;
Label* miss = can_do_fast_api_call ? &miss_cleanup : miss_label;
Register holder =
- stub_compiler_->CheckPrototypes(object, receiver,
interceptor_holder,
- scratch1, scratch2, name,
- depth1, miss);
+ stub_compiler_->CheckPrototypes(object, receiver,
+ interceptor_holder, scratch1,
+ scratch2, name, depth1, miss);
// Invoke an interceptor and if it provides a value,
// branch to |regular_invoke|.
@@ -785,9 +785,16 @@
// Check that the maps from interceptor's holder to constant function's
// holder haven't changed and thus we can use cached constant function.
- stub_compiler_->CheckPrototypes(interceptor_holder, receiver,
- lookup->holder(), scratch1,
- scratch2, name, depth2, miss);
+ if (interceptor_holder != lookup->holder()) {
+ stub_compiler_->CheckPrototypes(interceptor_holder, receiver,
+ lookup->holder(), scratch1,
+ scratch2, name, depth2, miss);
+ // CheckPrototypes has a side effect of fetching a 'holder'
+ // for API (object which is instanceof for the signature). It's
+ // safe to omit it here, as if present, it should be fetched
+ // by the previous CheckPrototypes.
+ ASSERT((depth2 == kInvalidProtoDepth) || (depth1 !=
kInvalidProtoDepth));
+ }
// Invoke function.
if (can_do_fast_api_call) {
=======================================
--- /branches/bleeding_edge/src/ia32/stub-cache-ia32.cc Wed May 26 02:24:44
2010
+++ /branches/bleeding_edge/src/ia32/stub-cache-ia32.cc Wed May 26 07:04:37
2010
@@ -381,10 +381,12 @@
return;
}
- // Note: starting a frame here makes GC aware of pointers pushed below.
+ // Save necessary data before invoking an interceptor.
+ // Requires a frame to make GC aware of pushed pointers.
__ EnterInternalFrame();
- if (lookup->type() == CALLBACKS) {
+ if (lookup->type() == CALLBACKS && !receiver.is(holder)) {
+ // CALLBACKS case needs a receiver to be passed into C++ callback.
__ push(receiver);
}
__ push(holder);
@@ -410,22 +412,25 @@
__ bind(&interceptor_failed);
__ pop(name_);
__ pop(holder);
- if (lookup->type() == CALLBACKS) {
+ if (lookup->type() == CALLBACKS && !receiver.is(holder)) {
__ pop(receiver);
}
__ LeaveInternalFrame();
- if (lookup->type() == FIELD) {
- // We found FIELD property in prototype chain of interceptor's
holder.
- // Check that the maps from interceptor's holder to field's holder
- // haven't changed...
+ // Check that the maps from interceptor's holder to lookup's holder
+ // haven't changed. And load lookup's holder into |holder| register.
+ if (interceptor_holder != lookup->holder()) {
holder = stub_compiler->CheckPrototypes(interceptor_holder, holder,
lookup->holder(), scratch1,
scratch2,
name,
miss_label);
- // ... and retrieve a field from field's holder.
+ }
+
+ if (lookup->type() == FIELD) {
+ // We found FIELD property in prototype chain of interceptor's
holder.
+ // Retrieve a field from field's holder.
stub_compiler->GenerateFastPropertyLoad(masm, eax,
holder, lookup->holder(),
lookup->GetFieldIndex());
@@ -438,23 +443,11 @@
ASSERT(callback != NULL);
ASSERT(callback->getter() != NULL);
- // Prepare for tail call: push receiver to stack after return
address.
- Label cleanup;
+ // Tail call to runtime.
+ // Important invariant in CALLBACKS case: the code above must be
+ // structured to never clobber |receiver| register.
__ pop(scratch2); // return address
__ push(receiver);
- __ push(scratch2);
-
- // Check that the maps from interceptor's holder to callback's holder
- // haven't changed.
- holder = stub_compiler->CheckPrototypes(interceptor_holder, holder,
- lookup->holder(), scratch1,
- scratch2,
- name,
- &cleanup);
-
- // Continue tail call preparation: push remaining parameters after
- // return address.
- __ pop(scratch2); // return address
__ push(holder);
__ mov(holder, Immediate(Handle<AccessorInfo>(callback)));
__ push(holder);
@@ -462,17 +455,9 @@
__ push(name_);
__ push(scratch2); // restore return address
- // Tail call to runtime.
ExternalReference ref =
ExternalReference(IC_Utility(IC::kLoadCallbackProperty));
__ TailCallExternalReference(ref, 5, 1);
-
- // Clean up code: we pushed receiver after return address and
- // need to remove it from there.
- __ bind(&cleanup);
- __ pop(scratch1); // return address.
- __ pop(scratch2); // receiver.
- __ push(scratch1);
}
}
@@ -683,9 +668,9 @@
Label miss_cleanup;
Label* miss = can_do_fast_api_call ? &miss_cleanup : miss_label;
Register holder =
- stub_compiler_->CheckPrototypes(object, receiver,
interceptor_holder,
- scratch1, scratch2, name,
- depth1, miss);
+ stub_compiler_->CheckPrototypes(object, receiver,
+ interceptor_holder, scratch1,
+ scratch2, name, depth1, miss);
// Invoke an interceptor and if it provides a value,
// branch to |regular_invoke|.
@@ -698,10 +683,16 @@
// Check that the maps from interceptor's holder to constant function's
// holder haven't changed and thus we can use cached constant function.
- stub_compiler_->CheckPrototypes(interceptor_holder, receiver,
- lookup->holder(),
- scratch1, scratch2, name,
- depth2, miss);
+ if (interceptor_holder != lookup->holder()) {
+ stub_compiler_->CheckPrototypes(interceptor_holder, receiver,
+ lookup->holder(), scratch1,
+ scratch2, name, depth2, miss);
+ // CheckPrototypes has a side effect of fetching a 'holder'
+ // for API (object which is instanceof for the signature). It's
+ // safe to omit it here, as if present, it should be fetched
+ // by the previous CheckPrototypes.
+ ASSERT((depth2 == kInvalidProtoDepth) || (depth1 !=
kInvalidProtoDepth));
+ }
// Invoke function.
if (can_do_fast_api_call) {
=======================================
--- /branches/bleeding_edge/src/x64/stub-cache-x64.cc Wed May 26 02:24:44
2010
+++ /branches/bleeding_edge/src/x64/stub-cache-x64.cc Wed May 26 07:04:37
2010
@@ -455,10 +455,12 @@
return;
}
- // Note: starting a frame here makes GC aware of pointers pushed below.
+ // Save necessary data before invoking an interceptor.
+ // Requires a frame to make GC aware of pushed pointers.
__ EnterInternalFrame();
- if (lookup->type() == CALLBACKS) {
+ if (lookup->type() == CALLBACKS && !receiver.is(holder)) {
+ // CALLBACKS case needs a receiver to be passed into C++ callback.
__ push(receiver);
}
__ push(holder);
@@ -484,24 +486,25 @@
__ bind(&interceptor_failed);
__ pop(name_);
__ pop(holder);
- if (lookup->type() == CALLBACKS) {
+ if (lookup->type() == CALLBACKS && !receiver.is(holder)) {
__ pop(receiver);
}
__ LeaveInternalFrame();
- if (lookup->type() == FIELD) {
- // We found FIELD property in prototype chain of interceptor's
holder.
- // Check that the maps from interceptor's holder to field's holder
- // haven't changed...
- holder = stub_compiler->CheckPrototypes(interceptor_holder,
- holder,
- lookup->holder(),
- scratch1,
+ // Check that the maps from interceptor's holder to lookup's holder
+ // haven't changed. And load lookup's holder into |holder| register.
+ if (interceptor_holder != lookup->holder()) {
+ holder = stub_compiler->CheckPrototypes(interceptor_holder, holder,
+ lookup->holder(), scratch1,
scratch2,
name,
miss_label);
- // ... and retrieve a field from field's holder.
+ }
+
+ if (lookup->type() == FIELD) {
+ // We found FIELD property in prototype chain of interceptor's
holder.
+ // Retrieve a field from field's holder.
stub_compiler->GenerateFastPropertyLoad(masm,
rax,
holder,
@@ -516,23 +519,11 @@
ASSERT(callback != NULL);
ASSERT(callback->getter() != NULL);
- // Prepare for tail call. Push receiver to stack after return
address.
- Label cleanup;
+ // Tail call to runtime.
+ // Important invariant in CALLBACKS case: the code above must be
+ // structured to never clobber |receiver| register.
__ pop(scratch2); // return address
__ push(receiver);
- __ push(scratch2);
-
- // Check that the maps from interceptor's holder to callback's holder
- // haven't changed.
- holder = stub_compiler->CheckPrototypes(interceptor_holder, holder,
- lookup->holder(), scratch1,
- scratch2,
- name,
- &cleanup);
-
- // Continue tail call preparation: push remaining parameters after
- // return address.
- __ pop(scratch2); // return address
__ push(holder);
__ Move(holder, Handle<AccessorInfo>(callback));
__ push(holder);
@@ -540,17 +531,9 @@
__ push(name_);
__ push(scratch2); // restore return address
- // Tail call to runtime.
ExternalReference ref =
ExternalReference(IC_Utility(IC::kLoadCallbackProperty));
__ TailCallExternalReference(ref, 5, 1);
-
- // Clean up code: we pushed receiver after return address and
- // need to remove it from there.
- __ bind(&cleanup);
- __ pop(scratch1); // return address
- __ pop(scratch2); // receiver
- __ push(scratch1);
}
}
@@ -761,9 +744,9 @@
Label miss_cleanup;
Label* miss = can_do_fast_api_call ? &miss_cleanup : miss_label;
Register holder =
- stub_compiler_->CheckPrototypes(object, receiver,
interceptor_holder,
- scratch1, scratch2, name,
- depth1, miss);
+ stub_compiler_->CheckPrototypes(object, receiver,
+ interceptor_holder, scratch1,
+ scratch2, name, depth1, miss);
// Invoke an interceptor and if it provides a value,
// branch to |regular_invoke|.
@@ -776,10 +759,16 @@
// Check that the maps from interceptor's holder to constant function's
// holder haven't changed and thus we can use cached constant function.
- stub_compiler_->CheckPrototypes(interceptor_holder, receiver,
- lookup->holder(),
- scratch1, scratch2, name,
- depth2, miss);
+ if (interceptor_holder != lookup->holder()) {
+ stub_compiler_->CheckPrototypes(interceptor_holder, receiver,
+ lookup->holder(), scratch1,
+ scratch2, name, depth2, miss);
+ // CheckPrototypes has a side effect of fetching a 'holder'
+ // for API (object which is instanceof for the signature). It's
+ // safe to omit it here, as if present, it should be fetched
+ // by the previous CheckPrototypes.
+ ASSERT((depth2 == kInvalidProtoDepth) || (depth1 !=
kInvalidProtoDepth));
+ }
// Invoke function.
if (can_do_fast_api_call) {
=======================================
--- /branches/bleeding_edge/test/cctest/test-api.cc Tue May 25 05:14:49 2010
+++ /branches/bleeding_edge/test/cctest/test-api.cc Wed May 26 07:04:37 2010
@@ -6245,12 +6245,25 @@
templ->SetAccessor(v8_str("y"), Return239);
LocalContext context;
context->Global()->Set(v8_str("o"), templ->NewInstance());
+
+ // Check the case when receiver and interceptor's holder
+ // are the same objects.
v8::Handle<Value> value = CompileRun(
"var result = 0;"
"for (var i = 0; i < 7; i++) {"
" result = o.y;"
"}");
CHECK_EQ(239, value->Int32Value());
+
+ // Check the case when interceptor's holder is in proto chain
+ // of receiver.
+ value = CompileRun(
+ "r = { __proto__: o };"
+ "var result = 0;"
+ "for (var i = 0; i < 7; i++) {"
+ " result = r.y;"
+ "}");
+ CHECK_EQ(239, value->Int32Value());
}
@@ -6265,6 +6278,8 @@
context->Global()->Set(v8_str("o"), templ_o->NewInstance());
context->Global()->Set(v8_str("p"), templ_p->NewInstance());
+ // Check the case when receiver and interceptor's holder
+ // are the same objects.
v8::Handle<Value> value = CompileRun(
"o.__proto__ = p;"
"var result = 0;"
@@ -6272,6 +6287,16 @@
" result = o.x + o.y;"
"}");
CHECK_EQ(239 + 42, value->Int32Value());
+
+ // Check the case when interceptor's holder is in proto chain
+ // of receiver.
+ value = CompileRun(
+ "r = { __proto__: o };"
+ "var result = 0;"
+ "for (var i = 0; i < 7; i++) {"
+ " result = r.x + r.y;"
+ "}");
+ CHECK_EQ(239 + 42, value->Int32Value());
}
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
