ARM implementations of LoadElements, LoadKeyedFastElement, StoreNamedField, S...

ARM implementations of LoadElements, LoadKeyedFastElement, StoreNamedField, StoreKeyedFastElement.

BUG=none
TEST=none

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

[Søren Thygesen Gjesse, 2011-01-06 10:38:11]

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

http://codereview.chromium.org/5967008/diff/1/src/arm/lithium-codegen-arm.cc#...
src/arm/lithium-codegen-arm.cc:1591: Label done;
r9 -> scratch0()

http://codereview.chromium.org/5967008/diff/1/src/arm/lithium-codegen-arm.cc#...
src/arm/lithium-codegen-arm.cc:1610: void
LCodeGen::DoLoadKeyedFastElement(LLoadKeyedFastElement* instr) {
r9 -> scratch0(), move

Register scratch = scratch0()

to the beginning of the function.

http://codereview.chromium.org/5967008/diff/1/src/arm/lithium-codegen-arm.cc#...
src/arm/lithium-codegen-arm.cc:1830: Register scratch = r9;
r9 -> scratch0()

http://codereview.chromium.org/5967008/diff/1/src/arm/lithium-codegen-arm.cc#...
src/arm/lithium-codegen-arm.cc:1833: ASSERT(!object.is(value) &&
!scratch.is(object) && !scratch.is(value));
Remove the scratch check part from the assert.

http://codereview.chromium.org/5967008/diff/1/src/arm/lithium-codegen-arm.cc#...
src/arm/lithium-codegen-arm.cc:1843: if (instr->needs_write_barrier()) {
I don't think we need a temp register on ARM, as we have the scratch register.
You should remove the temp register from the ARM LStoreNamedField instruction.

http://codereview.chromium.org/5967008/diff/1/src/arm/lithium-codegen-arm.cc#...
src/arm/lithium-codegen-arm.cc:1881: Register key = instr->key()->IsRegister() ?
ToRegister(instr->key()) : no_reg;
r9 -> scratch0()

http://codereview.chromium.org/5967008/diff/1/src/arm/lithium-codegen-arm.cc#...
src/arm/lithium-codegen-arm.cc:1896: // Update the write barrier unless we're
certain that we're storing a smi.
Please change comment to something like

// Update the write barrier if required.

Or maybe just remove the comment altogether. Same for the ia32 version.

[Alexandre, 2011-01-06 14:54:44]

Thanks for the review.
The patch should be uploaded soon.

Alexandre

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

http://codereview.chromium.org/5967008/diff/1/src/arm/lithium-codegen-arm.cc#...
src/arm/lithium-codegen-arm.cc:1591: Label done;
On 2011/01/06 10:38:11, Søren Gjesse wrote:
> r9 -> scratch0()

Done.

http://codereview.chromium.org/5967008/diff/1/src/arm/lithium-codegen-arm.cc#...
src/arm/lithium-codegen-arm.cc:1610: void
LCodeGen::DoLoadKeyedFastElement(LLoadKeyedFastElement* instr) {
On 2011/01/06 10:38:11, Søren Gjesse wrote:
> r9 -> scratch0(), move
> 
> Register scratch = scratch0()
> 
> to the beginning of the function.

Done.

http://codereview.chromium.org/5967008/diff/1/src/arm/lithium-codegen-arm.cc#...
src/arm/lithium-codegen-arm.cc:1830: Register scratch = r9;
On 2011/01/06 10:38:11, Søren Gjesse wrote:
> r9 -> scratch0()

Done.

http://codereview.chromium.org/5967008/diff/1/src/arm/lithium-codegen-arm.cc#...
src/arm/lithium-codegen-arm.cc:1833: ASSERT(!object.is(value) &&
!scratch.is(object) && !scratch.is(value));
On 2011/01/06 10:38:11, Søren Gjesse wrote:
> Remove the scratch check part from the assert.

Done.

http://codereview.chromium.org/5967008/diff/1/src/arm/lithium-codegen-arm.cc#...
src/arm/lithium-codegen-arm.cc:1843: if (instr->needs_write_barrier()) {
Removed.
On 2011/01/06 10:38:11, Søren Gjesse wrote:
> I don't think we need a temp register on ARM, as we have the scratch register.
> You should remove the temp register from the ARM LStoreNamedField instruction.

http://codereview.chromium.org/5967008/diff/1/src/arm/lithium-codegen-arm.cc#...
src/arm/lithium-codegen-arm.cc:1881: Register key = instr->key()->IsRegister() ?
ToRegister(instr->key()) : no_reg;
On 2011/01/06 10:38:11, Søren Gjesse wrote:
> r9 -> scratch0()

Done.

http://codereview.chromium.org/5967008/diff/1/src/arm/lithium-codegen-arm.cc#...
src/arm/lithium-codegen-arm.cc:1896: // Update the write barrier unless we're
certain that we're storing a smi.
Removed the comment. The function's name speaks for itself.
On 2011/01/06 10:38:11, Søren Gjesse wrote:
> Please change comment to something like
> 
> // Update the write barrier if required.
> 
> Or maybe just remove the comment altogether. Same for the ia32 version.

[Søren Thygesen Gjesse, 2011-01-07 07:40:08]

LGTM

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

Thank you for the patch.