Fix three bugs with handling negative zero in the optimizing compiler.
* Bug fix for range analysis (contributed by Andy Wingo). Ranges of double values have to include negative zero. Original code review: http://codereview.chromium.org/7514040/ * Fix a bug in optimized Math.round on ARM. When emitting minus-zero checks we previously return a wrong result because of incorrect register assignment. * Fix performance problem in IA32 and x64. Refine the checks for minus zero and avoid unnecessary deoptimizations on Math.floor. * Improve mjsunit test for Math.round to make sure we also get the optimized version of the code for each test case. Review URL: http://codereview.chromium.org/7604028 git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@8877 ce2b1a6d-e550-0410-aec6-3dcde31c8c00
This commit is contained in:
parent
bd18514972
commit
f17bd8ca51
@ -3014,19 +3014,18 @@ void LCodeGen::DoMathFloor(LUnaryMathOperation* instr) {
|
||||
void LCodeGen::DoMathRound(LUnaryMathOperation* instr) {
|
||||
DoubleRegister input = ToDoubleRegister(instr->InputAt(0));
|
||||
Register result = ToRegister(instr->result());
|
||||
Register scratch1 = result;
|
||||
Register scratch2 = scratch0();
|
||||
Register scratch = scratch0();
|
||||
Label done, check_sign_on_zero;
|
||||
|
||||
// Extract exponent bits.
|
||||
__ vmov(scratch1, input.high());
|
||||
__ ubfx(scratch2,
|
||||
scratch1,
|
||||
__ vmov(result, input.high());
|
||||
__ ubfx(scratch,
|
||||
result,
|
||||
HeapNumber::kExponentShift,
|
||||
HeapNumber::kExponentBits);
|
||||
|
||||
// If the number is in ]-0.5, +0.5[, the result is +/- 0.
|
||||
__ cmp(scratch2, Operand(HeapNumber::kExponentBias - 2));
|
||||
__ cmp(scratch, Operand(HeapNumber::kExponentBias - 2));
|
||||
__ mov(result, Operand(0), LeaveCC, le);
|
||||
if (instr->hydrogen()->CheckFlag(HValue::kBailoutOnMinusZero)) {
|
||||
__ b(le, &check_sign_on_zero);
|
||||
@ -3036,19 +3035,19 @@ void LCodeGen::DoMathRound(LUnaryMathOperation* instr) {
|
||||
|
||||
// The following conversion will not work with numbers
|
||||
// outside of ]-2^32, 2^32[.
|
||||
__ cmp(scratch2, Operand(HeapNumber::kExponentBias + 32));
|
||||
__ cmp(scratch, Operand(HeapNumber::kExponentBias + 32));
|
||||
DeoptimizeIf(ge, instr->environment());
|
||||
|
||||
// Save the original sign for later comparison.
|
||||
__ and_(scratch2, scratch1, Operand(HeapNumber::kSignMask));
|
||||
__ and_(scratch, result, Operand(HeapNumber::kSignMask));
|
||||
|
||||
__ Vmov(double_scratch0(), 0.5);
|
||||
__ vadd(input, input, double_scratch0());
|
||||
|
||||
// Check sign of the result: if the sign changed, the input
|
||||
// value was in ]0.5, 0[ and the result should be -0.
|
||||
__ vmov(scratch1, input.high());
|
||||
__ eor(scratch1, scratch1, Operand(scratch2), SetCC);
|
||||
__ vmov(result, input.high());
|
||||
__ eor(result, result, Operand(scratch), SetCC);
|
||||
if (instr->hydrogen()->CheckFlag(HValue::kBailoutOnMinusZero)) {
|
||||
DeoptimizeIf(mi, instr->environment());
|
||||
} else {
|
||||
@ -3059,8 +3058,8 @@ void LCodeGen::DoMathRound(LUnaryMathOperation* instr) {
|
||||
__ EmitVFPTruncate(kRoundToMinusInf,
|
||||
double_scratch0().low(),
|
||||
input,
|
||||
scratch1,
|
||||
scratch2);
|
||||
result,
|
||||
scratch);
|
||||
DeoptimizeIf(ne, instr->environment());
|
||||
__ vmov(result, double_scratch0().low());
|
||||
|
||||
@ -3069,8 +3068,8 @@ void LCodeGen::DoMathRound(LUnaryMathOperation* instr) {
|
||||
__ cmp(result, Operand(0));
|
||||
__ b(ne, &done);
|
||||
__ bind(&check_sign_on_zero);
|
||||
__ vmov(scratch1, input.high());
|
||||
__ tst(scratch1, Operand(HeapNumber::kSignMask));
|
||||
__ vmov(scratch, input.high());
|
||||
__ tst(scratch, Operand(HeapNumber::kSignMask));
|
||||
DeoptimizeIf(ne, instr->environment());
|
||||
}
|
||||
__ bind(&done);
|
||||
|
@ -862,19 +862,10 @@ void HInstanceOf::PrintDataTo(StringStream* stream) {
|
||||
|
||||
|
||||
Range* HValue::InferRange() {
|
||||
if (representation().IsTagged()) {
|
||||
// Tagged values are always in int32 range when converted to integer,
|
||||
// but they can contain -0.
|
||||
Range* result = new Range();
|
||||
result->set_can_be_minus_zero(true);
|
||||
return result;
|
||||
} else if (representation().IsNone()) {
|
||||
return NULL;
|
||||
} else {
|
||||
// Untagged integer32 cannot be -0 and we don't compute ranges for
|
||||
// untagged doubles.
|
||||
return new Range();
|
||||
}
|
||||
// Untagged integer32 cannot be -0, all other representations can.
|
||||
Range* result = new Range();
|
||||
result->set_can_be_minus_zero(!representation().IsInteger32());
|
||||
return result;
|
||||
}
|
||||
|
||||
|
||||
|
@ -2756,13 +2756,23 @@ void LCodeGen::DoMathFloor(LUnaryMathOperation* instr) {
|
||||
XMMRegister xmm_scratch = xmm0;
|
||||
Register output_reg = ToRegister(instr->result());
|
||||
XMMRegister input_reg = ToDoubleRegister(instr->value());
|
||||
Label done;
|
||||
|
||||
// Deoptimize on negative numbers.
|
||||
__ xorps(xmm_scratch, xmm_scratch); // Zero the register.
|
||||
__ ucomisd(input_reg, xmm_scratch);
|
||||
DeoptimizeIf(below, instr->environment());
|
||||
|
||||
if (instr->hydrogen()->CheckFlag(HValue::kBailoutOnMinusZero)) {
|
||||
DeoptimizeIf(below_equal, instr->environment());
|
||||
} else {
|
||||
DeoptimizeIf(below, instr->environment());
|
||||
// Check for negative zero.
|
||||
Label positive_sign;
|
||||
__ j(above, &positive_sign, Label::kNear);
|
||||
__ movmskpd(output_reg, input_reg);
|
||||
__ test(output_reg, Immediate(1));
|
||||
DeoptimizeIf(not_zero, instr->environment());
|
||||
__ Set(output_reg, Immediate(0));
|
||||
__ jmp(&done, Label::kNear);
|
||||
__ bind(&positive_sign);
|
||||
}
|
||||
|
||||
// Use truncating instruction (OK because input is positive).
|
||||
@ -2771,6 +2781,7 @@ void LCodeGen::DoMathFloor(LUnaryMathOperation* instr) {
|
||||
// Overflow is signalled with minint.
|
||||
__ cmp(output_reg, 0x80000000u);
|
||||
DeoptimizeIf(equal, instr->environment());
|
||||
__ bind(&done);
|
||||
}
|
||||
|
||||
|
||||
|
@ -2773,13 +2773,19 @@ void LCodeGen::DoMathFloor(LUnaryMathOperation* instr) {
|
||||
__ cmpl(output_reg, Immediate(0x80000000));
|
||||
DeoptimizeIf(equal, instr->environment());
|
||||
} else {
|
||||
// Deoptimize on negative inputs.
|
||||
__ xorps(xmm_scratch, xmm_scratch); // Zero the register.
|
||||
__ ucomisd(input_reg, xmm_scratch);
|
||||
|
||||
DeoptimizeIf(below, instr->environment());
|
||||
if (instr->hydrogen()->CheckFlag(HValue::kBailoutOnMinusZero)) {
|
||||
DeoptimizeIf(below_equal, instr->environment());
|
||||
} else {
|
||||
DeoptimizeIf(below, instr->environment());
|
||||
// Check for negative zero.
|
||||
__ j(above, &positive_sign, Label::kNear);
|
||||
__ movmskpd(output_reg, input_reg);
|
||||
__ testq(output_reg, Immediate(1));
|
||||
DeoptimizeIf(not_zero, instr->environment());
|
||||
__ Set(output_reg, Immediate(0));
|
||||
__ jmp(&done);
|
||||
__ bind(&positive_sign);
|
||||
}
|
||||
|
||||
// Use truncating instruction (OK because input is positive).
|
||||
@ -2789,6 +2795,7 @@ void LCodeGen::DoMathFloor(LUnaryMathOperation* instr) {
|
||||
__ cmpl(output_reg, Immediate(0x80000000));
|
||||
DeoptimizeIf(equal, instr->environment());
|
||||
}
|
||||
__ bind(&done);
|
||||
}
|
||||
|
||||
|
||||
|
@ -51,6 +51,17 @@ function test() {
|
||||
testFloor(-Infinity, -Infinity);
|
||||
testFloor(NaN, NaN);
|
||||
|
||||
// Ensure that a negative zero coming from Math.floor is properly handled
|
||||
// by other operations.
|
||||
function ifloor(x) {
|
||||
return 1 / Math.floor(x);
|
||||
}
|
||||
assertEquals(-Infinity, ifloor(-0));
|
||||
assertEquals(-Infinity, ifloor(-0));
|
||||
assertEquals(-Infinity, ifloor(-0));
|
||||
%OptimizeFunctionOnNextCall(ifloor);
|
||||
assertEquals(-Infinity, ifloor(-0));
|
||||
|
||||
testFloor(0, 0.1);
|
||||
testFloor(0, 0.49999999999999994);
|
||||
testFloor(0, 0.5);
|
||||
@ -129,3 +140,19 @@ function test() {
|
||||
for (var i = 0; i < 500; i++) {
|
||||
test();
|
||||
}
|
||||
|
||||
|
||||
// Regression test for a bug where a negative zero coming from Math.floor
|
||||
// was not properly handled by other operations.
|
||||
function floorsum(i, n) {
|
||||
var ret = Math.floor(n);
|
||||
while (--i > 0) {
|
||||
ret += Math.floor(n);
|
||||
}
|
||||
return ret;
|
||||
}
|
||||
assertEquals(-0, floorsum(1, -0));
|
||||
%OptimizeFunctionOnNextCall(floorsum);
|
||||
// The optimized function will deopt. Run it with enough iterations to try
|
||||
// to optimize via OSR (triggering the bug).
|
||||
assertEquals(-0, floorsum(100000, -0));
|
||||
|
@ -1,4 +1,4 @@
|
||||
// Copyright 2010 the V8 project authors. All rights reserved.
|
||||
// Copyright 2011 the V8 project authors. All rights reserved.
|
||||
// Redistribution and use in source and binary forms, with or without
|
||||
// modification, are permitted provided that the following conditions are
|
||||
// met:
|
||||
@ -27,10 +27,12 @@
|
||||
|
||||
// Flags: --allow-natives-syntax
|
||||
|
||||
var test_id = 0;
|
||||
function testRound(expect, input) {
|
||||
function doRound(input) {
|
||||
return Math.round(input);
|
||||
}
|
||||
// Make source code different on each invocation to make
|
||||
// sure it gets optimized each time.
|
||||
var doRound = new Function('input',
|
||||
'"' + (test_id++) + '";return Math.round(input)');
|
||||
assertEquals(expect, doRound(input));
|
||||
assertEquals(expect, doRound(input));
|
||||
assertEquals(expect, doRound(input));
|
||||
@ -44,6 +46,21 @@ testRound(Infinity, Infinity);
|
||||
testRound(-Infinity, -Infinity);
|
||||
testRound(NaN, NaN);
|
||||
|
||||
// Regression test for a bug where a negative zero coming from Math.round
|
||||
// was not properly handled by other operations.
|
||||
function roundsum(i, n) {
|
||||
var ret = Math.round(n);
|
||||
while (--i > 0) {
|
||||
ret += Math.round(n);
|
||||
}
|
||||
return ret;
|
||||
}
|
||||
assertEquals(-0, roundsum(1, -0));
|
||||
%OptimizeFunctionOnNextCall(roundsum);
|
||||
// The optimized function will deopt. Run it with enough iterations to try
|
||||
// to optimize via OSR (triggering the bug).
|
||||
assertEquals(-0, roundsum(100000, -0));
|
||||
|
||||
testRound(1, 0.5);
|
||||
testRound(1, 0.7);
|
||||
testRound(1, 1);
|
||||
|
Loading…
Reference in New Issue
Block a user