Adapt to new calling convention on ARM.

Adapt to new calling convention on ARM:
- Simplified frame entry and frame exit code.
- Added ArgumentsAdaptorTrampoline and check for matching argument counts in the InvokePrologue.
- Removed definition and uses of USE_OLD_CALLING_CONVENTIONS.
- Changed MacroAssembler::InvokeBuiltin to match ia32 version.
- Start introducing convenience instructions in the ARM assembler as needed. These instructions take all Register parameters to avoid extra typing of "Operand(reg)".


To keep the architectures in sync these changes have been made to the ia32 files:
- Changed MacroAssembler::EnterFrame(StackFrame::Type type) to MacroAssembler::EnterInternalFrame().


These parts are still missing:
- unimplemented: Builtins::Generate_FunctionApply - large limit
- unimplemented: Builtins::Generate_ArgumentsAdaptorTrampoline - non-function call
- The files have not been lint'd yet.



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

[iposva, 2008-09-11 06:56:11]

This is a first set of changes to adapt ARM to the new calling convention. There
are still 5 tests failing.

[Kasper Lund, 2008-09-11 10:43:23]

Overall this LGTM. It seems to be getting really close to working even in all
the weird corner cases.

http://codereview.chromium.org/1930/diff/1/13
File src/simulator-arm.cc (right):

http://codereview.chromium.org/1930/diff/1/13#newcode94
Line 94: OS::Abort();
Is this intentional?

[Feng Qian, 2008-09-11 21:33:12]

LGTM.

My comments are minor, looks like frames-ia32.cc and frames-arm.cc are very
close to each other, you can refactor them in another CL.

Looking forward to your submission.

http://codereview.chromium.org/1930/diff/1/9
File src/builtins-arm.cc (right):

http://codereview.chromium.org/1930/diff/1/9#newcode67
Line 67: __ push(r1);  // constructor function
The constructor function pushed on the stack seems not used. Double check!

http://codereview.chromium.org/1930/diff/1/9#newcode100
Line 100: // r3: number of arguments (smi-tagged)
How about adding one more line comment here:
// r2: caller sp

http://codereview.chromium.org/1930/diff/1/18
File src/codegen-arm.cc (right):

http://codereview.chromium.org/1930/diff/1/18#newcode3876
Line 3876: ASSERT(args->length() == 1);
Please update comment above function declaration.

http://codereview.chromium.org/1930/diff/1/8
File src/frames-arm.cc (right):

http://codereview.chromium.org/1930/diff/1/8#newcode48
Line 48: return static_cast<StackFrame::Type>(Smi::cast(marker)->value());
Now this function looks exactly like the on in frame-ia32.cc. Can we move it to
frames.cc?

http://codereview.chromium.org/1930/diff/1/8#newcode99
Line 99: return fp() + offset + (arguments * kPointerSize);
The same as the one in frames-ia32.cc

http://codereview.chromium.org/1930/diff/1/20
File src/macro-assembler-arm.cc (right):

http://codereview.chromium.org/1930/diff/1/20#newcode299
Line 299: 
This part is a bit complicated. I can see unnecessary mov instructions in
different paths. For example, when both actual and expected are immediate but
not equal, there are two instructions that move actual.immediate() into r0.

http://codereview.chromium.org/1930/diff/1/20#newcode309
Line 309: mov(r0, Operand(actual.immediate()));
This mov(r0, actual...) seems redundant.

http://codereview.chromium.org/1930/diff/1/20#newcode316
Line 316: mov(r0, Operand(actual.immediate()));
This mov(r0, actual...) seems redundant.

http://codereview.chromium.org/1930/diff/1/20#newcode665
Line 665: Builtins::JavaScript id, bool* resolved) {
Can this function be made that same as in IA32? (non-static).

Part of code looks identical as the same function in IA32. You may want to
refactor the common code out.

[iposva, 2008-09-12 00:18:14]

First set of replies...

http://codereview.chromium.org/1930/diff/1/9
File src/builtins-arm.cc (right):

http://codereview.chromium.org/1930/diff/1/9#newcode67
Line 67: __ push(r1);  // constructor function
On 2008/09/11 21:33:13, Feng Qian wrote:
> The constructor function pushed on the stack seems not used. Double check!

It is used for example on line 78:
__ ldr(r1, MemOperand(sp, kPointerSize));

http://codereview.chromium.org/1930/diff/1/9#newcode100
Line 100: // r3: number of arguments (smi-tagged)
On 2008/09/11 21:33:13, Feng Qian wrote:
> How about adding one more line comment here:
> // r2: caller sp
Fixed

http://codereview.chromium.org/1930/diff/1/18
File src/codegen-arm.cc (right):

http://codereview.chromium.org/1930/diff/1/18#newcode3876
Line 3876: ASSERT(args->length() == 1);
On 2008/09/11 21:33:13, Feng Qian wrote:
> Please update comment above function declaration.
Outdated comment removed.

http://codereview.chromium.org/1930/diff/1/8
File src/frames-arm.cc (right):

http://codereview.chromium.org/1930/diff/1/8#newcode48
Line 48: return static_cast<StackFrame::Type>(Smi::cast(marker)->value());
On 2008/09/11 21:33:13, Feng Qian wrote:
> Now this function looks exactly like the on in frame-ia32.cc. Can we move it
to
> frames.cc?

This change is already unwieldy. Some of the cleanup should move to a follow up
change.

http://codereview.chromium.org/1930/diff/1/20
File src/macro-assembler-arm.cc (right):

http://codereview.chromium.org/1930/diff/1/20#newcode299
Line 299: 
On 2008/09/11 21:33:13, Feng Qian wrote:
> This part is a bit complicated. I can see unnecessary mov instructions in
> different paths. For example, when both actual and expected are immediate but
> not equal, there are two instructions that move actual.immediate() into r0.
>  

Removed leftover code from the old calling convention. Thanks for spotting!

http://codereview.chromium.org/1930/diff/1/20#newcode665
Line 665: Builtins::JavaScript id, bool* resolved) {
On 2008/09/11 21:33:13, Feng Qian wrote:
> Can this function be made that same as in IA32? (non-static).
> 
> Part of code looks identical as the same function in IA32. You may want to
> refactor the common code out.

Factored the common code out into Builtins::GetCode(JavaScript id, bool*
resolved).

http://codereview.chromium.org/1930/diff/1/13
File src/simulator-arm.cc (right):

http://codereview.chromium.org/1930/diff/1/13#newcode94
Line 94: OS::Abort();
On 2008/09/11 10:43:23, Kasper Lund wrote:
> Is this intentional?

Yes, this was just needed so that test.py does not wait until the timeout
expires when hitting a stop instruction. Will be removed before putback.