Bias commutative single-use register inputs and support lea adds

Bias commutative single-use register inputs and support lea adds

This improves register allocation for many common add and multiply patterns on ia32 and x64 by reducing register pressure.

R=jkummerow@chromium.org

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

[Jakob Kummerow, 2013-05-03 09:01:37]

LGTM with comments.

What about double representation? I think the same heuristics should apply in
that case too. Happy to take another look if you decide to extend this CL;
otherwise we can do that in a separate change.

https://codereview.chromium.org/14856015/diff/1/src/ia32/lithium-ia32.cc
File src/ia32/lithium-ia32.cc (right):

https://codereview.chromium.org/14856015/diff/1/src/ia32/lithium-ia32.cc#newc...
src/ia32/lithium-ia32.cc:1622: // Check to see if it would be advantageous to
use
nit: use what?

[danno, 2013-05-07 11:29:05]

Please take another look

[Sven Panne, 2013-05-07 11:38:40]

Quick DBCs...

https://codereview.chromium.org/14856015/diff/16001/src/hydrogen-instructions.h
File src/hydrogen-instructions.h (right):

https://codereview.chromium.org/14856015/diff/16001/src/hydrogen-instructions...
src/hydrogen-instructions.h:3362: if (IsCommutative()) {
Starting with "if (!IsCommutative()) return false;" is more readable IMHO, it
follows a "comb-like" pattern. Perhaps one could fold this into a single boolean
expression, but I am not sure if this is more readable or not.

https://codereview.chromium.org/14856015/diff/16001/src/ia32/lithium-ia32.cc
File src/ia32/lithium-ia32.cc (right):

https://codereview.chromium.org/14856015/diff/16001/src/ia32/lithium-ia32.cc#...
src/ia32/lithium-ia32.cc:1621: LInstruction* result = NULL;
Ternary aversion? Use ?:

[Jakob Kummerow, 2013-05-07 11:53:58]

LGTM with Sven's comments addressed.

All ia32 comments apply to x64 too.

https://codereview.chromium.org/14856015/diff/16001/src/ia32/lithium-ia32.cc
File src/ia32/lithium-ia32.cc (right):

https://codereview.chromium.org/14856015/diff/16001/src/ia32/lithium-ia32.cc#...
src/ia32/lithium-ia32.cc:1608: // than an add. This is the case when no overflow
check is needed and are
nit: s/are/there are/

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

https://codereview.chromium.org/14856015/diff/16001/src/x64/lithium-x64.cc#ne...
src/x64/lithium-x64.cc:1539: if (instr->CheckFlag(HValue::kCanOverflow)) {
This condition is now a dupe of the above.

[danno, 2013-05-08 08:37:18]

Feedback addressed

https://codereview.chromium.org/14856015/diff/16001/src/hydrogen-instructions.h
File src/hydrogen-instructions.h (right):

https://codereview.chromium.org/14856015/diff/16001/src/hydrogen-instructions...
src/hydrogen-instructions.h:3362: if (IsCommutative()) {
On 2013/05/07 11:38:40, Sven Panne wrote:
> Starting with "if (!IsCommutative()) return false;" is more readable IMHO, it
> follows a "comb-like" pattern. Perhaps one could fold this into a single
boolean
> expression, but I am not sure if this is more readable or not.

Done.

https://codereview.chromium.org/14856015/diff/16001/src/ia32/lithium-ia32.cc
File src/ia32/lithium-ia32.cc (right):

https://codereview.chromium.org/14856015/diff/16001/src/ia32/lithium-ia32.cc#...
src/ia32/lithium-ia32.cc:1608: // than an add. This is the case when no overflow
check is needed and are
On 2013/05/07 11:53:58, Jakob wrote:
> nit: s/are/there are/

Done.

https://codereview.chromium.org/14856015/diff/16001/src/ia32/lithium-ia32.cc#...
src/ia32/lithium-ia32.cc:1621: LInstruction* result = NULL;
On 2013/05/07 11:38:40, Sven Panne wrote:
> Ternary aversion? Use ?:

Done.

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

https://codereview.chromium.org/14856015/diff/16001/src/x64/lithium-x64.cc#ne...
src/x64/lithium-x64.cc:1539: if (instr->CheckFlag(HValue::kCanOverflow)) {
On 2013/05/07 11:53:58, Jakob wrote:
> This condition is now a dupe of the above.

Done.

[danno, 2013-05-08 08:37:35]

Committed patchset #7 manually as r14587 (presubmit successful).