I would like to restrict this a bit at first. I don't think we want to
generate
ICs for all combinations of slow and fast case objects. The stubs will be
huge
and it will not be clear where to cache them. In order to keep this small
and
isolated at first I would like to start out with only doing this for a
slow-case
receiver with the property on a fast case prototype.
First batch of comments included.
http://codereview.chromium.org/2801018/diff/39001/9002
File src/arm/stub-cache-arm.cc (right):
http://codereview.chromium.org/2801018/diff/39001/9002#newcode745
src/arm/stub-cache-arm.cc:745: Register) {
Let's move this register argument to before/after scratch and give it a
name?
http://codereview.chromium.org/2801018/diff/39001/9003
File src/globals.h (right):
http://codereview.chromium.org/2801018/diff/39001/9003#newcode468
src/globals.h:468: PROTOTYPE_MAP // For hashes (except GlobalObjects).
hashes -> slow properties objects?
http://codereview.chromium.org/2801018/diff/39001/9006
File src/ia32/stub-cache-ia32.cc (right):
http://codereview.chromium.org/2801018/diff/39001/9006#newcode154
src/ia32/stub-cache-ia32.cc:154: static const int kProbes = 4;
This loop has to be different. We need to check for deleted elements in
the dictionary. We need to skip deleted elements until we reach an
element that is not deleted. At that point, if the element is absent we
can proceed.
http://codereview.chromium.org/2801018/diff/39001/9006#newcode858
src/ia32/stub-cache-ia32.cc:858: // fast and global obgects or do
negative lookup for normal objects.
obgects -> objects
http://codereview.chromium.org/2801018/diff/39001/9007
File src/ic-inl.h (right):
http://codereview.chromium.org/2801018/diff/39001/9007#newcode97
src/ic-inl.h:97: !object->HasFastProperties() &&
I don't think this is what we want to do. I don't think we want to use
the prototype map for all stubs generated for objects that do not have
fast properties. I think we should only use the prototype map for the
new call stubs.
http://codereview.chromium.org/2801018/diff/39001/9008
File src/ic.cc (right):
http://codereview.chromium.org/2801018/diff/39001/9008#newcode138
src/ic.cc:138: // Returns false when generating a stub is possible, but
effect is usually
I don't understand why these conditions are right? Please explain why
these conditions make sense?
http://codereview.chromium.org/2801018/diff/39001/9008#newcode140
src/ic.cc:140: static bool DoesWorthToCache(LookupResult* lookup,
Object* receiver) {
DoesWorthToCache -> IsWorthCaching
http://codereview.chromium.org/2801018/diff/39001/9008#newcode150
src/ic.cc:150: static bool
HashNormalObjectsInPrototypeChain(LookupResult* lookup,
Hash -> Has?
http://codereview.chromium.org/2801018/diff/39001/9008#newcode177
src/ic.cc:177: if (cache_holder == OWN_MAP &&
I don't understand why you have to change anything for the case where
the cache holder is the map of the object itself. We should only use the
prototype map for the newly added call stubs?
This code seems to complicated. We should be able to keep this as is
except for the new type of IC where we have to do something different.
http://codereview.chromium.org/2801018/diff/39001/9008#newcode183
src/ic.cc:183: // or a global object. IC::GetCodeCacheMap is not
appliable.
appliable -> applicable
http://codereview.chromium.org/2801018/diff/39001/9008#newcode187
src/ic.cc:187: // IC::GetCodeCacheMap is not appliable.
appliable -> applicable
http://codereview.chromium.org/2801018/diff/39001/9008#newcode645
src/ic.cc:645: // Cache code holding map should be consistent with
I don't understand the need for this change. You only need to change how
this works for the new type of stub. That new type of stub has special
code flags so you can figure out which map to use.
http://codereview.chromium.org/2801018/diff/39001/9009
File src/ic.h (right):
http://codereview.chromium.org/2801018/diff/39001/9009#newcode120
src/ic.h:120: // Determines which map must be used for keeping the code
stub.
I don't think this is the right place to determine. I think we should
leave GetCodeCacheMapForObject intact and split this in the IC
LoadFunction code instead.
http://codereview.chromium.org/2801018/diff/39001/9010
File src/objects-inl.h (right):
http://codereview.chromium.org/2801018/diff/39001/9010#newcode2274
src/objects-inl.h:2274: if (holder == PROTOTYPE_MAP) bits |=
kFlagsICHolderMask;
kFlagsCacheInPrototypeMapMask?
http://codereview.chromium.org/2801018/show
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
