From: fschneider@chromium.org Date: Wed, 7 Dec 2011 10:13:46 +0000 (+0000) Subject: Fix a bug with register use in optimized Math.round. X-Git-Tag: upstream/4.7.83~17756 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=c1662a199b6d23de4af4933e998b74b148c5c172;p=platform%2Fupstream%2Fv8.git Fix a bug with register use in optimized Math.round. We're not allowed to modify the input register and have to use a temporary instead, otherwise the result of expressions containing Math.round can be wrong. BUG=106351 TEST=test/mjsunit/compiler/regress-106351.js Review URL: http://codereview.chromium.org/8833007 git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@10190 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- diff --git a/src/arm/lithium-codegen-arm.cc b/src/arm/lithium-codegen-arm.cc index a7161aa..8b3c1a4 100644 --- a/src/arm/lithium-codegen-arm.cc +++ b/src/arm/lithium-codegen-arm.cc @@ -3059,11 +3059,11 @@ void LCodeGen::DoMathRound(LUnaryMathOperation* instr) { __ and_(scratch, result, Operand(HeapNumber::kSignMask)); __ Vmov(double_scratch0(), 0.5); - __ vadd(input, input, double_scratch0()); + __ vadd(double_scratch0(), 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(result, input.high()); + __ vmov(result, double_scratch0().high()); __ eor(result, result, Operand(scratch), SetCC); if (instr->hydrogen()->CheckFlag(HValue::kBailoutOnMinusZero)) { DeoptimizeIf(mi, instr->environment()); @@ -3074,7 +3074,7 @@ void LCodeGen::DoMathRound(LUnaryMathOperation* instr) { __ EmitVFPTruncate(kRoundToMinusInf, double_scratch0().low(), - input, + double_scratch0(), result, scratch); DeoptimizeIf(ne, instr->environment()); diff --git a/src/ia32/lithium-codegen-ia32.cc b/src/ia32/lithium-codegen-ia32.cc index 7c6ad77..d3ed88b 100644 --- a/src/ia32/lithium-codegen-ia32.cc +++ b/src/ia32/lithium-codegen-ia32.cc @@ -2900,12 +2900,12 @@ void LCodeGen::DoMathRound(LUnaryMathOperation* instr) { __ movdbl(xmm_scratch, Operand::StaticVariable(one_half)); __ ucomisd(xmm_scratch, input_reg); __ j(above, &below_half); - // input = input + 0.5 - __ addsd(input_reg, xmm_scratch); + // xmm_scratch = input + 0.5 + __ addsd(xmm_scratch, input_reg); // Compute Math.floor(value + 0.5). // Use truncating instruction (OK because input is positive). - __ cvttsd2si(output_reg, Operand(input_reg)); + __ cvttsd2si(output_reg, Operand(xmm_scratch)); // Overflow is signalled with minint. __ cmp(output_reg, 0x80000000u); diff --git a/src/mips/lithium-codegen-mips.cc b/src/mips/lithium-codegen-mips.cc index fc430ab..54c6dd6 100644 --- a/src/mips/lithium-codegen-mips.cc +++ b/src/mips/lithium-codegen-mips.cc @@ -2928,11 +2928,11 @@ void LCodeGen::DoMathRound(LUnaryMathOperation* instr) { __ And(scratch, result, Operand(HeapNumber::kSignMask)); __ Move(double_scratch0(), 0.5); - __ add_d(input, input, double_scratch0()); + __ add_d(double_scratch0(), 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. - __ mfc1(result, input.high()); + __ mfc1(result, double_scratch0().high()); __ Xor(result, result, Operand(scratch)); if (instr->hydrogen()->CheckFlag(HValue::kBailoutOnMinusZero)) { // ARM uses 'mi' here, which is 'lt' @@ -2952,7 +2952,7 @@ void LCodeGen::DoMathRound(LUnaryMathOperation* instr) { __ EmitFPUTruncate(kRoundToMinusInf, double_scratch0().low(), - input, + double_scratch0(), result, except_flag); diff --git a/src/x64/lithium-codegen-x64.cc b/src/x64/lithium-codegen-x64.cc index 07d0f67..2b31f22 100644 --- a/src/x64/lithium-codegen-x64.cc +++ b/src/x64/lithium-codegen-x64.cc @@ -2792,10 +2792,10 @@ void LCodeGen::DoMathRound(LUnaryMathOperation* instr) { // This addition might give a result that isn't the correct for // rounding, due to loss of precision, but only for a number that's // so big that the conversion below will overflow anyway. - __ addsd(input_reg, xmm_scratch); + __ addsd(xmm_scratch, input_reg); // Compute Math.floor(input). // Use truncating instruction (OK because input is positive). - __ cvttsd2si(output_reg, input_reg); + __ cvttsd2si(output_reg, xmm_scratch); // Overflow is signalled with minint. __ cmpl(output_reg, Immediate(0x80000000)); DeoptimizeIf(equal, instr->environment()); diff --git a/test/mjsunit/compiler/regress-106351.js b/test/mjsunit/compiler/regress-106351.js new file mode 100644 index 0000000..2a67a05 --- /dev/null +++ b/test/mjsunit/compiler/regress-106351.js @@ -0,0 +1,38 @@ +// 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: +// +// * Redistributions of source code must retain the above copyright +// notice, this list of conditions and the following disclaimer. +// * Redistributions in binary form must reproduce the above +// copyright notice, this list of conditions and the following +// disclaimer in the documentation and/or other materials provided +// with the distribution. +// * Neither the name of Google Inc. nor the names of its +// contributors may be used to endorse or promote products derived +// from this software without specific prior written permission. +// +// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS +// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT +// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR +// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT +// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT +// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, +// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY +// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT +// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + +// Flags: --allow-natives-syntax + +// Test Math.round with the input reused in the same expression. +function test(x) { + var v = Math.round(x) - x; + assertEquals(0.5, v); +} + +for (var i = 0; i < 5; ++i) test(0.5); +%OptimizeFunctionOnNextCall(test); +test(0.5);