Another batch of comments. I think we are getting there. Thanks for doing the statistics and simplifications based on them.
IsContextual really means that we are performing a global load. Global loads of properties that are not there throws ReferenceErrors. Loads of other properties that are not there do not throw exceptions and just returns undefined. http://codereview.chromium.org/140069/diff/6006/5020 File src/ia32/macro-assembler-ia32.cc (right): http://codereview.chromium.org/140069/diff/6006/5020#newcode476 Line 476: Register MacroAssembler::CheckMaps(JSObject* object, Register object_reg, Could you add a comment in the header file explaining that a NULL holder will generate map checks all the way to the top of the prototype chain? http://codereview.chromium.org/140069/diff/6006/5020#newcode497 Line 497: Object* proto = object->GetPrototype(); This code is really part of the loop termination condition. How about replacing the while with a for: // Check all maps to the holder or to the top of the prototype chain. for (Object* proto = object->GetPrototype(); proto != Heap::null_value() && object != holder; proto = proto->GetPrototype()) { // Generate code } http://codereview.chromium.org/140069/diff/6006/5024 File src/ia32/stub-cache-ia32.cc (right): http://codereview.chromium.org/140069/diff/6006/5024#newcode398 Line 398: struct LoadInterceptorCompiler { Make this a class with public and a private part. Put the constructor before the methods and have 'name' be a private field. Since these are only supposed to be allocate on the stack, you should use class LoadInterceptorCompiler BASE_EMBEDDED { // ... } http://codereview.chromium.org/140069/diff/6006/5024#newcode407 Line 407: // Prepare tail call to follow Add period at the end of comment. http://codereview.chromium.org/140069/diff/6006/5024#newcode443 Line 443: // Cleanup a garbage of prepared second call How about just: "Pop lookup result." Above, you could add a "Push lookup result." comment where you do the three calls to push. http://codereview.chromium.org/140069/diff/6006/5024#newcode446 Line 446: __ push(scratch2); Do you mean scratch1 here? Do we have a test case that gets here? http://codereview.chromium.org/140069/diff/6006/5024#newcode471 Line 471: struct CallInterceptorCompiler { Make this a class as well. http://codereview.chromium.org/140069/diff/6006/5025 File src/stub-cache.cc (right): http://codereview.chromium.org/140069/diff/6006/5025#newcode785 Line 785: * but doesn't look down if interceptor returns nothing. 'look down' -> 'search the prototype chain'? http://codereview.chromium.org/140069/diff/6006/5026 File src/stub-cache.h (right): http://codereview.chromium.org/140069/diff/6006/5026#newcode308 Line 308: Object* ThrowUndefinedException(Arguments args); This one is unused now, isn't it? If so, please remove. http://codereview.chromium.org/140069 --~--~---------~--~----~------------~-------~--~----~ v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev -~----------~----~----~----~------~----~------~--~---
