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.cc#newcode628 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.cc#newcode683 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#newcode1271 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 http://codereview.chromium.org/6448001/ -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
