Change StoreIC interface on ARM platform

Change StoreIC interface on ARM platform

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

[Kevin Millikin (Chromium), 2010-02-01 17:55:57]

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

http://codereview.chromium.org/554152/diff/1/2#newcode2734
src/arm/codegen-arm.cc:2734: __ ldr(r9, frame_->Top());
I prefer r1.  It's the next one after r0 (like edx is next after eax) and it's
the one 'traditionally' used for left subexpressions of binary operations.

http://codereview.chromium.org/554152/diff/1/2#newcode2735
src/arm/codegen-arm.cc:2735: frame_->PrepareForCall(0, 0);
Please don't call this explicitly.

http://codereview.chromium.org/554152/diff/1/2#newcode2737
src/arm/codegen-arm.cc:2737: __ ldr(r0, frame_->Top());
Doesn't this overwrite the value in r0 with the receiver?

http://codereview.chromium.org/554152/diff/1/2#newcode2738
src/arm/codegen-arm.cc:2738: __ Call(ic, RelocInfo::CodeTarget);
frame_->CallCodeObject(ic, RelocInfo::CODE_TARGET, 0);

http://codereview.chromium.org/554152/diff/1/2#newcode4404
src/arm/codegen-arm.cc:4404: Result value(r0);
Since we don't use the register allocator on ARM (and don't have immediate plans
to), clean this up.  No need to allocate registers:

frame->EmitPop(r0);
frame->EmitPop(r1);
__ mov(r2, Operand(name));
frame->CallCodeObject(ic, RelocInfo::CODE_TARGET, 0);
frame->EmitPush(r0);
set_unloaded();

http://codereview.chromium.org/554152/diff/1/5
File src/arm/debug-arm.cc (right):

http://codereview.chromium.org/554152/diff/1/5#newcode143
src/arm/debug-arm.cc:143: //  -- [sp]  : receiver
Comment needs updating.

http://codereview.chromium.org/554152/diff/1/4
File src/arm/virtual-frame-arm.cc (right):

http://codereview.chromium.org/554152/diff/1/4#newcode306
src/arm/virtual-frame-arm.cc:306: UNREACHABLE();
default is UNREACHABLE.  Make LoadIC go through the no-result-passed version of
CallCodeObject (in fact, you should slowly make all of them go that way and
remove the versions that take results pointers as arguments).

[William Hesse, 2010-02-10 13:11:20]

All requested changes made.  CallCodeObject versions taking Result arguments
were already removed in a separate change.  StoreStubCompiler methods are now
changed in this CL, in stub-cache-arm.cc, to agree with the StoreIC interface
change.

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

http://codereview.chromium.org/554152/diff/1/2#newcode2734
src/arm/codegen-arm.cc:2734: __ ldr(r9, frame_->Top());
On 2010/02/01 17:55:58, Kevin Millikin wrote:
> I prefer r1.  It's the next one after r0 (like edx is next after eax) and it's
> the one 'traditionally' used for left subexpressions of binary operations.

Done.

http://codereview.chromium.org/554152/diff/1/2#newcode2735
src/arm/codegen-arm.cc:2735: frame_->PrepareForCall(0, 0);
On 2010/02/01 17:55:58, Kevin Millikin wrote:
> Please don't call this explicitly.

Done.

http://codereview.chromium.org/554152/diff/1/2#newcode2737
src/arm/codegen-arm.cc:2737: __ ldr(r0, frame_->Top());
On 2010/02/01 17:55:58, Kevin Millikin wrote:
> Doesn't this overwrite the value in r0 with the receiver?

Done.

http://codereview.chromium.org/554152/diff/1/2#newcode2738
src/arm/codegen-arm.cc:2738: __ Call(ic, RelocInfo::CodeTarget);
On 2010/02/01 17:55:58, Kevin Millikin wrote:
> frame_->CallCodeObject(ic, RelocInfo::CODE_TARGET, 0);

Done.

http://codereview.chromium.org/554152/diff/1/2#newcode4404
src/arm/codegen-arm.cc:4404: Result value(r0);
On 2010/02/01 17:55:58, Kevin Millikin wrote:
> Since we don't use the register allocator on ARM (and don't have immediate
plans
> to), clean this up.  No need to allocate registers:
> 
> frame->EmitPop(r0);
> frame->EmitPop(r1);
> __ mov(r2, Operand(name));
> frame->CallCodeObject(ic, RelocInfo::CODE_TARGET, 0);
> frame->EmitPush(r0);
> set_unloaded();

Done.

http://codereview.chromium.org/554152/diff/1/5
File src/arm/debug-arm.cc (right):

http://codereview.chromium.org/554152/diff/1/5#newcode143
src/arm/debug-arm.cc:143: //  -- [sp]  : receiver
On 2010/02/01 17:55:58, Kevin Millikin wrote:
> Comment needs updating.

Done.

http://codereview.chromium.org/554152/diff/1/4
File src/arm/virtual-frame-arm.cc (right):

http://codereview.chromium.org/554152/diff/1/4#newcode306
src/arm/virtual-frame-arm.cc:306: UNREACHABLE();
All of the CallCodeObject functions replaced by the version that takes no Result
pointers as arguments.


On 2010/02/01 17:55:58, Kevin Millikin wrote:
> default is UNREACHABLE.  Make LoadIC go through the no-result-passed version
of
> CallCodeObject (in fact, you should slowly make all of them go that way and
> remove the versions that take results pointers as arguments).

[Kevin Millikin (Chromium), 2010-02-10 14:07:10]

LGTM

http://codereview.chromium.org/554152/diff/5002/5004
File src/arm/full-codegen-arm.cc (right):

http://codereview.chromium.org/554152/diff/5002/5004#newcode953
src/arm/full-codegen-arm.cc:953: Apply(context, r0);
Every path on this function ends with Apply(context, r0) so you could lift out
out of the branches.

http://codereview.chromium.org/554152/diff/5002/5007
File src/arm/stub-cache-arm.cc (right):

http://codereview.chromium.org/554152/diff/5002/5007#newcode1259
src/arm/stub-cache-arm.cc:1259: __ push(ip);
This sequence of three pushes can be replaced with stm if you wish.