Some improvements in register usage in lithium compilation of LoadKeyed/StoreKeyed operations.

Some improvements in register usage in lithium compilation of LoadKeyed/StoreKeyed operations.

BUG=

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

[mvstanton, 2012-11-05 14:42:37]

Hi Danno, here are the changes we discussed Friday. I've found an additional
optimization in that for arm external loadkeyed we can use a RegAtStart in some
cases instead of a Reg.

[danno, 2012-11-07 22:34:38]

LGTM with comments

http://codereview.chromium.org/11365084/diff/9001/src/arm/lithium-arm.cc
File src/arm/lithium-arm.cc (right):

http://codereview.chromium.org/11365084/diff/9001/src/arm/lithium-arm.cc#newc...
src/arm/lithium-arm.cc:1884: external_pointer =
UseRegisterAtStart(instr->elements());
I don't see the difference in these two case in the codegen file, and in
practice since there are no temp registers, it probably doesn't make a
difference (I am not 100% sure whether UseRegister overlaps with the return
register. If it really doesn't make a difference, UseRegister all the time
probably will simplify the code.

http://codereview.chromium.org/11365084/diff/9001/src/x64/lithium-codegen-x64.cc
File src/x64/lithium-codegen-x64.cc (right):

http://codereview.chromium.org/11365084/diff/9001/src/x64/lithium-codegen-x64...
src/x64/lithium-codegen-x64.cc:2616: inline void
LCodeGen::HandleKeyedAlterations(T* hydrogen_instr, LOperand* key) {
How about "PrepareKeyForKeyedOp" or "PrepareKeyForKeyedOperation"?

http://codereview.chromium.org/11365084/diff/9001/src/x64/lithium-x64.cc
File src/x64/lithium-x64.cc (right):

http://codereview.chromium.org/11365084/diff/9001/src/x64/lithium-x64.cc#newc...
src/x64/lithium-x64.cc:1847: if (instr->is_external()) {
Wrap this in #if DEBUG like you did elsewhere?

http://codereview.chromium.org/11365084/diff/9001/src/x64/lithium-x64.h
File src/x64/lithium-x64.h (right):

http://codereview.chromium.org/11365084/diff/9001/src/x64/lithium-x64.h#newco...
src/x64/lithium-x64.h:1375: inline static bool ArrayOpClobbersKey(T *value) {
You can move this to the .cc file. It's only used internally.

[mvstanton, 2012-11-09 09:43:13]

thanks Danno for the good advice. I wasn't able to move the ArrayOpClobbersKey
though, see my comment below.

https://codereview.chromium.org/11365084/diff/9001/src/arm/lithium-arm.cc
File src/arm/lithium-arm.cc (right):

https://codereview.chromium.org/11365084/diff/9001/src/arm/lithium-arm.cc#new...
src/arm/lithium-arm.cc:1884: external_pointer =
UseRegisterAtStart(instr->elements());
On 2012/11/07 22:34:38, danno wrote:
> I don't see the difference in these two case in the codegen file, and in
> practice since there are no temp registers, it probably doesn't make a
> difference (I am not 100% sure whether UseRegister overlaps with the return
> register. If it really doesn't make a difference, UseRegister all the time
> probably will simplify the code.

Done.

https://codereview.chromium.org/11365084/diff/9001/src/x64/lithium-codegen-x6...
File src/x64/lithium-codegen-x64.cc (right):

https://codereview.chromium.org/11365084/diff/9001/src/x64/lithium-codegen-x6...
src/x64/lithium-codegen-x64.cc:2616: inline void
LCodeGen::HandleKeyedAlterations(T* hydrogen_instr, LOperand* key) {
On 2012/11/07 22:34:38, danno wrote:
> How about "PrepareKeyForKeyedOp" or "PrepareKeyForKeyedOperation"?

Done.

https://codereview.chromium.org/11365084/diff/9001/src/x64/lithium-x64.cc
File src/x64/lithium-x64.cc (right):

https://codereview.chromium.org/11365084/diff/9001/src/x64/lithium-x64.cc#new...
src/x64/lithium-x64.cc:1847: if (instr->is_external()) {
On 2012/11/07 22:34:38, danno wrote:
> Wrap this in #if DEBUG like you did elsewhere?

Done.

https://codereview.chromium.org/11365084/diff/9001/src/x64/lithium-x64.h
File src/x64/lithium-x64.h (right):

https://codereview.chromium.org/11365084/diff/9001/src/x64/lithium-x64.h#newc...
src/x64/lithium-x64.h:1375: inline static bool ArrayOpClobbersKey(T *value) {
On 2012/11/07 22:34:38, danno wrote:
> You can move this to the .cc file. It's only used internally.

Shucks, it is actually used in both lithium-x64.cc and lithium-codegen-x64.cc so
I would rather keep it here. I thought about moving it "deeper" into codegen by
placing it in lithium-codegen-x64.h, but since it operates on Hydrogen
instructions that didn't feel right either.

[danno, 2012-11-09 10:06:43]

Still LGTM