Optimize loads from variables that might be shadowed by variables...

Optimize loads from variables that might be shadowed by variables
introduced by eval.

In the cases where calls to eval have not introduced any variables, we
do not need to perform a runtime call.  Instead, we verify that the
context extension objects have not been created and perform a direct
load.

Not implemented for ARM yet and the scope resolution code could use
some better abstractions.  I'd like to do that in a separate
changelist.

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

[Kasper Lund, 2009-02-17 13:39:02]

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?

[Mads Ager (chromium), 2009-02-18 09:39:28]

On 2009/02/17 13:39:02, Kasper Lund wrote:
> LGTM. Do we have good test coverage of this stuff already?

Not really.  I have added a couple of tests.

> 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))...

Done.

> 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.

Great idea!  Done.

This also revealed a bug where we would treat unresolved variables in local
scopes inside an eval scope as globals.  We cannot do that since we do not have
information about the surrounding scopes in eval scopes.  Therefore, there could
be with statements in outer scopes that we do no know about.  Fixed this issue
by propagating information about eval scopes to inner scopes.

Could you have a look again?

[Mads Ager (chromium), 2009-02-18 10:16:18]

OK, hang on.  No need to review this yet.  We need a "slow" fast-case for the
eval scopes that checks all extension objects.

[Mads Ager (chromium), 2009-02-18 12:54:20]

Turns out I do not have enough scope information (yet) to do the "slow"
fast-case for eval scopes.  I have converged on the current patch set for now. 
Could you have a look?