If you change the CheckMaps to CheckPrototypes and add a few tests of why that is necessary, it LGTM.
http://codereview.chromium.org/140069/diff/6055/6057 File src/ia32/stub-cache-ia32.cc (right): http://codereview.chromium.org/140069/diff/6055/6057#newcode365 Line 365: if (lookup->type() == FIELD) { I think you should a comment here that explains in words what kind of intercepted loads you can optimize. http://codereview.chromium.org/140069/diff/6055/6057#newcode371 Line 371: optimize = callback->getter() != 0; 0 -> NULL? http://codereview.chromium.org/140069/diff/6055/6057#newcode380 Line 380: __ EnterInternalFrame(); Maybe explain why you need a frame here? http://codereview.chromium.org/140069/diff/6055/6057#newcode409 Line 409: holder = masm->CheckMaps(holder_obj, holder, lookup->holder(), CheckPrototypes. http://codereview.chromium.org/140069/diff/6055/6057#newcode418 Line 418: ASSERT(callback); callback != NULL http://codereview.chromium.org/140069/diff/6055/6057#newcode419 Line 419: ASSERT(callback->getter()); callback->getter() != NULL http://codereview.chromium.org/140069/diff/6055/6057#newcode426 Line 426: holder = masm->CheckMaps(holder_obj, holder, lookup->holder(), CheckPrototypes. http://codereview.chromium.org/140069/diff/6055/6057#newcode463 Line 463: explicit LoadInterceptorCompiler(Register name) : name(name) {} Constructor should go before the methods. http://codereview.chromium.org/140069/diff/6055/6057#newcode466 Line 466: Register name; name_? http://codereview.chromium.org/140069/diff/6055/6057#newcode485 Line 485: if (function->is_compiled() && !holder_obj->IsJSArray()) { I think this deserves a comment. Why is it a problem if holder_obj is a JS array? http://codereview.chromium.org/140069/diff/6055/6057#newcode512 Line 512: masm->CheckMaps(holder_obj, receiver, lookup->holder(), Please rewrite using CheckPrototypes. http://codereview.chromium.org/140069/diff/6055/6057#newcode557 Line 557: explicit CallInterceptorCompiler(const ParameterCount& arguments) Constructor before methods in definition. http://codereview.chromium.org/140069/diff/6055/6057#newcode561 Line 561: const ParameterCount& arguments; Any good reason not to call these arguments_ and argc_? http://codereview.chromium.org/140069/diff/6055/6056 File test/cctest/test-api.cc (right): http://codereview.chromium.org/140069/diff/6055/6056#newcode5028 Line 5028: // a stub, but interceptor produced value on its own Terminate comment with . http://codereview.chromium.org/140069/diff/6055/6056#newcode5048 Line 5048: // a stub, but it got invalidated later on on -> on. http://codereview.chromium.org/140069/diff/6055/6056#newcode5140 Line 5140: // a stub, but interceptor produced value on its own own -> own. http://codereview.chromium.org/140069/diff/6055/6056#newcode5168 Line 5168: // a stub, but it got invalidated later on Ditto. http://codereview.chromium.org/140069/diff/6055/6056#newcode5286 Line 5286: // a value, we can fetch regular value Ditto. http://codereview.chromium.org/140069 --~--~---------~--~----~------------~-------~--~----~ v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev -~----------~----~----~----~------~----~------~--~---
