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

git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@4732 ce2b1a6d-e550-0410-aec6-3dcde31c8c00

diff --git a/src/arm/stub-cache-arm.cc b/src/arm/stub-cache-arm.cc
index d82ef21ce02399c44ed960e76af583c0548f4eac..7d03775aad65a8b66bad908de02942b956e83b07 100644 (file)
--- a/src/arm/stub-cache-arm.cc
+++ b/src/arm/stub-cache-arm.cc
@@ -461,11 +461,16 @@ class LoadInterceptorCompiler BASE_EMBEDDED {
       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 @@ class LoadInterceptorCompiler BASE_EMBEDDED {
     __ 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 @@ class LoadInterceptorCompiler BASE_EMBEDDED {
       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 @@ class CallInterceptorCompiler BASE_EMBEDDED {
     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 @@ class CallInterceptorCompiler BASE_EMBEDDED {
 
     // 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) {

diff --git a/src/ia32/stub-cache-ia32.cc b/src/ia32/stub-cache-ia32.cc
index eb555d705d10a1b5ab48f89e708661bdaa7c11cd..3d73c4628fedbc0cff229af620c10bc296508180 100644 (file)
--- a/src/ia32/stub-cache-ia32.cc
+++ b/src/ia32/stub-cache-ia32.cc
@@ -381,10 +381,12 @@ class LoadInterceptorCompiler BASE_EMBEDDED {
       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 @@ class LoadInterceptorCompiler BASE_EMBEDDED {
     __ 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 @@ class LoadInterceptorCompiler BASE_EMBEDDED {
       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 @@ class LoadInterceptorCompiler BASE_EMBEDDED {
       __ 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 @@ class CallInterceptorCompiler BASE_EMBEDDED {
     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 @@ class CallInterceptorCompiler BASE_EMBEDDED {
 
     // 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) {

diff --git a/src/x64/stub-cache-x64.cc b/src/x64/stub-cache-x64.cc
index 8b095cbbe68bd6d84b131638c057305453152308..f3478dfd9d9d12e71b7f4727746c49295c9b54e4 100644 (file)
--- a/src/x64/stub-cache-x64.cc
+++ b/src/x64/stub-cache-x64.cc
@@ -455,10 +455,12 @@ class LoadInterceptorCompiler BASE_EMBEDDED {
       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 @@ class LoadInterceptorCompiler BASE_EMBEDDED {
     __ 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 @@ class LoadInterceptorCompiler BASE_EMBEDDED {
       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 @@ class LoadInterceptorCompiler BASE_EMBEDDED {
       __ 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 @@ class CallInterceptorCompiler BASE_EMBEDDED {
     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 @@ class CallInterceptorCompiler BASE_EMBEDDED {
 
     // 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) {

diff --git a/test/cctest/test-api.cc b/test/cctest/test-api.cc
index 46eaccd5395f9590526393bdbbdae4340615a987..6254eaa708a7dff85c1c2447821b702688939ab4 100644 (file)
--- a/test/cctest/test-api.cc
+++ b/test/cctest/test-api.cc
@@ -6245,12 +6245,25 @@ THREADED_TEST(InterceptorLoadICWithCallbackOnHolder) {
   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 @@ THREADED_TEST(InterceptorLoadICWithCallbackOnProto) {
   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 @@ THREADED_TEST(InterceptorLoadICWithCallbackOnProto) {
       "  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());
 }