Handle negative input in inlined Math.round on Intel CPUs.

R=jkummerow@chromium.org
BUG=v8:2451

Review URL: https://chromiumcodereview.appspot.com/12342037

git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@13764 ce2b1a6d-e550-0410-aec6-3dcde31c8c00

src/assembler.cc

@@ -91,6 +91,7 @@ namespace internal {
 struct DoubleConstant BASE_EMBEDDED {
   double min_int;
   double one_half;
+  double minus_one_half;
   double minus_zero;
   double zero;
   double uint8_max_value;
@@ -853,6 +854,7 @@ void RelocInfo::Verify() {
 void ExternalReference::SetUp() {
   double_constants.min_int = kMinInt;
   double_constants.one_half = 0.5;
+  double_constants.minus_one_half = -0.5;
   double_constants.minus_zero = -0.0;
   double_constants.uint8_max_value = 255;
   double_constants.zero = 0.0;
@@ -1209,6 +1211,12 @@ ExternalReference ExternalReference::address_of_one_half() {
 }
 
 
+ExternalReference ExternalReference::address_of_minus_one_half() {
+  return ExternalReference(
+      reinterpret_cast<void*>(&double_constants.minus_one_half));
+}
+
+
 ExternalReference ExternalReference::address_of_minus_zero() {
   return ExternalReference(
       reinterpret_cast<void*>(&double_constants.minus_zero));

src/assembler.h

@@ -736,6 +736,7 @@ class ExternalReference BASE_EMBEDDED {
   // Static variables containing common double constants.
   static ExternalReference address_of_min_int();
   static ExternalReference address_of_one_half();
+  static ExternalReference address_of_minus_one_half();
   static ExternalReference address_of_minus_zero();
   static ExternalReference address_of_zero();
   static ExternalReference address_of_uint8_max_value();

src/ia32/lithium-codegen-ia32.cc

@@ -3734,47 +3734,75 @@ void LCodeGen::DoMathFloor(LUnaryMathOperation* instr) {
 
 void LCodeGen::DoMathRound(LUnaryMathOperation* instr) {
   CpuFeatures::Scope scope(SSE2);
-  XMMRegister xmm_scratch = xmm0;
   Register output_reg = ToRegister(instr->result());
   XMMRegister input_reg = ToDoubleRegister(instr->value());
-
-  Label below_half, done;
-  // xmm_scratch = 0.5
+  XMMRegister xmm_scratch = xmm0;
   ExternalReference one_half = ExternalReference::address_of_one_half();
+  ExternalReference minus_one_half =
+      ExternalReference::address_of_minus_one_half();
+  bool minus_zero_check =
+      instr->hydrogen()->CheckFlag(HValue::kBailoutOnMinusZero);
+
   __ movdbl(xmm_scratch, Operand::StaticVariable(one_half));
-  __ ucomisd(xmm_scratch, input_reg);
-  __ j(above, &below_half);
-  // xmm_scratch = input + 0.5
-  __ addsd(xmm_scratch, input_reg);
 
-  // Compute Math.floor(value + 0.5).
-  // Use truncating instruction (OK because input is positive).
+  if (CpuFeatures::IsSupported(SSE4_1) && !minus_zero_check) {
+    CpuFeatures::Scope scope(SSE4_1);
+
+    __ addsd(xmm_scratch, input_reg);
+    __ roundsd(xmm_scratch, input_reg, Assembler::kRoundDown);
     __ cvttsd2si(output_reg, Operand(xmm_scratch));
+    // Overflow is signalled with minint.
+    __ cmp(output_reg, 0x80000000u);
+    __ RecordComment("D2I conversion overflow");
+    DeoptimizeIf(equal, instr->environment());
+  } else {
+    Label done, round_to_zero, below_one_half, do_not_compensate;
+    __ ucomisd(xmm_scratch, input_reg);
+    __ j(above, &below_one_half);
 
+    // CVTTSD2SI rounds towards zero, since 0.5 <= x, we use floor(0.5 + x).
+    __ addsd(xmm_scratch, input_reg);
+    __ cvttsd2si(output_reg, Operand(xmm_scratch));
     // Overflow is signalled with minint.
     __ cmp(output_reg, 0x80000000u);
+    __ RecordComment("D2I conversion overflow");
     DeoptimizeIf(equal, instr->environment());
     __ jmp(&done);
 
-  __ bind(&below_half);
+    __ bind(&below_one_half);
+    __ movdbl(xmm_scratch, Operand::StaticVariable(minus_one_half));
+    __ ucomisd(xmm_scratch, input_reg);
+    __ j(below_equal, &round_to_zero);
 
+    // CVTTSD2SI rounds towards zero, we use ceil(x - (-0.5)) and then
+    // compare and compensate.
+    __ subsd(input_reg, xmm_scratch);
+    __ cvttsd2si(output_reg, Operand(input_reg));
+    // Catch minint due to overflow, and to prevent overflow when compensating.
+    __ cmp(output_reg, 0x80000000u);
+    __ RecordComment("D2I conversion overflow");
+    DeoptimizeIf(equal, instr->environment());
+
+    __ cvtsi2sd(xmm_scratch, output_reg);
+    __ ucomisd(xmm_scratch, input_reg);
+    __ j(equal, &done);
+    __ sub(output_reg, Immediate(1));
+    // No overflow because we already ruled out minint.
+    __ jmp(&done);
+
+    __ bind(&round_to_zero);
     // We return 0 for the input range [+0, 0.5[, or [-0.5, 0.5[ if
     // we can ignore the difference between a result of -0 and +0.
-  if (instr->hydrogen()->CheckFlag(HValue::kBailoutOnMinusZero)) {
+    if (minus_zero_check) {
       // If the sign is positive, we return +0.
       __ movmskpd(output_reg, input_reg);
       __ test(output_reg, Immediate(1));
+      __ RecordComment("Minus zero");
       DeoptimizeIf(not_zero, instr->environment());
-  } else {
-    // If the input is >= -0.5, we return +0.
-    __ mov(output_reg, Immediate(0xBF000000));
-    __ movd(xmm_scratch, Operand(output_reg));
-    __ cvtss2sd(xmm_scratch, xmm_scratch);
-    __ ucomisd(input_reg, xmm_scratch);
-    DeoptimizeIf(below, instr->environment());
     }
     __ Set(output_reg, Immediate(0));
     __ bind(&done);
+  }
 }
 
 

src/ia32/lithium-ia32.cc

@@ -1145,7 +1145,16 @@ LInstruction* LChunkBuilder::DoUnaryMathOperation(HUnaryMathOperation* instr) {
                                                                   input);
     return MarkAsCall(DefineFixedDouble(result, xmm1), instr);
   } else {
-    LOperand* input = UseRegisterAtStart(instr->value());
+    LOperand* input;
+    if (op == kMathRound &&
+        (!CpuFeatures::IsSupported(SSE4_1) ||
+         instr->CheckFlag(HValue::kBailoutOnMinusZero))) {
+      // Math.round implemented without roundsd.  Input may be overwritten.
+      ASSERT(instr->value()->representation().IsDouble());
+      input = UseTempRegister(instr->value());
+    } else {
+      input = UseRegisterAtStart(instr->value());
+    }
     LOperand* context = UseAny(instr->context());  // Deferred use by MathAbs.
     if (op == kMathPowHalf) {
       LOperand* temp = TempRegister();

src/x64/lithium-codegen-x64.cc

@@ -3486,7 +3486,7 @@ void LCodeGen::DoMathFloor(LUnaryMathOperation* instr) {
     DeoptimizeIf(equal, instr->environment());
   } else {
     Label negative_sign, done;
-    // Deoptimize on negative inputs.
+    // Deoptimize on unordered.
     __ xorps(xmm_scratch, xmm_scratch);  // Zero the register.
     __ ucomisd(input_reg, xmm_scratch);
     DeoptimizeIf(parity_even, instr->environment());
@@ -3530,48 +3530,72 @@ void LCodeGen::DoMathRound(LUnaryMathOperation* instr) {
   const XMMRegister xmm_scratch = xmm0;
   Register output_reg = ToRegister(instr->result());
   XMMRegister input_reg = ToDoubleRegister(instr->value());
+  static int64_t one_half = V8_INT64_C(0x3FE0000000000000);  // 0.5
+  static int64_t minus_one_half = V8_INT64_C(0xBFE0000000000000);  // -0.5
 
-  Label done;
-  // xmm_scratch = 0.5
-  __ movq(kScratchRegister, V8_INT64_C(0x3FE0000000000000), RelocInfo::NONE64);
+  bool minus_zero_check =
+      instr->hydrogen()->CheckFlag(HValue::kBailoutOnMinusZero);
+
+  __ movq(kScratchRegister, one_half, RelocInfo::NONE64);
   __ movq(xmm_scratch, kScratchRegister);
-  Label below_half;
+
+  if (CpuFeatures::IsSupported(SSE4_1) && !minus_zero_check) {
+    CpuFeatures::Scope scope(SSE4_1);
+    __ addsd(xmm_scratch, input_reg);
+    __ roundsd(xmm_scratch, input_reg, Assembler::kRoundDown);
+    __ cvttsd2si(output_reg, xmm_scratch);
+    // Overflow is signalled with minint.
+    __ cmpl(output_reg, Immediate(0x80000000));
+    __ RecordComment("D2I conversion overflow");
+    DeoptimizeIf(equal, instr->environment());
+  } else {
+    Label done, round_to_zero, below_one_half, do_not_compensate;
     __ ucomisd(xmm_scratch, input_reg);
-  // If input_reg is NaN, this doesn't jump.
-  __ j(above, &below_half, Label::kNear);
-  // input = input + 0.5
-  // 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.
+    __ j(above, &below_one_half);
+
+    // CVTTSD2SI rounds towards zero, since 0.5 <= x, we use floor(0.5 + x).
     __ addsd(xmm_scratch, input_reg);
-  // Compute Math.floor(input).
-  // Use truncating instruction (OK because input is positive).
     __ cvttsd2si(output_reg, xmm_scratch);
     // Overflow is signalled with minint.
     __ cmpl(output_reg, Immediate(0x80000000));
+    __ RecordComment("D2I conversion overflow");
     DeoptimizeIf(equal, instr->environment());
     __ jmp(&done);
 
-  __ bind(&below_half);
-  if (instr->hydrogen()->CheckFlag(HValue::kBailoutOnMinusZero)) {
-    // Bailout if negative (including -0).
+    __ bind(&below_one_half);
+    __ movq(kScratchRegister, minus_one_half, RelocInfo::NONE64);
+    __ movq(xmm_scratch, kScratchRegister);
+    __ ucomisd(xmm_scratch, input_reg);
+    __ j(below_equal, &round_to_zero);
+
+    // CVTTSD2SI rounds towards zero, we use ceil(x - (-0.5)) and then
+    // compare and compensate.
+    __ subsd(input_reg, xmm_scratch);
+    __ cvttsd2si(output_reg, input_reg);
+    // 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, &done, Label::kNear);
+    __ subl(output_reg, Immediate(1));
+    // No overflow because we already ruled out minint.
+    __ jmp(&done);
+
+    __ bind(&round_to_zero);
+    // We return 0 for the input range [+0, 0.5[, or [-0.5, 0.5[ if
+    // we can ignore the difference between a result of -0 and +0.
+    if (minus_zero_check) {
       __ movq(output_reg, input_reg);
       __ testq(output_reg, output_reg);
+      __ RecordComment("Minus zero");
       DeoptimizeIf(negative, instr->environment());
-  } else {
-    // Bailout if below -0.5, otherwise round to (positive) zero, even
-    // if negative.
-    // xmm_scrach = -0.5
-    __ movq(kScratchRegister,
-            V8_INT64_C(0xBFE0000000000000),
-            RelocInfo::NONE64);
-    __ movq(xmm_scratch, kScratchRegister);
-    __ ucomisd(input_reg, xmm_scratch);
-    DeoptimizeIf(below, instr->environment());
     }
-  __ xorl(output_reg, output_reg);
-
+    __ Set(output_reg, 0);
     __ bind(&done);
+  }
 }
 
 

src/x64/lithium-x64.cc

@@ -1084,7 +1084,16 @@ LInstruction* LChunkBuilder::DoUnaryMathOperation(HUnaryMathOperation* instr) {
     LMathExp* result = new(zone()) LMathExp(value, temp1, temp2);
     return DefineAsRegister(result);
   } else {
-    LOperand* input = UseRegisterAtStart(instr->value());
+    LOperand* input;
+    if (op == kMathRound &&
+        (!CpuFeatures::IsSupported(SSE4_1) ||
+         instr->CheckFlag(HValue::kBailoutOnMinusZero))) {
+      // Math.round implemented without roundsd.  Input may be overwritten.
+      ASSERT(instr->value()->representation().IsDouble());
+      input = UseTempRegister(instr->value());
+    } else {
+      input = UseRegisterAtStart(instr->value());
+    }
     LUnaryMathOperation* result = new(zone()) LUnaryMathOperation(input);
     switch (op) {
       case kMathAbs:

test/mjsunit/regress/regress-2451.js

+// Copyright 2013 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
+
+function f() {
+  assertEquals(-1.0, Math.round(-1.5));
+  assertEquals(-2.0, Math.round(-2.5));
+  assertEquals(-1.0, Math.round(-0.5000000000000001));
+}
+
+f();
+f();
+%OptimizeFunctionOnNextCall(f);
+f();
+assertTrue(%GetOptimizationStatus(f) != 2);
+