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 (file)
--- 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 (file)
--- 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 (file)
--- 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 (file)
--- 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 (file)
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);