LGTM
http://codereview.chromium.org/6246099/diff/5/src/x64/lithium-codegen-x64.cc File src/x64/lithium-codegen-x64.cc (right): http://codereview.chromium.org/6246099/diff/5/src/x64/lithium-codegen-x64.cc#newcode679 src/x64/lithium-codegen-x64.cc:679: __ testl(ToRegister(left), Immediate(0x80000000)); Seems wasteful to compare to a 4-byte literal. Just do testl(ToRegister(left), ToRegister(left)); DeoptimizeIf(negative, ...) Maybe cache the result of ToRegister(left), instead of calling it multiple times. http://codereview.chromium.org/6246099/diff/5/src/x64/lithium-codegen-x64.cc#newcode701 src/x64/lithium-codegen-x64.cc:701: __ testl(ToRegister(left), Immediate(0x80000000)); Test to self and deopt if negative here too. http://codereview.chromium.org/6246099/diff/5/src/x64/lithium-x64.cc File src/x64/lithium-x64.cc (right): http://codereview.chromium.org/6246099/diff/5/src/x64/lithium-x64.cc#newcode762 src/x64/lithium-x64.cc:762: LOperand* left = UseFixed(instr->left(), rdx); Why fixed registers? Is it to match the calling convention of the binary op stub? http://codereview.chromium.org/6246099/diff/5/src/x64/lithium-x64.cc#newcode799 src/x64/lithium-x64.cc:799: // by 0 and the result cannot be truncated to int32. It's a little tricky to have a constant value that happens to be zero when the operand isn't constant. The "constant_value" variable should be meaningless in that case. How about a binary flag that says bool can_deopt = (op == Token::SHR); and inside if (right_value->IsConstant), set it to false if the constant is non-zero. http://codereview.chromium.org/6246099/ -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
