[v8-dev] Correctly retain argument value when deopting from Math.round on x64. (issue 199013002)

Thu, 13 Mar 2014 05:46:26 -0700 

Reviewers: Jakob,

Description:
Correctly retain argument value when deopting from Math.round on x64.

[email protected]
BUG=351624
LOG=N

Please review this at https://codereview.chromium.org/199013002/

SVN Base: https://v8.googlecode.com/svn/branches/bleeding_edge

Affected files (+40, -11 lines):
  M src/x64/lithium-codegen-x64.cc
  M src/x64/lithium-x64.h
  M src/x64/lithium-x64.cc
  A test/mjsunit/regress/regress-351624.js


Index: src/x64/lithium-codegen-x64.cc
diff --git a/src/x64/lithium-codegen-x64.cc b/src/x64/lithium-codegen-x64.cc
index f350b07d5bfc64ea2acb0330bcb8569eefdfeaa8..d4271ace52d249d4c9e16c716cd99da0b72c8e89 100644 
--- a/src/x64/lithium-codegen-x64.cc
+++ b/src/x64/lithium-codegen-x64.cc
@@ -3604,6 +3604,7 @@ void LCodeGen::DoMathRound(LMathRound* instr) {
   const XMMRegister xmm_scratch = double_scratch0();
   Register output_reg = ToRegister(instr->result());
   XMMRegister input_reg = ToDoubleRegister(instr->value());
+  XMMRegister input_temp = ToDoubleRegister(instr->temp());
   static int64_t one_half = V8_INT64_C(0x3FE0000000000000);  // 0.5
   static int64_t minus_one_half = V8_INT64_C(0xBFE0000000000000);  // -0.5

@@ -3631,21 +3632,19 @@ void LCodeGen::DoMathRound(LMathRound* instr) {

   // CVTTSD2SI rounds towards zero, we use ceil(x - (-0.5)) and then
   // compare and compensate.
-  __ movq(kScratchRegister, input_reg);  // Back up input_reg.
-  __ subsd(input_reg, xmm_scratch);
-  __ cvttsd2si(output_reg, input_reg);
+  __ movq(input_temp, input_reg);  // Do not alter input_reg.
+  __ subsd(input_temp, xmm_scratch);
+  __ cvttsd2si(output_reg, input_temp);
// Catch minint due to overflow, and to prevent overflow when compensating. 
   __ cmpl(output_reg, Immediate(0x80000000));
   __ RecordComment("D2I conversion overflow");
   DeoptimizeIf(equal, instr->environment());

   __ Cvtlsi2sd(xmm_scratch, output_reg);
-  __ ucomisd(input_reg, xmm_scratch);
-  __ j(equal, &restore, Label::kNear);
+  __ ucomisd(xmm_scratch, input_temp);
+  __ j(equal, &done, dist);
   __ subl(output_reg, Immediate(1));
   // No overflow because we already ruled out minint.
-  __ bind(&restore);
-  __ movq(input_reg, kScratchRegister);  // Restore input_reg.
   __ jmp(&done, dist);

   __ bind(&round_to_zero);
Index: src/x64/lithium-x64.cc
diff --git a/src/x64/lithium-x64.cc b/src/x64/lithium-x64.cc
index 625b4f4ce5f57c7f450c9176aa6c322369e0eada..aad9597f1148c9396da9220feb0da202873ef123 100644 
--- a/src/x64/lithium-x64.cc
+++ b/src/x64/lithium-x64.cc
@@ -1133,8 +1133,9 @@ LInstruction* LChunkBuilder::DoMathFloor(HUnaryMathOperation* instr) { 


 LInstruction* LChunkBuilder::DoMathRound(HUnaryMathOperation* instr) {
-  LOperand* input = UseRegisterAtStart(instr->value());
-  LMathRound* result = new(zone()) LMathRound(input);
+  LOperand* input = UseRegister(instr->value());
+  LOperand* temp = FixedTemp(xmm4);
+  LMathRound* result = new(zone()) LMathRound(input, temp);
   return AssignEnvironment(DefineAsRegister(result));
 }

Index: src/x64/lithium-x64.h
diff --git a/src/x64/lithium-x64.h b/src/x64/lithium-x64.h
index e8760723b9f135b3d224c30ab19a2ded45a0b8b4..8ae13197321cf06e7f9f3c2a3e71fc8c9b6f83ad 100644 
--- a/src/x64/lithium-x64.h
+++ b/src/x64/lithium-x64.h
@@ -835,13 +835,15 @@ class LMathFloor V8_FINAL : public LTemplateInstruction<1, 1, 0> { 
 };


-class LMathRound V8_FINAL : public LTemplateInstruction<1, 1, 0> {
+class LMathRound V8_FINAL : public LTemplateInstruction<1, 1, 1> {
  public:
-  explicit LMathRound(LOperand* value) {
+  explicit LMathRound(LOperand* value, LOperand* temp) {
     inputs_[0] = value;
+    temps_[0] = temp;
   }

   LOperand* value() { return inputs_[0]; }
+  LOperand* temp() { return temps_[0]; }

   DECLARE_CONCRETE_INSTRUCTION(MathRound, "math-round")
   DECLARE_HYDROGEN_ACCESSOR(UnaryMathOperation)
Index: test/mjsunit/regress/regress-351624.js
diff --git a/test/mjsunit/regress/regress-351624.js b/test/mjsunit/regress/regress-351624.js 
new file mode 100644
index 0000000000000000000000000000000000000000..3d217fff6685755e80041c37f918e50e18147006 
--- /dev/null
+++ b/test/mjsunit/regress/regress-351624.js
@@ -0,0 +1,27 @@
+// Copyright 2014 the V8 project authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+// Flags: --allow-natives-syntax
+
+var big = 1e10;
+var backup = new Float64Array(1);
+
+function mult0(val){
+  var prod = val * big;
+  backup[0] = prod;
+  print("input: " + prod);
+  var rounded = Math.round(prod);
+  print("rounded: " + rounded);
+  if (prod !== backup[0]) throw new Error("arg!");
+  return rounded;
+}
+
+var count = 5;
+for (var i = 0; i < count; i++) {
+  if (i == count - 1) %OptimizeFunctionOnNextCall(mult0);
+  var result = mult0(-1);
+  if (result != -big) {
+    throw new Error('Iteration ' + i + ': ' + result);
+  }
+}


--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
--- You received this message because you are subscribed to the Google Groups "v8-dev" group. 
To unsubscribe from this group and stop receiving emails from it, send an email 
to [email protected].
For more options, visit https://groups.google.com/d/optout.

Reply via email to