Support for calls on named and keyed properties of the form:...

Support for calls on named and keyed properties in the fast compiler of the form:

o.x() and o[expr]()

other changes:
- Fix missing relocation info for StoreIC on global object.
- Generate only one common return sequence instead of always appending 
  "return <undefined>" at the end of each function: The first JS 
  return-statement will generate the common return sequence. All
  other return-statements will generate a unconditional branch to the common
  return sequence.



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

[Kevin Millikin (Chromium), 2009-10-29 19:43:13]

Comments.  Code generator comments apply to all three platforms.

http://codereview.chromium.org/340037/diff/1/10
File src/arm/fast-codegen-arm.cc (right):

http://codereview.chromium.org/340037/diff/1/10#newcode104
Line 104: if (return_label_.is_bound()) {
This code is duplicated, so it should be abstracted into
"GenerateReturnSequence" or "EmitReturnSequence".  (I think Bill used "Emit"
earlier for short-circuited boolean operations.)

You can do that as a separate change right after you put this one in.

http://codereview.chromium.org/340037/diff/1/10#newcode571
Line 571: void FastCodeGenerator::DoCallWithIC(Call* expr) {
EmitCallWithIC?

http://codereview.chromium.org/340037/diff/1/10#newcode584
Line 584: __ Call(ic, RelocInfo::CODE_TARGET_CONTEXT);
I think the reloc info here has to depend on whether the receiver is the global
object or not.

http://codereview.chromium.org/340037/diff/1/10#newcode621
Line 621: // Use IC
Use call IC.

http://codereview.chromium.org/340037/diff/1/10#newcode624
Line 624: Visit(prop->obj());
Comment that you will handle keyed method calls with a keyed load followed by
function call.

http://codereview.chromium.org/340037/diff/1/10#newcode631
Line 631: __ add(sp, sp, Operand(kPointerSize));
__ pop() on ARM.  But I think we can be clever about stack manipulation starting
here:

// Receiver into r1.
if (prop->is_synthetic()) {
  __ ldr(r1, CodeGenerator::GlobalObject());
} else {
  __ ldr(r1, MemOperand(sp, kPointerSize));
}
// Overwrite (object,key) with (function,receiver).
__ str(r0, MemOperand(sp, kPointerSize));
__ str(r1, MemOperand(sp));

http://codereview.chromium.org/340037/diff/1/10#newcode650
Line 650: __ mov(r1, Operand(var->name()));
Indentation is off.

[fschneider, 2009-10-30 08:39:00]

Uploaded new patch.
Added a mjsunit test.

http://codereview.chromium.org/340037/diff/1/10
File src/arm/fast-codegen-arm.cc (right):

http://codereview.chromium.org/340037/diff/1/10#newcode104
Line 104: if (return_label_.is_bound()) {
On 2009/10/29 19:43:13, Kevin Millikin wrote:
> This code is duplicated, so it should be abstracted into
> "GenerateReturnSequence" or "EmitReturnSequence".  (I think Bill used "Emit"
> earlier for short-circuited boolean operations.)
> 
> You can do that as a separate change right after you put this one in.

Yes. Good point.

http://codereview.chromium.org/340037/diff/1/10#newcode571
Line 571: void FastCodeGenerator::DoCallWithIC(Call* expr) {
On 2009/10/29 19:43:13, Kevin Millikin wrote:
> EmitCallWithIC?

Done.

http://codereview.chromium.org/340037/diff/1/10#newcode584
Line 584: __ Call(ic, RelocInfo::CODE_TARGET_CONTEXT);
On 2009/10/29 19:43:13, Kevin Millikin wrote:
> I think the reloc info here has to depend on whether the receiver is the
global
> object or not.

Good point. Done.

http://codereview.chromium.org/340037/diff/1/10#newcode621
Line 621: // Use IC
On 2009/10/29 19:43:13, Kevin Millikin wrote:
> Use call IC.

Done.

http://codereview.chromium.org/340037/diff/1/10#newcode624
Line 624: Visit(prop->obj());
On 2009/10/29 19:43:13, Kevin Millikin wrote:
> Comment that you will handle keyed method calls with a keyed load followed by
> function call.

Done.

http://codereview.chromium.org/340037/diff/1/10#newcode631
Line 631: __ add(sp, sp, Operand(kPointerSize));
On 2009/10/29 19:43:13, Kevin Millikin wrote:
> __ pop() on ARM.  But I think we can be clever about stack manipulation
starting
> here:
> 
> // Receiver into r1.
> if (prop->is_synthetic()) {
>   __ ldr(r1, CodeGenerator::GlobalObject());
> } else {
>   __ ldr(r1, MemOperand(sp, kPointerSize));
> }
> // Overwrite (object,key) with (function,receiver).
> __ str(r0, MemOperand(sp, kPointerSize));
> __ str(r1, MemOperand(sp));

Agreed. I left IA-32 and x64 unchanged since push/pop reg are more compact than
mov [esp], reg.

http://codereview.chromium.org/340037/diff/1/10#newcode650
Line 650: __ mov(r1, Operand(var->name()));
On 2009/10/29 19:43:13, Kevin Millikin wrote:
> Indentation is off.

Done.

[Kevin Millikin (Chromium), 2009-10-30 09:25:59]

LGTM.

http://codereview.chromium.org/340037/diff/3001/4002
File src/ia32/fast-codegen-ia32.cc (right):

http://codereview.chromium.org/340037/diff/3001/4002#newcode565
Line 565: void FastCodeGenerator::EmitCallWithIC(Call* expr, bool is_global) {
It is probably nicer to pass the RelocInfo::Mode here.  Booleans are hard to
understant at the call site.

[fschneider, 2009-10-30 10:11:41]

http://codereview.chromium.org/340037/diff/3001/4002
File src/ia32/fast-codegen-ia32.cc (right):

http://codereview.chromium.org/340037/diff/3001/4002#newcode565
Line 565: void FastCodeGenerator::EmitCallWithIC(Call* expr, bool is_global) {
On 2009/10/30 09:25:59, Kevin Millikin wrote:
> It is probably nicer to pass the RelocInfo::Mode here.  Booleans are hard to
> understant at the call site.

Done. (also on other platforms)