ARM64: Sign extension on MemOperand for keyed ops

ARM64: Sign extension on MemOperand for keyed ops

SXTW extend mode is usually cheaper on loads and stores than arithmetic,
so move it to the memory accesses where possible for Keyed loads and
stores.

BUG=
R=ulan@chromium.org

Committed: https://code.google.com/p/v8/source/detail?r=21172

[ulan, 2014-05-05 08:21:21]

lgtm with comments:

https://codereview.chromium.org/268483002/diff/1/src/arm64/lithium-codegen-ar...
File src/arm64/lithium-codegen-arm64.cc (left):

https://codereview.chromium.org/268483002/diff/1/src/arm64/lithium-codegen-ar...
src/arm64/lithium-codegen-arm64.cc:5178:
ASSERT(instr->hydrogen()->elements_kind() == FAST_SMI_ELEMENTS);
Can we keep these two asserts?

https://codereview.chromium.org/268483002/diff/1/src/arm64/lithium-codegen-ar...
File src/arm64/lithium-codegen-arm64.cc (right):

https://codereview.chromium.org/268483002/diff/1/src/arm64/lithium-codegen-ar...
src/arm64/lithium-codegen-arm64.cc:3442: bool int32_and_fast_smi =
representation.IsInteger32() &&
representation.IsInteger32() should imply (instr->hydrogen()->elements_kind() ==
FAST_SMI_ELEMENTS)

https://codereview.chromium.org/268483002/diff/1/src/arm64/lithium-codegen-ar...
src/arm64/lithium-codegen-arm64.cc:3523: if (representation.IsInteger32() &&
representation.IsInteger32() should imply (instr->hydrogen()->elements_kind() ==
FAST_SMI_ELEMENTS)

Could you instead of checking elements_kind add

ASSERT(instr->hydrogen()->elements_kind() == FAST_SMI_ELEMENTS);

in this branch?

https://codereview.chromium.org/268483002/diff/1/src/arm64/lithium-codegen-ar...
src/arm64/lithium-codegen-arm64.cc:5191: (instr->hydrogen()->elements_kind() ==
FAST_SMI_ELEMENTS)) {
representation.IsInteger32() should imply (instr->hydrogen()->elements_kind() ==
FAST_SMI_ELEMENTS)

https://codereview.chromium.org/268483002/diff/1/src/arm64/lithium-codegen-ar...
src/arm64/lithium-codegen-arm64.cc:5219: __ Add(element_addr, store_base,
mem_op.OffsetAsOperand());
Using mem_op.base() instead of store_base would be less fragile.

[m.m.capewell, 2014-05-06 13:20:45]

https://codereview.chromium.org/268483002/diff/1/src/arm64/lithium-codegen-ar...
File src/arm64/lithium-codegen-arm64.cc (left):

https://codereview.chromium.org/268483002/diff/1/src/arm64/lithium-codegen-ar...
src/arm64/lithium-codegen-arm64.cc:5178:
ASSERT(instr->hydrogen()->elements_kind() == FAST_SMI_ELEMENTS);
On 2014/05/05 08:21:21, ulan wrote:
> Can we keep these two asserts?

Done.

https://codereview.chromium.org/268483002/diff/1/src/arm64/lithium-codegen-ar...
File src/arm64/lithium-codegen-arm64.cc (right):

https://codereview.chromium.org/268483002/diff/1/src/arm64/lithium-codegen-ar...
src/arm64/lithium-codegen-arm64.cc:3442: bool int32_and_fast_smi =
representation.IsInteger32() &&
On 2014/05/05 08:21:21, ulan wrote:
> representation.IsInteger32() should imply (instr->hydrogen()->elements_kind()
==
> FAST_SMI_ELEMENTS)

Done.

https://codereview.chromium.org/268483002/diff/1/src/arm64/lithium-codegen-ar...
src/arm64/lithium-codegen-arm64.cc:3523: if (representation.IsInteger32() &&
On 2014/05/05 08:21:21, ulan wrote:
> representation.IsInteger32() should imply (instr->hydrogen()->elements_kind()
==
> FAST_SMI_ELEMENTS)
> 
> Could you instead of checking elements_kind add
> 
> ASSERT(instr->hydrogen()->elements_kind() == FAST_SMI_ELEMENTS);
> 
> in this branch?

Done.

https://codereview.chromium.org/268483002/diff/1/src/arm64/lithium-codegen-ar...
src/arm64/lithium-codegen-arm64.cc:5191: (instr->hydrogen()->elements_kind() ==
FAST_SMI_ELEMENTS)) {
On 2014/05/05 08:21:21, ulan wrote:
> representation.IsInteger32() should imply (instr->hydrogen()->elements_kind()
==
> FAST_SMI_ELEMENTS)

Done.

https://codereview.chromium.org/268483002/diff/1/src/arm64/lithium-codegen-ar...
src/arm64/lithium-codegen-arm64.cc:5219: __ Add(element_addr, store_base,
mem_op.OffsetAsOperand());
On 2014/05/05 08:21:21, ulan wrote:
> Using mem_op.base() instead of store_base would be less fragile.

Done.

[m.m.capewell, 2014-05-06 14:28:43]

Committed patchset #1 manually as r21172 (presubmit successful).