Re-enable ICs for loads and calls that skips a global object during...

Re-enable ICs for loads and calls that skips a global object during
lookup through the prototype chain.

Committed: http://code.google.com/p/v8/source/detail?r=2425

[antonm, 2009-07-10 09:28:43]

Kasper, LGTM but I am not qualified enough to understand every bit.

I asked you many questions in review---they are mostly for my education, so if
you feel it slows us down, feel free to ignore.

http://codereview.chromium.org/155344/diff/1/8
File src/arm/stub-cache-arm.cc (right):

http://codereview.chromium.org/155344/diff/1/8#newcode361
Line 361: Register StubCompiler::CheckPrototypes(JSObject* object,
maybe it should go to macro-assembler?

http://codereview.chromium.org/155344/diff/1/8#newcode375
Line 375: if (object->IsGlobalObject()) {
just curious, I guess there might be only single global object in prototype
chain (or no)?  If yes, may be could break out of the loop after global object
is met.

http://codereview.chromium.org/155344/diff/1/8#newcode382
Line 382: JSGlobalPropertyCell* cell = JSGlobalPropertyCell::cast(probe);
for my education: why it must be the hole?

http://codereview.chromium.org/155344/diff/1/8#newcode384
Line 384: __ mov(scratch, Operand(Handle<Object>(cell)));
cell always live not in new space, correct?

http://codereview.chromium.org/155344/diff/1/8#newcode390
Line 390: object = JSObject::cast(object->GetPrototype());
just for my education: it's always safe to assume that prototype is JSObject if
we started with JSObject?

http://codereview.chromium.org/155344/diff/1/8#newcode398
Line 398: void StubCompiler::GenerateLoadField(JSObject* object,
cosmetic issue: is the move of methods needed---if you can persuade codereview
to compare former and new versions of methods side by side, it might be easier
to understand the change.  By no means insisting.

http://codereview.chromium.org/155344/diff/1/7
File src/objects-inl.h (right):

http://codereview.chromium.org/155344/diff/1/7#newcode2656
Line 2656: ASSERT(!key->IsString() || details.IsDeleted() || details.index() >
0);
why this change?  just curious.

http://codereview.chromium.org/155344/diff/1/2
File src/objects.cc (right):

http://codereview.chromium.org/155344/diff/1/2#newcode1731
Line 1731: if (!IsGlobalObject()) result->DisallowCaching();
maybe move that up, as else clause to if at 1715?