Refactor assignment in the ARM code generator...

Refactor assignment in the ARM code generator

This is mainly a port of r3899. It also adds handling of initilization blocks in ARM which had no special handling before.

The "calling conventions" used for

  EmitNamedLoad
  EmitNamedStore
  EmitKeyedLoad
  EmitKeyedStore

are somewhat mixed, but will become more aligned as the use of register allication and passing of argument in registers to IC's is extended.

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

[Erik Corry, 2010-05-03 19:16:50]

LGTM with comments

http://codereview.chromium.org/1846002/diff/17001/18001
File src/arm/codegen-arm.cc (right):

http://codereview.chromium.org/1846002/diff/17001/18001#newcode3332
src/arm/codegen-arm.cc:3332: ASSERT_EQ(NULL, var);
How do we know this assert is true?  I would like more comments in this function
explaining what the stack holds at each point.  Perhaps the flags
(is_trivial_receiver, starts_initialization_block, ends_initialization_block,
is_compound, var->isnull) can be distilled into a set of orthogonal variables
and the stack layout explained for each value of each variable.

http://codereview.chromium.org/1846002/diff/17001/18001#newcode3361
src/arm/codegen-arm.cc:3361: Load(node->value());
If this is a constant Smi can we use a SmiOperation instead?

http://codereview.chromium.org/1846002/diff/17001/18001#newcode3377
src/arm/codegen-arm.cc:3377: frame_->EmitPop(VirtualFrame::scratch0());
Seems a bit dodgy assuming that the scratch registers don't get overwritten by a
trivial receiver.  If you are satisfied that this is currently the case then
lets leave it in, but we need to start tracking off-stack registers soon.

http://codereview.chromium.org/1846002/diff/17001/18001#newcode3406
src/arm/codegen-arm.cc:3406: void
CodeGenerator::EmitKeyedPropertyAssignment(Assignment* node) {
All the same comments apply to this function as the function above.

http://codereview.chromium.org/1846002/diff/17001/18003
File src/arm/virtual-frame-arm.cc (right):

http://codereview.chromium.org/1846002/diff/17001/18003#newcode513
src/arm/virtual-frame-arm.cc:513: __ mov(r1, r0);
You forget to update the top_of_stack_state_ here and in the next case.

[Søren Thygesen Gjesse, 2010-05-04 09:31:35]

http://codereview.chromium.org/1846002/diff/17001/18001
File src/arm/codegen-arm.cc (right):

http://codereview.chromium.org/1846002/diff/17001/18001#newcode3332
src/arm/codegen-arm.cc:3332: ASSERT_EQ(NULL, var);
On 2010/05/03 19:16:50, Erik Corry wrote:
> How do we know this assert is true?  I would like more comments in this
function
> explaining what the stack holds at each point.  Perhaps the flags
> (is_trivial_receiver, starts_initialization_block, ends_initialization_block,
> is_compound, var->isnull) can be distilled into a set of orthogonal variables
> and the stack layout explained for each value of each variable.

Initialization block consists of assignments on the form expr.x = ..., so this
will never be an assignment to a variable.

I have tried to improve the comments and provide stack layout so that it is
easier to follow what is going on.

http://codereview.chromium.org/1846002/diff/17001/18001#newcode3361
src/arm/codegen-arm.cc:3361: Load(node->value());
On 2010/05/03 19:16:50, Erik Corry wrote:
> If this is a constant Smi can we use a SmiOperation instead?

Yes we can.

http://codereview.chromium.org/1846002/diff/17001/18001#newcode3377
src/arm/codegen-arm.cc:3377: frame_->EmitPop(VirtualFrame::scratch0());
On 2010/05/03 19:16:50, Erik Corry wrote:
> Seems a bit dodgy assuming that the scratch registers don't get overwritten by
a
> trivial receiver.  If you are satisfied that this is currently the case then
> lets leave it in, but we need to start tracking off-stack registers soon.

Changed to swap [tos] with [tos+1] after loading the receiver.

http://codereview.chromium.org/1846002/diff/17001/18001#newcode3406
src/arm/codegen-arm.cc:3406: void
CodeGenerator::EmitKeyedPropertyAssignment(Assignment* node) {
On 2010/05/03 19:16:50, Erik Corry wrote:
> All the same comments apply to this function as the function above.

Done.

http://codereview.chromium.org/1846002/diff/17001/18003
File src/arm/virtual-frame-arm.cc (right):

http://codereview.chromium.org/1846002/diff/17001/18003#newcode513
src/arm/virtual-frame-arm.cc:513: __ mov(r1, r0);
On 2010/05/03 19:16:50, Erik Corry wrote:
> You forget to update the top_of_stack_state_ here and in the next case.

Thats right, but there is no need. r0 and r1 have the duplicated value, so both
R0_R1_TOS and R1_R0_TOS are fine. Added a comment though.