x64: Add MulI and DivI to lithium instructions.

x64: Add MulI and DivI to lithium instructions.



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

[William Hesse, 2011-02-08 09:42:47]

LGTM, with comments.

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

http://codereview.chromium.org/6448001/diff/4002/src/x64/lithium-codegen-x64....
src/x64/lithium-codegen-x64.cc:628: NearLabel left_not_zero;
Should we combine the zero test and the Min-int test into
test(left_reg, Immediate(0x7FFFFFFF)
j(not_zero, &safe);
...
further tests

http://codereview.chromium.org/6448001/diff/4002/src/x64/lithium-codegen-x64....
src/x64/lithium-codegen-x64.cc:683: if
(ToInteger32(LConstantOperand::cast(right)) < 0) {
What about the case where right is constant 0?  And left is negative.

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

http://codereview.chromium.org/6448001/diff/4002/src/x64/lithium-x64.cc#newco...
src/x64/lithium-x64.cc:1271: LOperand* value = UseFixed(instr->left(), rax);
Dividend might be a more specific name than value.

[Rico, 2011-02-08 11:22:04]

Please take another look, added a regression test for the constant 0 right
operand with negative left value.

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

http://codereview.chromium.org/6448001/diff/4002/src/x64/lithium-codegen-x64....
src/x64/lithium-codegen-x64.cc:628: NearLabel left_not_zero;
On 2011/02/08 09:42:47, William Hesse wrote:
> Should we combine the zero test and the Min-int test into
> test(left_reg, Immediate(0x7FFFFFFF)
> j(not_zero, &safe);
> ...
> further tests

I guess combining these could cause us to deoptimize in cases where we would not
need to deoptimize normally? (e.g., if we should not bailout on minus zero, but
we can overflow)

http://codereview.chromium.org/6448001/diff/4002/src/x64/lithium-codegen-x64....
src/x64/lithium-codegen-x64.cc:683: if
(ToInteger32(LConstantOperand::cast(right)) < 0) {
On 2011/02/08 09:42:47, William Hesse wrote:
> What about the case where right is constant 0?  And left is negative.
Fixed - here and on ia32 and arm. Bug and regression test added

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

http://codereview.chromium.org/6448001/diff/4002/src/x64/lithium-x64.cc#newco...
src/x64/lithium-x64.cc:1271: LOperand* value = UseFixed(instr->left(), rax);
On 2011/02/08 09:42:47, William Hesse wrote:
> Dividend might be a more specific name than value.
Changed here and on ia32 and arm

[William Hesse, 2011-02-08 14:19:41]

LGTM.