Needs to be fixed, as described in comments.
Please include a message when uploading, so I get an email.


http://codereview.chromium.org/7489045/diff/16001/src/assembler.cc
File src/assembler.cc (right):

http://codereview.chromium.org/7489045/diff/16001/src/assembler.cc#newcode1115
src/assembler.cc:1115: ASSERT(IsCompareOp(op));
Needs to be Token::IsCompareOp(op).

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

http://codereview.chromium.org/7489045/diff/16001/src/x64/lithium-codegen-x64.cc#newcode1556
src/x64/lithium-codegen-x64.cc:1556: __ cmpl(ToOperand(left),
Immediate(value));
This case is not needed, because left is created with
"UseRegisterAtStart".
None of the new code is ever hit, because of how left and right are
created in lithium-x64.cc.
This needs to be changed - I would recommend
UseRegisterOrConstantAtStart for integer left
UseOrConstantAtStart for integer right

UseRegisterOrConstantAtStart for double left
UseRegisterOrConstantAtStart for double right.

Then the only one that could be a memory operand is right.

These changes should also be made to the ia32 platform.

http://codereview.chromium.org/7489045/diff/16001/src/x64/lithium-codegen-x64.cc#newcode1568
src/x64/lithium-codegen-x64.cc:1568: __ cmp(ToRegister(left),
ToOperand(right));
This must be a cmpl.

http://codereview.chromium.org/7489045/

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

Reply via email to