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.cc#newcode628
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.cc#newcode683
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#newcode1271
src/x64/lithium-x64.cc:1271: LOperand* value = UseFixed(instr->left(),
rax);
Dividend might be a more specific name than value.

http://codereview.chromium.org/6448001/

--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev

Reply via email to