Revision: 4426
Author: [email protected]
Date: Thu Apr 15 04:25:41 2010
Log: Reapply load ICs for nonexistent properties.
We need to be careful to check global property cells for the property
encountered during lookup. Therefore, the ICs have to be specific to
the name of the property if global objects are involved. In
principle, this means that we could get a large number of monomorphic
ICs for the same map if there is a global object in the prototype
chain. However, since this is only done for normal load ICs and not
for keyed load ICs I do not expect this to be a problem. I will
experiment with it once this goes in.
BUG=675
Review URL: http://codereview.chromium.org/1559033
http://code.google.com/p/v8/source/detail?r=4426
Added:
/branches/bleeding_edge/test/mjsunit/regress/regress-675.js
Modified:
/branches/bleeding_edge/src/arm/stub-cache-arm.cc
/branches/bleeding_edge/src/globals.h
/branches/bleeding_edge/src/ia32/stub-cache-ia32.cc
/branches/bleeding_edge/src/ic.cc
/branches/bleeding_edge/src/stub-cache.cc
/branches/bleeding_edge/src/stub-cache.h
/branches/bleeding_edge/src/x64/stub-cache-x64.cc
=======================================
--- /dev/null
+++ /branches/bleeding_edge/test/mjsunit/regress/regress-675.js Thu Apr 15
04:25:41 2010
@@ -0,0 +1,61 @@
+// 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:
+//
+// * Redistributions of source code must retain the above copyright
+// notice, this list of conditions and the following disclaimer.
+// * Redistributions in binary form must reproduce the above
+// copyright notice, this list of conditions and the following
+// disclaimer in the documentation and/or other materials provided
+// with the distribution.
+// * Neither the name of Google Inc. nor the names of its
+// contributors may be used to endorse or promote products derived
+// from this software without specific prior written permission.
+//
+// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+// Regression test for http://code.google.com/p/v8/issues/detail?id=675.
+//
+// Test that load ICs for nonexistent properties check global
+// property cells.
+
+function f() { return this.x; }
+
+// Initialize IC for nonexistent x property on global object.
+f();
+f();
+
+// Assign to global property cell for x.
+this.x = 23;
+
+// Check that we bail out from the IC.
+assertEquals(23, f());
+
+
+// Same test, but test that the global property cell is also checked
+// if the global object is the last object in the prototype chain for
+// the load.
+this.__proto__ = null;
+function g() { return this.y; }
+
+// Initialize IC.
+g();
+g();
+
+// Update global property cell.
+this.y = 42;
+
+// Check that IC bails out.
+assertEquals(42, g());
+
=======================================
--- /branches/bleeding_edge/src/arm/stub-cache-arm.cc Wed Apr 14 04:45:03
2010
+++ /branches/bleeding_edge/src/arm/stub-cache-arm.cc Thu Apr 15 04:25:41
2010
@@ -598,6 +598,28 @@
miss);
}
}
+
+
+// Generate code to check that a global property cell is empty. Create
+// the property cell at compilation time if no cell exists for the
+// property.
+static Object* GenerateCheckPropertyCell(MacroAssembler* masm,
+ GlobalObject* global,
+ String* name,
+ Register scratch,
+ Label* miss) {
+ Object* probe = global->EnsurePropertyCell(name);
+ if (probe->IsFailure()) return probe;
+ JSGlobalPropertyCell* cell = JSGlobalPropertyCell::cast(probe);
+ ASSERT(cell->value()->IsTheHole());
+ __ mov(scratch, Operand(Handle<Object>(cell)));
+ __ ldr(scratch,
+ FieldMemOperand(scratch, JSGlobalPropertyCell::kValueOffset));
+ __ LoadRoot(ip, Heap::kTheHoleValueRootIndex);
+ __ cmp(scratch, ip);
+ __ b(ne, miss);
+ return cell;
+}
#undef __
@@ -620,23 +642,19 @@
masm()->CheckMaps(object, object_reg, holder, holder_reg, scratch,
miss);
// If we've skipped any global objects, it's not enough to verify
- // that their maps haven't changed.
+ // that their maps haven't changed. We also need to check that the
+ // property cell for the property is still empty.
while (object != holder) {
if (object->IsGlobalObject()) {
- GlobalObject* global = GlobalObject::cast(object);
- Object* probe = global->EnsurePropertyCell(name);
- if (probe->IsFailure()) {
- set_failure(Failure::cast(probe));
+ Object* cell = GenerateCheckPropertyCell(masm(),
+ GlobalObject::cast(object),
+ name,
+ scratch,
+ miss);
+ if (cell->IsFailure()) {
+ set_failure(Failure::cast(cell));
return result;
}
- JSGlobalPropertyCell* cell = JSGlobalPropertyCell::cast(probe);
- ASSERT(cell->value()->IsTheHole());
- __ mov(scratch, Operand(Handle<Object>(cell)));
- __ ldr(scratch,
- FieldMemOperand(scratch, JSGlobalPropertyCell::kValueOffset));
- __ LoadRoot(ip, Heap::kTheHoleValueRootIndex);
- __ cmp(scratch, ip);
- __ b(ne, miss);
}
object = JSObject::cast(object->GetPrototype());
}
@@ -1387,6 +1405,46 @@
// Return the generated code.
return GetCode(NORMAL, name);
}
+
+
+Object* LoadStubCompiler::CompileLoadNonexistent(String* name,
+ JSObject* object,
+ JSObject* last) {
+ // ----------- S t a t e -------------
+ // -- r2 : name
+ // -- lr : return address
+ // -- [sp] : receiver
+ // -----------------------------------
+ Label miss;
+
+ // Load receiver.
+ __ ldr(r0, MemOperand(sp, 0));
+
+ // Check the maps of the full prototype chain.
+ CheckPrototypes(object, r0, last, r3, r1, name, &miss);
+
+ // If the last object in the prototype chain is a global object,
+ // check that the global property cell is empty.
+ if (last->IsGlobalObject()) {
+ Object* cell = GenerateCheckPropertyCell(masm(),
+ GlobalObject::cast(last),
+ name,
+ r1,
+ &miss);
+ if (cell->IsFailure()) return cell;
+ }
+
+ // Return undefined if maps of the full prototype chain are still the
+ // same and no global property with this name contains a value.
+ __ LoadRoot(r0, Heap::kUndefinedValueRootIndex);
+ __ Ret();
+
+ __ bind(&miss);
+ GenerateLoadMiss(masm(), Code::LOAD_IC);
+
+ // Return the generated code.
+ return GetCode(NONEXISTENT, Heap::empty_string());
+}
Object* LoadStubCompiler::CompileLoadField(JSObject* object,
=======================================
--- /branches/bleeding_edge/src/globals.h Wed Apr 14 04:45:03 2010
+++ /branches/bleeding_edge/src/globals.h Thu Apr 15 04:25:41 2010
@@ -428,7 +428,11 @@
CONSTANT_TRANSITION = 6, // only in fast mode
NULL_DESCRIPTOR = 7, // only in fast mode
// All properties before MAP_TRANSITION are real.
- FIRST_PHANTOM_PROPERTY_TYPE = MAP_TRANSITION
+ FIRST_PHANTOM_PROPERTY_TYPE = MAP_TRANSITION,
+ // There are no IC stubs for NULL_DESCRIPTORS. Therefore,
+ // NULL_DESCRIPTOR can be used as the type flag for IC stubs for
+ // nonexistent properties.
+ NONEXISTENT = NULL_DESCRIPTOR
};
=======================================
--- /branches/bleeding_edge/src/ia32/stub-cache-ia32.cc Wed Apr 14 04:45:03
2010
+++ /branches/bleeding_edge/src/ia32/stub-cache-ia32.cc Thu Apr 15 04:25:41
2010
@@ -948,6 +948,26 @@
// Return the value (register eax).
__ ret(0);
}
+
+
+// Generate code to check that a global property cell is empty. Create
+// the property cell at compilation time if no cell exists for the
+// property.
+static Object* GenerateCheckPropertyCell(MacroAssembler* masm,
+ GlobalObject* global,
+ String* name,
+ Register scratch,
+ Label* miss) {
+ Object* probe = global->EnsurePropertyCell(name);
+ if (probe->IsFailure()) return probe;
+ JSGlobalPropertyCell* cell = JSGlobalPropertyCell::cast(probe);
+ ASSERT(cell->value()->IsTheHole());
+ __ mov(scratch, Immediate(Handle<Object>(cell)));
+ __ cmp(FieldOperand(scratch, JSGlobalPropertyCell::kValueOffset),
+ Immediate(Factory::the_hole_value()));
+ __ j(not_equal, miss, not_taken);
+ return cell;
+}
#undef __
@@ -968,21 +988,19 @@
push_at_depth, miss);
// If we've skipped any global objects, it's not enough to verify
- // that their maps haven't changed.
+ // that their maps haven't changed. We also need to check that the
+ // property cell for the property is still empty.
while (object != holder) {
if (object->IsGlobalObject()) {
- GlobalObject* global = GlobalObject::cast(object);
- Object* probe = global->EnsurePropertyCell(name);
- if (probe->IsFailure()) {
- set_failure(Failure::cast(probe));
+ Object* cell = GenerateCheckPropertyCell(masm(),
+ GlobalObject::cast(object),
+ name,
+ scratch,
+ miss);
+ if (cell->IsFailure()) {
+ set_failure(Failure::cast(cell));
return result;
}
- JSGlobalPropertyCell* cell = JSGlobalPropertyCell::cast(probe);
- ASSERT(cell->value()->IsTheHole());
- __ mov(scratch, Immediate(Handle<Object>(cell)));
- __ cmp(FieldOperand(scratch, JSGlobalPropertyCell::kValueOffset),
- Immediate(Factory::the_hole_value()));
- __ j(not_equal, miss, not_taken);
}
object = JSObject::cast(object->GetPrototype());
}
@@ -1947,6 +1965,44 @@
return GetCode(transition == NULL ? FIELD : MAP_TRANSITION, name);
}
+
+Object* LoadStubCompiler::CompileLoadNonexistent(String* name,
+ JSObject* object,
+ JSObject* last) {
+ // ----------- S t a t e -------------
+ // -- eax : receiver
+ // -- ecx : name
+ // -- esp[0] : return address
+ // -----------------------------------
+ Label miss;
+
+ // Check the maps of the full prototype chain. Also check that
+ // global property cells up to (but not including) the last object
+ // in the prototype chain are empty.
+ CheckPrototypes(object, eax, last, ebx, edx, name, &miss);
+
+ // If the last object in the prototype chain is a global object,
+ // check that the global property cell is empty.
+ if (last->IsGlobalObject()) {
+ Object* cell = GenerateCheckPropertyCell(masm(),
+ GlobalObject::cast(last),
+ name,
+ edx,
+ &miss);
+ if (cell->IsFailure()) return cell;
+ }
+
+ // Return undefined if maps of the full prototype chain are still the
+ // same and no global property with this name contains a value.
+ __ mov(eax, Factory::undefined_value());
+ __ ret(0);
+
+ __ bind(&miss);
+ GenerateLoadMiss(masm(), Code::LOAD_IC);
+
+ // Return the generated code.
+ return GetCode(NONEXISTENT, Heap::empty_string());
+}
Object* LoadStubCompiler::CompileLoadField(JSObject* object,
=======================================
--- /branches/bleeding_edge/src/ic.cc Wed Apr 14 04:45:03 2010
+++ /branches/bleeding_edge/src/ic.cc Thu Apr 15 04:25:41 2010
@@ -694,8 +694,8 @@
State state,
Handle<Object> object,
Handle<String> name) {
- // Bail out if we didn't find a result.
- if (!lookup->IsProperty() || !lookup->IsCacheable()) return;
+ // Bail out if the result is not cacheable.
+ if (!lookup->IsCacheable()) return;
// Loading properties from values is not common, so don't try to
// deal with non-JS objects here.
@@ -709,6 +709,9 @@
// Set the target to the pre monomorphic stub to delay
// setting the monomorphic state.
code = pre_monomorphic_stub();
+ } else if (!lookup->IsProperty()) {
+ // Nonexistent property. The result is undefined.
+ code = StubCache::ComputeLoadNonexistent(*name, *receiver);
} else {
// Compute monomorphic stub.
switch (lookup->type()) {
=======================================
--- /branches/bleeding_edge/src/stub-cache.cc Wed Apr 14 04:45:03 2010
+++ /branches/bleeding_edge/src/stub-cache.cc Thu Apr 15 04:25:41 2010
@@ -91,6 +91,38 @@
primary->value = code;
return code;
}
+
+
+Object* StubCache::ComputeLoadNonexistent(String* name, JSObject*
receiver) {
+ // If no global objects are present in the prototype chain, the load
+ // nonexistent IC stub can be shared for all names for a given map
+ // and we use the empty string for the map cache in that case. If
+ // there are global objects involved, we need to check global
+ // property cells in the stub and therefore the stub will be
+ // specific to the name.
+ String* cache_name = Heap::empty_string();
+ if (receiver->IsGlobalObject()) cache_name = name;
+ JSObject* last = receiver;
+ while (last->GetPrototype() != Heap::null_value()) {
+ last = JSObject::cast(last->GetPrototype());
+ if (last->IsGlobalObject()) cache_name = name;
+ }
+ // Compile the stub that is either shared for all names or
+ // name specific if there are global objects involved.
+ Code::Flags flags =
+ Code::ComputeMonomorphicFlags(Code::LOAD_IC, NONEXISTENT);
+ Object* code = receiver->map()->FindInCodeCache(cache_name, flags);
+ if (code->IsUndefined()) {
+ LoadStubCompiler compiler;
+ code = compiler.CompileLoadNonexistent(cache_name, receiver, last);
+ if (code->IsFailure()) return code;
+ PROFILE(CodeCreateEvent(Logger::LOAD_IC_TAG, Code::cast(code),
cache_name));
+ Object* result =
+ receiver->map()->UpdateCodeCache(cache_name, Code::cast(code));
+ if (result->IsFailure()) return result;
+ }
+ return Set(name, receiver->map(), Code::cast(code));
+}
Object* StubCache::ComputeLoadField(String* name,
=======================================
--- /branches/bleeding_edge/src/stub-cache.h Wed Apr 14 04:45:03 2010
+++ /branches/bleeding_edge/src/stub-cache.h Thu Apr 15 04:25:41 2010
@@ -56,6 +56,8 @@
// Computes the right stub matching. Inserts the result in the
// cache before returning. This might compile a stub if needed.
+ static Object* ComputeLoadNonexistent(String* name, JSObject* receiver);
+
static Object* ComputeLoadField(String* name,
JSObject* receiver,
JSObject* holder,
@@ -461,18 +463,25 @@
class LoadStubCompiler: public StubCompiler {
public:
+ Object* CompileLoadNonexistent(String* name,
+ JSObject* object,
+ JSObject* last);
+
Object* CompileLoadField(JSObject* object,
JSObject* holder,
int index,
String* name);
+
Object* CompileLoadCallback(String* name,
JSObject* object,
JSObject* holder,
AccessorInfo* callback);
+
Object* CompileLoadConstant(JSObject* object,
JSObject* holder,
Object* value,
String* name);
+
Object* CompileLoadInterceptor(JSObject* object,
JSObject* holder,
String* name);
@@ -494,17 +503,21 @@
JSObject* object,
JSObject* holder,
int index);
+
Object* CompileLoadCallback(String* name,
JSObject* object,
JSObject* holder,
AccessorInfo* callback);
+
Object* CompileLoadConstant(String* name,
JSObject* object,
JSObject* holder,
Object* value);
+
Object* CompileLoadInterceptor(JSObject* object,
JSObject* holder,
String* name);
+
Object* CompileLoadArrayLength(String* name);
Object* CompileLoadStringLength(String* name);
Object* CompileLoadFunctionPrototype(String* name);
=======================================
--- /branches/bleeding_edge/src/x64/stub-cache-x64.cc Wed Apr 14 04:45:03
2010
+++ /branches/bleeding_edge/src/x64/stub-cache-x64.cc Thu Apr 15 04:25:41
2010
@@ -647,6 +647,26 @@
const ParameterCount& arguments_;
Register name_;
};
+
+
+// Generate code to check that a global property cell is empty. Create
+// the property cell at compilation time if no cell exists for the
+// property.
+static Object* GenerateCheckPropertyCell(MacroAssembler* masm,
+ GlobalObject* global,
+ String* name,
+ Register scratch,
+ Label* miss) {
+ Object* probe = global->EnsurePropertyCell(name);
+ if (probe->IsFailure()) return probe;
+ JSGlobalPropertyCell* cell = JSGlobalPropertyCell::cast(probe);
+ ASSERT(cell->value()->IsTheHole());
+ __ Move(scratch, Handle<Object>(cell));
+ __ Cmp(FieldOperand(scratch, JSGlobalPropertyCell::kValueOffset),
+ Factory::the_hole_value());
+ __ j(not_equal, miss);
+ return cell;
+}
#undef __
@@ -1142,6 +1162,48 @@
// Return the generated code.
return GetCode(CONSTANT_FUNCTION, name);
}
+
+
+Object* LoadStubCompiler::CompileLoadNonexistent(String* name,
+ JSObject* object,
+ JSObject* last) {
+ // ----------- S t a t e -------------
+ // -- rcx : name
+ // -- rsp[0] : return address
+ // -- rsp[8] : receiver
+ // -----------------------------------
+ Label miss;
+
+ // Load receiver.
+ __ movq(rax, Operand(rsp, kPointerSize));
+
+ // Check the maps of the full prototype chain. Also check that
+ // global property cells up to (but not including) the last object
+ // in the prototype chain are empty.
+ CheckPrototypes(object, rax, last, rbx, rdx, name, &miss);
+
+ // If the last object in the prototype chain is a global object,
+ // check that the global property cell is empty.
+ if (last->IsGlobalObject()) {
+ Object* cell = GenerateCheckPropertyCell(masm(),
+ GlobalObject::cast(last),
+ name,
+ rdx,
+ &miss);
+ if (cell->IsFailure()) return cell;
+ }
+
+ // Return undefined if maps of the full prototype chain are still the
+ // same and no global property with this name contains a value.
+ __ LoadRoot(rax, Heap::kUndefinedValueRootIndex);
+ __ ret(0);
+
+ __ bind(&miss);
+ GenerateLoadMiss(masm(), Code::LOAD_IC);
+
+ // Return the generated code.
+ return GetCode(NONEXISTENT, Heap::empty_string());
+}
Object* LoadStubCompiler::CompileLoadField(JSObject* object,
@@ -1765,21 +1827,19 @@
__ CheckMaps(object, object_reg, holder, holder_reg, scratch, miss);
// If we've skipped any global objects, it's not enough to verify
- // that their maps haven't changed.
+ // that their maps haven't changed. We also need to check that the
+ // property cell for the property is still empty.
while (object != holder) {
if (object->IsGlobalObject()) {
- GlobalObject* global = GlobalObject::cast(object);
- Object* probe = global->EnsurePropertyCell(name);
- if (probe->IsFailure()) {
- set_failure(Failure::cast(probe));
+ Object* cell = GenerateCheckPropertyCell(masm(),
+ GlobalObject::cast(object),
+ name,
+ scratch,
+ miss);
+ if (cell->IsFailure()) {
+ set_failure(Failure::cast(cell));
return result;
}
- JSGlobalPropertyCell* cell = JSGlobalPropertyCell::cast(probe);
- ASSERT(cell->value()->IsTheHole());
- __ Move(scratch, Handle<Object>(cell));
- __ Cmp(FieldOperand(scratch, JSGlobalPropertyCell::kValueOffset),
- Factory::the_hole_value());
- __ j(not_equal, miss);
}
object = JSObject::cast(object->GetPrototype());
}
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
