Add a genuine unary minus instruction to Crankshaft....

Add a genuine unary minus instruction to Crankshaft.

This change introduces an instruction for negation instead
of generating a multiplication with -1.

The code for x64 and ARM is not included in this change.



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

[Kevin Millikin (Chromium), 2011-02-11 12:12:59]

LGTM, with comments below.

http://codereview.chromium.org/6461021/diff/1/src/hydrogen-instructions.h
File src/hydrogen-instructions.h (right):

http://codereview.chromium.org/6461021/diff/1/src/hydrogen-instructions.h#new...
src/hydrogen-instructions.h:1368: if (!to.IsTagged()) {
This function always makes me nervous, because the developer has to understand
that it's not OK to change from an untagged representation back to tagged.

Maybe we should handle the else case here, or assert that if to.IsTagged() then
from.IsTagged().

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

http://codereview.chromium.org/6461021/diff/1/src/ia32/lithium-codegen-ia32.c...
src/ia32/lithium-codegen-ia32.cc:1027: __ test(eax, Operand(eax));
eax ==> input?

http://codereview.chromium.org/6461021/diff/1/src/ia32/lithium-codegen-ia32.c...
src/ia32/lithium-codegen-ia32.cc:1042: __ psllq(xmm0, 32);
Gesundheit.

http://codereview.chromium.org/6461021/diff/1/src/ia32/lithium-ia32.h
File src/ia32/lithium-ia32.h (right):

http://codereview.chromium.org/6461021/diff/1/src/ia32/lithium-ia32.h#newcode...
src/ia32/lithium-ia32.h:1059: explicit LNegI(LOperand* value) {
I've been trying to create simple accessors for the inputs (e.g., LOperand*
value() { return inputs_[0]; }).

It's not so bad for unary operations, but for the others I don't want the
programmer to have to remember the position of the arguments.

It also makes it easier to add arguments or reorder them in the LInstruction
without having to guard against explicit use of indices.

[fschneider, 2011-02-11 12:44:12]

http://codereview.chromium.org/6461021/diff/1/src/hydrogen-instructions.h
File src/hydrogen-instructions.h (right):

http://codereview.chromium.org/6461021/diff/1/src/hydrogen-instructions.h#new...
src/hydrogen-instructions.h:1368: if (!to.IsTagged()) {
On 2011/02/11 12:12:59, Kevin Millikin wrote:
> This function always makes me nervous, because the developer has to understand
> that it's not OK to change from an untagged representation back to tagged.
> 
> Maybe we should handle the else case here, or assert that if to.IsTagged()
then
> from.IsTagged().

Done.

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

http://codereview.chromium.org/6461021/diff/1/src/ia32/lithium-codegen-ia32.c...
src/ia32/lithium-codegen-ia32.cc:1027: __ test(eax, Operand(eax));
On 2011/02/11 12:12:59, Kevin Millikin wrote:
> eax ==> input?

Thanks, of course.

http://codereview.chromium.org/6461021/diff/1/src/ia32/lithium-ia32.h
File src/ia32/lithium-ia32.h (right):

http://codereview.chromium.org/6461021/diff/1/src/ia32/lithium-ia32.h#newcode...
src/ia32/lithium-ia32.h:1059: explicit LNegI(LOperand* value) {
On 2011/02/11 12:12:59, Kevin Millikin wrote:
> I've been trying to create simple accessors for the inputs (e.g., LOperand*
> value() { return inputs_[0]; }).
> 
> It's not so bad for unary operations, but for the others I don't want the
> programmer to have to remember the position of the arguments.
> 
> It also makes it easier to add arguments or reorder them in the LInstruction
> without having to guard against explicit use of indices.

Done.

[Vitaly Repeshko, 2011-02-11 17:17:56]

Drive by.

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

http://codereview.chromium.org/6461021/diff/1/src/ia32/lithium-codegen-ia32.c...
src/ia32/lithium-codegen-ia32.cc:1042: __ psllq(xmm0, 32);
On 2011/02/11 12:12:59, Kevin Millikin wrote:
> Gesundheit.

It might be faster to load the minus zero constant instead (we already have it:
ExternalReference::address_of_minus_zero) because it doesn't need an extra
general purpose register.