X64: Fix error in division & modulus, adjust mjsunit test status, fix lint er...

X64: Fix error in division & modulus, adjust mjsunit test status, fix lint error in objects.h

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

[Lasse Reichstein, 2009-07-29 13:03:56]

LGTM

http://codereview.chromium.org/159584/diff/9/1004
File src/x64/codegen-x64.cc (right):

http://codereview.chromium.org/159584/diff/9/1004#newcode4968
Line 4968: if (!left_is_in_rax) __ movq(rax, left->reg());
Use movsxlq(rax, left->reg()) here and movsxlq(rax,rax) in the else branch.

http://codereview.chromium.org/159584/diff/9/1004#newcode4977
Line 4977: __ idiv(right->reg());
Could we use idivl instead, and avoid the sign extensions?

http://codereview.chromium.org/159584/diff/9/1004#newcode6611
Line 6611: __ idiv(rbx);
If you change anything above, do it here too.

[William Hesse, 2009-07-29 14:36:12]

Comments addressed by using idivl instead of idivq.  Please rereview.

[Lasse Reichstein, 2009-07-30 05:11:16]

LGTM

http://codereview.chromium.org/159584/diff/14/17
File src/x64/codegen-x64.cc (right):

http://codereview.chromium.org/159584/diff/14/17#newcode5009
Line 5009: // zero.  The frame is unchanged in this block, so local control
The frame doesn't seem unchanged by the entire block (there is a frame_->Push).
Could you rewrite the comment?

http://codereview.chromium.org/159584/diff/14/17#newcode5059
Line 5059: __ movq(answer.reg(), left->reg());
Use movl? It might save a REX prefix byte.

http://codereview.chromium.org/159584/diff/14/17#newcode6632
Line 6632: __ NegativeZeroTest(rdx, rcx, slow);  // use ecx = x | y
ecx => rcx in comment

[William Hesse, 2009-07-30 07:29:03]

http://codereview.chromium.org/159584/diff/14/17
File src/x64/codegen-x64.cc (right):

http://codereview.chromium.org/159584/diff/14/17#newcode5009
Line 5009: // zero.  The frame is unchanged in this block, so local control
On 2009/07/30 05:11:16, Lasse Reichstein wrote:
> The frame doesn't seem unchanged by the entire block (there is a
frame_->Push).
> Could you rewrite the comment?

Done.

http://codereview.chromium.org/159584/diff/14/17#newcode5059
Line 5059: __ movq(answer.reg(), left->reg());
On 2009/07/30 05:11:16, Lasse Reichstein wrote:
> Use movl? It might save a REX prefix byte.

Done.

http://codereview.chromium.org/159584/diff/14/17#newcode6632
Line 6632: __ NegativeZeroTest(rdx, rcx, slow);  // use ecx = x | y
On 2009/07/30 05:11:16, Lasse Reichstein wrote:
> ecx => rcx in comment

No, ecx is used.  Made explicit.

[Kevin Millikin (Chromium), 2009-07-30 07:52:05]

Drive by.

http://codereview.chromium.org/159584/diff/38/1017
File src/x64/codegen-x64.cc (right):

http://codereview.chromium.org/159584/diff/38/1017#newcode5009
Line 5009: // and the target, so a Label can be used.
To me this is less precise than what was before.

Try: "The frame is unchanged on all paths reaching non_zero_result, so..." or
"There is no frame effect on any path reaching non_zero_result, so...".

And write the same thing at both sites so the reader doesn't have to puzzle out
whether it's the same reason or a different one.