LGTM. Do we have good test coverage of this stuff already?

http://codereview.chromium.org/20419/diff/17/25
File src/codegen-ia32.cc (right):

http://codereview.chromium.org/20419/diff/17/25#newcode465
Line 465: __ cmp(tmp2, 0);
More compact encoding would be: test(tmp2, tmp2) followed by j(not_zero,
...). Alternatively you could get rid of the tmp2 altogether by doing a
__ cmp(ContextOperand(...), Immediate(0))...

http://codereview.chromium.org/20419/diff/17/25#newcode474
Line 474: __ cmp(tmp2, 0);
Ditto.

http://codereview.chromium.org/20419/diff/17/25#newcode2418
Line 2418: // Check that all extension objects from the current context
to
Why can't you use a variant of the ContextSlotOperandCheckExtensions
code that only checks extension objects for intermediate scopes that
call eval? It would be nice to be able to unroll the loop (I guess the
nesting isn't very deep usually) and only traverse the chain to the
outermost scope that calls eval instead of going all the way to the
global context.

http://codereview.chromium.org/20419/diff/17/25#newcode2423
Line 2423: __ mov(ecx, FieldOperand(ebx, HeapObject::kMapOffset));
This can be done in one cmp instruction.

http://codereview.chromium.org/20419/diff/17/25#newcode2428
Line 2428: __ cmp(edx, 0);
Use test(edx, edx) or one instruction cmp?

http://codereview.chromium.org/20419

--~--~---------~--~----~------------~-------~--~----~
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
-~----------~----~----~----~------~----~------~--~---

Reply via email to