Support for property assignment in the fast compiler....

Support for property assignment in the fast compiler.

The code for .result = (b.y = 99) where b is a global variable is:

  push [esi+0x17]
  mov ecx,0xf5c229ad          ;; object: 0xf5c229ad <String[1]: b>
  call LoadIC_Initialize
  nop
  mov [esp],eax
  mov eax,0xc6
  mov ecx,0xf5c25c41          ;; object: 0xf5c25c41 <String[1]: y>
  call StoreIC_Initialize
  nop
  mov [esp],eax
  pop [ebp+0xf4]

There is still some room for improvement in the generated code.

Other changes:
 - Replaced switch-statement in FastCodeGenerator::VisitProperty with DropAndMove(...)
 - Do not emit nop after IC calls on ARM.


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

[Kevin Millikin (Chromium), 2009-10-28 14:17:10]

http://codereview.chromium.org/347001/diff/1/3
File src/compiler.cc (right):

http://codereview.chromium.org/347001/diff/1/3#newcode671
Line 671: // global variables and and properties.
"We support plain non-compound assignments to properties, parameters and
non-context (stack-allocated) locals, and global variables."

Otherwise it sounds like they might be only global properties.

http://codereview.chromium.org/347001/diff/1/3#newcode685
Line 685: Visit(expr->target());
Hmmm.  Here it's probably OK to visit the property as if it were an rvalue, but
I think we should avoid it.  Imagine if the selector had to do something
different for lvalue and rvalue properties....

Better:

if (var == NULL) {
  Property* prop = expr->target()->AsProperty();
  if (prop == NULL) BAILOUT(...);
  Visit(prop->obj());
  Visit(prop->key());
} else ...

http://codereview.chromium.org/347001/diff/1/3#newcode688
Line 688: ASSERT(var->slot() != NULL);
It occurs to me that we could hit the var->slot() == NULL case.  (Due to a
left-hand side that is not a property but is a proxy that rewrites to a
property.)  We should fix that by adding a bailout for var->slot() == NULL.

http://codereview.chromium.org/347001/diff/1/2
File src/ia32/fast-codegen-ia32.cc (right):

http://codereview.chromium.org/347001/diff/1/2#newcode470
Line 470: __ add(Operand(esp), Immediate(kPointerSize));
Indentation.

http://codereview.chromium.org/347001/diff/1/4
File src/x64/fast-codegen-x64.cc (right):

http://codereview.chromium.org/347001/diff/1/4#newcode482
Line 482: __ addq(rsp, Immediate(kPointerSize));
Indentation.

[fschneider, 2009-10-28 17:43:30]

http://codereview.chromium.org/347001/diff/1/3
File src/compiler.cc (right):

http://codereview.chromium.org/347001/diff/1/3#newcode671
Line 671: // global variables and and properties.
On 2009/10/28 14:17:11, Kevin Millikin wrote:
> "We support plain non-compound assignments to properties, parameters and
> non-context (stack-allocated) locals, and global variables."
> 
> Otherwise it sounds like they might be only global properties.

Done.

http://codereview.chromium.org/347001/diff/1/3#newcode685
Line 685: Visit(expr->target());
On 2009/10/28 14:17:11, Kevin Millikin wrote:
> Hmmm.  Here it's probably OK to visit the property as if it were an rvalue,
but
> I think we should avoid it.  Imagine if the selector had to do something
> different for lvalue and rvalue properties....
> 
> Better:
> 
> if (var == NULL) {
>   Property* prop = expr->target()->AsProperty();
>   if (prop == NULL) BAILOUT(...);
>   Visit(prop->obj());
>   Visit(prop->key());
> } else ...

Done. Also I noticed that I don't need an else{} if we bailout on the then-part
:)

http://codereview.chromium.org/347001/diff/1/3#newcode688
Line 688: ASSERT(var->slot() != NULL);
On 2009/10/28 14:17:11, Kevin Millikin wrote:
> It occurs to me that we could hit the var->slot() == NULL case.  (Due to a
> left-hand side that is not a property but is a proxy that rewrites to a
> property.)  We should fix that by adding a bailout for var->slot() == NULL.

Done.

http://codereview.chromium.org/347001/diff/1/2
File src/ia32/fast-codegen-ia32.cc (right):

http://codereview.chromium.org/347001/diff/1/2#newcode470
Line 470: __ add(Operand(esp), Immediate(kPointerSize));
On 2009/10/28 14:17:11, Kevin Millikin wrote:
> Indentation.

Done.

http://codereview.chromium.org/347001/diff/1/4
File src/x64/fast-codegen-x64.cc (right):

http://codereview.chromium.org/347001/diff/1/4#newcode482
Line 482: __ addq(rsp, Immediate(kPointerSize));
On 2009/10/28 14:17:11, Kevin Millikin wrote:
> Indentation.

Done.

[fschneider, 2009-10-29 17:51:13]

New patch uploaded. Updates to merge with current release.

[Kevin Millikin (Chromium), 2009-10-29 19:06:24]

LGTM.