LGTM. Tag it. On Wed, Feb 2, 2011 at 5:05 PM, <[email protected]> wrote:
> Reviewers: Kevin Millikin, > > Description: > Merge r6573 to 3.0 branch. This is a fix for issue 1088, Math.pow(-0, > 0.5). > > Please review this at http://codereview.chromium.org/6334056/ > > SVN Base: http://v8.googlecode.com/svn/branches/3.0/ > > Affected files: > M src/arm/codegen-arm.cc > M src/assembler.cc > M src/ia32/code-stubs-ia32.cc > M src/ia32/codegen-ia32.cc > M src/ia32/lithium-codegen-ia32.cc > M src/x64/codegen-x64.cc > M test/mjsunit/math-pow.js > > > Index: src/arm/codegen-arm.cc > =================================================================== > --- src/arm/codegen-arm.cc (revision 6585) > +++ src/arm/codegen-arm.cc (working copy) > @@ -4698,12 +4698,15 @@ > runtime.entry_label(), > AVOID_NANS_AND_INFINITIES); > > + // Convert -0 into +0 by adding +0. > + __ vmov(d2, 0.0); > + __ vadd(d0, d2, d0); > // Load 1.0 into d2. > __ vmov(d2, 1.0); > > - // Calculate the reciprocal of the square root. 1/sqrt(x) = sqrt(1/x). > + // Calculate the reciprocal of the square root. > + __ vsqrt(d0, d0); > __ vdiv(d0, d2, d0); > - __ vsqrt(d0, d0); > > __ b(&allocate_return); > > @@ -4717,6 +4720,9 @@ > scratch1, scratch2, heap_number_map, s0, > runtime.entry_label(), > AVOID_NANS_AND_INFINITIES); > + // Convert -0 into +0 by adding +0. > + __ vmov(d2, 0.0); > + __ vadd(d0, d2, d0); > __ vsqrt(d0, d0); > > __ bind(&allocate_return); > Index: src/assembler.cc > =================================================================== > --- src/assembler.cc (revision 6585) > +++ src/assembler.cc (working copy) > @@ -838,8 +838,8 @@ > return power_double_int(x, y_int); // Returns 1.0 for exponent 0. > } > if (!isinf(x)) { > - if (y == 0.5) return sqrt(x); > - if (y == -0.5) return 1.0 / sqrt(x); > + if (y == 0.5) return sqrt(x + 0.0); // -0 must be converted to +0. > + if (y == -0.5) return 1.0 / sqrt(x + 0.0); > } > if (isnan(y) || ((x == 1 || x == -1) && isinf(y))) { > return OS::nan_value(); > Index: src/ia32/code-stubs-ia32.cc > =================================================================== > --- src/ia32/code-stubs-ia32.cc (revision 6585) > +++ src/ia32/code-stubs-ia32.cc (working copy) > @@ -3477,10 +3477,12 @@ > __ j(not_equal, ¬_minus_half); > > // Calculates reciprocal of square root. > - // Note that 1/sqrt(x) = sqrt(1/x)) > - __ divsd(xmm3, xmm0); > + // sqrtsd returns -0 when input is -0. ECMA spec requires +0. > + __ xorpd(xmm1, xmm1); > + __ addsd(xmm1, xmm0); > + __ sqrtsd(xmm1, xmm1); > + __ divsd(xmm3, xmm1); > __ movsd(xmm1, xmm3); > - __ sqrtsd(xmm1, xmm1); > __ jmp(&allocate_return); > > // Test for 0.5. > @@ -3492,7 +3494,9 @@ > __ ucomisd(xmm2, xmm1); > __ j(not_equal, &call_runtime); > // Calculates square root. > - __ movsd(xmm1, xmm0); > + // sqrtsd returns -0 when input is -0. ECMA spec requires +0. > + __ xorpd(xmm1, xmm1); > + __ addsd(xmm1, xmm0); > __ sqrtsd(xmm1, xmm1); > > __ bind(&allocate_return); > Index: src/ia32/codegen-ia32.cc > =================================================================== > --- src/ia32/codegen-ia32.cc (revision 6585) > +++ src/ia32/codegen-ia32.cc (working copy) > @@ -7951,10 +7951,12 @@ > __ j(not_equal, ¬_minus_half); > > // Calculates reciprocal of square root. > - // Note that 1/sqrt(x) = sqrt(1/x)) > - __ divsd(xmm3, xmm0); > + // sqrtsd returns -0 when input is -0. ECMA spec requires +0. > + __ xorpd(xmm1, xmm1); > + __ addsd(xmm1, xmm0); > + __ sqrtsd(xmm1, xmm1); > + __ divsd(xmm3, xmm1); > __ movsd(xmm1, xmm3); > - __ sqrtsd(xmm1, xmm1); > __ jmp(&allocate_return); > > // Test for 0.5. > @@ -7966,7 +7968,9 @@ > __ ucomisd(xmm2, xmm1); > call_runtime.Branch(not_equal); > // Calculates square root. > - __ movsd(xmm1, xmm0); > + // sqrtsd returns -0 when input is -0. ECMA spec requires +0. > + __ xorpd(xmm1, xmm1); > + __ addsd(xmm1, xmm0); > __ sqrtsd(xmm1, xmm1); > > JumpTarget done; > Index: src/ia32/lithium-codegen-ia32.cc > =================================================================== > --- src/ia32/lithium-codegen-ia32.cc (revision 6585) > +++ src/ia32/lithium-codegen-ia32.cc (working copy) > @@ -2388,6 +2388,8 @@ > __ movdbl(xmm_scratch, Operand::StaticVariable(negative_infinity)); > __ ucomisd(xmm_scratch, input_reg); > DeoptimizeIf(equal, instr->environment()); > + __ xorpd(xmm_scratch, xmm_scratch); > + __ addsd(input_reg, xmm_scratch); // Convert -0 to +0. > __ sqrtsd(input_reg, input_reg); > } > > Index: src/x64/codegen-x64.cc > =================================================================== > --- src/x64/codegen-x64.cc (revision 6585) > +++ src/x64/codegen-x64.cc (working copy) > @@ -6969,10 +6969,12 @@ > __ j(not_equal, ¬_minus_half); > > // Calculates reciprocal of square root. > - // Note that 1/sqrt(x) = sqrt(1/x)) > - __ divsd(xmm3, xmm0); > + // sqrtsd returns -0 when input is -0. ECMA spec requires +0. > + __ xorpd(xmm1, xmm1); > + __ addsd(xmm1, xmm0); > + __ sqrtsd(xmm1, xmm1); > + __ divsd(xmm3, xmm1); > __ movsd(xmm1, xmm3); > - __ sqrtsd(xmm1, xmm1); > __ jmp(&allocate_return); > > // Test for 0.5. > @@ -6985,7 +6987,9 @@ > call_runtime.Branch(not_equal); > > // Calculates square root. > - __ movsd(xmm1, xmm0); > + // sqrtsd returns -0 when input is -0. ECMA spec requires +0. > + __ xorpd(xmm1, xmm1); > + __ addsd(xmm1, xmm0); > __ sqrtsd(xmm1, xmm1); > > JumpTarget done; > Index: test/mjsunit/math-pow.js > =================================================================== > --- test/mjsunit/math-pow.js (revision 6585) > +++ test/mjsunit/math-pow.js (working copy) > @@ -58,10 +58,11 @@ > assertEquals(Infinity, Math.pow(2, Infinity)); > assertEquals(Infinity, Math.pow(-2, Infinity)); > > -assertEquals(+0, Math.pow(1.1, -Infinity)); > -assertEquals(+0, Math.pow(-1.1, -Infinity)); > -assertEquals(+0, Math.pow(2, -Infinity)); > -assertEquals(+0, Math.pow(-2, -Infinity)); > +// Because +0 == -0, we need to compare 1/{+,-}0 to {+,-}Infinity > +assertEquals(+Infinity, 1/Math.pow(1.1, -Infinity)); > +assertEquals(+Infinity, 1/Math.pow(-1.1, -Infinity)); > +assertEquals(+Infinity, 1/Math.pow(2, -Infinity)); > +assertEquals(+Infinity, 1/Math.pow(-2, -Infinity)); > > assertEquals(NaN, Math.pow(1, Infinity)); > assertEquals(NaN, Math.pow(1, -Infinity)); > @@ -81,8 +82,8 @@ > assertEquals(Infinity, Math.pow(Infinity, 0.1)); > assertEquals(Infinity, Math.pow(Infinity, 2)); > > -assertEquals(+0, Math.pow(Infinity, -0.1)); > -assertEquals(+0, Math.pow(Infinity, -2)); > +assertEquals(+Infinity, 1/Math.pow(Infinity, -0.1)); > +assertEquals(+Infinity, 1/Math.pow(Infinity, -2)); > > assertEquals(-Infinity, Math.pow(-Infinity, 3)); > assertEquals(-Infinity, Math.pow(-Infinity, 13)); > @@ -90,23 +91,23 @@ > assertEquals(Infinity, Math.pow(-Infinity, 3.1)); > assertEquals(Infinity, Math.pow(-Infinity, 2)); > > -assertEquals(-0, Math.pow(-Infinity, -3)); > -assertEquals(-0, Math.pow(-Infinity, -13)); > +assertEquals(-Infinity, 1/Math.pow(-Infinity, -3)); > +assertEquals(-Infinity, 1/Math.pow(-Infinity, -13)); > > -assertEquals(+0, Math.pow(-Infinity, -3.1)); > -assertEquals(+0, Math.pow(-Infinity, -2)); > +assertEquals(+Infinity, 1/Math.pow(-Infinity, -3.1)); > +assertEquals(+Infinity, 1/Math.pow(-Infinity, -2)); > > -assertEquals(+0, Math.pow(+0, 1.1)); > -assertEquals(+0, Math.pow(+0, 2)); > +assertEquals(+Infinity, 1/Math.pow(+0, 1.1)); > +assertEquals(+Infinity, 1/Math.pow(+0, 2)); > > assertEquals(Infinity, Math.pow(+0, -1.1)); > assertEquals(Infinity, Math.pow(+0, -2)); > > -assertEquals(-0, Math.pow(-0, 3)); > -assertEquals(-0, Math.pow(-0, 13)); > +assertEquals(-Infinity, 1/Math.pow(-0, 3)); > +assertEquals(-Infinity, 1/Math.pow(-0, 13)); > > -assertEquals(+0, Math.pow(-0, 3.1)); > -assertEquals(+0, Math.pow(-0, 2)); > +assertEquals(+Infinity, 1/Math.pow(-0, 3.1)); > +assertEquals(+Infinity, 1/Math.pow(-0, 2)); > > assertEquals(-Infinity, Math.pow(-0, -3)); > assertEquals(-Infinity, Math.pow(-0, -13)); > @@ -123,6 +124,18 @@ > assertEquals(NaN, Math.pow(-1000, 1.1)); > assertEquals(NaN, Math.pow(-1000, -1.1)); > > +assertEquals(+Infinity, 1/Math.pow(-0, 0.5)); > +assertEquals(+Infinity, 1/Math.pow(-0, 0.6)); > +assertEquals(-Infinity, 1/Math.pow(-0, 1)); > +assertEquals(-Infinity, 1/Math.pow(-0, 10000000001)); > + > +assertEquals(+Infinity, Math.pow(-0, -0.5)); > +assertEquals(+Infinity, Math.pow(-0, -0.6)); > +assertEquals(-Infinity, Math.pow(-0, -1)); > +assertEquals(-Infinity, Math.pow(-0, -10000000001)); > + > + > + > // Tests from Sputnik S8.5_A13_T1. > assertTrue((1*((Math.pow(2,53))-1)*(Math.pow(2,-1074))) === > 4.4501477170144023e-308); > assertTrue((1*(Math.pow(2,52))*(Math.pow(2,-1074))) === > 2.2250738585072014e-308); > > > -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
