Handle global variables potentially shadowed by eval-introduced...

Handle global variables potentially shadowed by eval-introduced
variables in full-codegen.


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

[Kevin Millikin (Chromium), 2010-09-09 09:59:26]

http://codereview.chromium.org/3295022/diff/7001/8001
File src/arm/full-codegen-arm.cc (right):

http://codereview.chromium.org/3295022/diff/7001/8001#newcode888
src/arm/full-codegen-arm.cc:888: Register context = cp;
This code might be clearer with some renaming:

Register current = cp;
Register next = r1;
Register temp = r2;
...

http://codereview.chromium.org/3295022/diff/7001/8001#newcode906
src/arm/full-codegen-arm.cc:906: context = tmp;
// Walk the rest of the chain using a single register (without clobbering cp).
current = next;

http://codereview.chromium.org/3295022/diff/7001/8001#newcode916
src/arm/full-codegen-arm.cc:916: __ Move(tmp, context);
Don't you know statically whether you've walked up the context chain?  Could you
do

if (!tmp.is(context)) {
  __ Move(tmp, context);
}

[Kasper Lund, 2010-09-09 10:02:19]

LGTM. It should be very easy to add the typeof case.

http://codereview.chromium.org/3295022/diff/7001/8001
File src/arm/full-codegen-arm.cc (right):

http://codereview.chromium.org/3295022/diff/7001/8001#newcode897
src/arm/full-codegen-arm.cc:897: __ ldr(tmp2,
CodeGenerator::ContextOperand(context,
If the full codegen had a ContextOperand (maybe just wrapping the one in
CodeGenerator) it would look nicer.

[Mads Ager (chromium), 2010-09-09 10:43:41]

Thanks. I will enable the other cases separately.

http://codereview.chromium.org/3295022/diff/7001/8001
File src/arm/full-codegen-arm.cc (right):

http://codereview.chromium.org/3295022/diff/7001/8001#newcode888
src/arm/full-codegen-arm.cc:888: Register context = cp;
On 2010/09/09 09:59:26, Kevin Millikin wrote:
> This code might be clearer with some renaming:
> 
> Register current = cp;
> Register next = r1;
> Register temp = r2;
> ...
> 

Done.

http://codereview.chromium.org/3295022/diff/7001/8001#newcode897
src/arm/full-codegen-arm.cc:897: __ ldr(tmp2,
CodeGenerator::ContextOperand(context,
On 2010/09/09 10:02:19, Kasper Lund wrote:
> If the full codegen had a ContextOperand (maybe just wrapping the one in
> CodeGenerator) it would look nicer.

Done.

http://codereview.chromium.org/3295022/diff/7001/8001#newcode906
src/arm/full-codegen-arm.cc:906: context = tmp;
On 2010/09/09 09:59:26, Kevin Millikin wrote:
> // Walk the rest of the chain using a single register (without clobbering cp).
> current = next;

Done.

http://codereview.chromium.org/3295022/diff/7001/8001#newcode916
src/arm/full-codegen-arm.cc:916: __ Move(tmp, context);
On 2010/09/09 09:59:26, Kevin Millikin wrote:
> Don't you know statically whether you've walked up the context chain?  Could
you
> do
> 
> if (!tmp.is(context)) {
>   __ Move(tmp, context);
> }

Yes I do and I have exactly that conditional on the other platforms. Done.