HLoadKeyed for Smis optimized for x64

HLoadKeyed for Smis optimized for x64

R=verwaest@chromium.org

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

[Toon Verwaest, 2013-12-09 13:47:23]

Added comments.

https://codereview.chromium.org/108933002/diff/1/src/x64/lithium-codegen-x64.cc
File src/x64/lithium-codegen-x64.cc (right):

https://codereview.chromium.org/108933002/diff/1/src/x64/lithium-codegen-x64....
src/x64/lithium-codegen-x64.cc:3074: int offs = FixedArray::kHeaderSize -
kHeapObjectTag;
Please write out variable names: "offset".

https://codereview.chromium.org/108933002/diff/1/src/x64/lithium-codegen-x64....
src/x64/lithium-codegen-x64.cc:3077: if (hinstr->type().IsSmi() &&
representation.IsInteger32()) {
Why check for hinstr->type().IsSmi()? You should only get to
representation.IsInteger32() if you converted a smi-load into an int32 load.

https://codereview.chromium.org/108933002/diff/1/src/x64/lithium-codegen-x64....
src/x64/lithium-codegen-x64.cc:3080: offs += kPointerSize / 2;
Isn't there a kSmiTagSize you can use? You just want to jump over the smi-tag.

[Igor Sheludko, 2013-12-09 17:27:06]

https://codereview.chromium.org/108933002/diff/1/src/x64/lithium-codegen-x64.cc
File src/x64/lithium-codegen-x64.cc (right):

https://codereview.chromium.org/108933002/diff/1/src/x64/lithium-codegen-x64....
src/x64/lithium-codegen-x64.cc:3074: int offs = FixedArray::kHeaderSize -
kHeapObjectTag;
On 2013/12/09 13:47:23, Toon Verwaest wrote:
> Please write out variable names: "offset".

Done.

https://codereview.chromium.org/108933002/diff/1/src/x64/lithium-codegen-x64....
src/x64/lithium-codegen-x64.cc:3077: if (hinstr->type().IsSmi() &&
representation.IsInteger32()) {
On 2013/12/09 13:47:23, Toon Verwaest wrote:
> Why check for hinstr->type().IsSmi()? You should only get to
> representation.IsInteger32() if you converted a smi-load into an int32 load.

We can't rely only on IsInteger32() case since we can get an int32
representation for external arrays. We dicussed this and decided that it is
better to check elements' kind against FAST_SMI_ELEMENTS.

https://codereview.chromium.org/108933002/diff/1/src/x64/lithium-codegen-x64....
src/x64/lithium-codegen-x64.cc:3080: offs += kPointerSize / 2;
On 2013/12/09 13:47:23, Toon Verwaest wrote:
> Isn't there a kSmiTagSize you can use? You just want to jump over the smi-tag.

kSmiTagSize is a number of bits, so we can't use it here, but I'll add an assert
instead:
STATIC_ASSERT(kSmiTagSize + kSmiShiftSize == 32);

[Toon Verwaest, 2013-12-09 17:51:19]

lgtm

[Igor Sheludko, 2013-12-18 17:41:18]

Committed patchset #2 manually as r18359 (presubmit successful).