From 23fc5b75a8ca64e77e5321a29feb38a0ea1026cd Mon Sep 17 00:00:00 2001
From: "svenpanne@chromium.org"
 <svenpanne@chromium.org@ce2b1a6d-e550-0410-aec6-3dcde31c8c00>
Date: Wed, 11 Jun 2014 13:29:25 +0000
Subject: [PATCH] Fixed flooring division by a power of 2, once again...

Avoid right shifts by zero bits: On ARM it actually means shifting by
32 bits (correctness issue) and on other platforms they are useless
(performance issue). This is fix for the fix in r20544.

BUG=v8:3259
LOG=y
R=yangguo@chromium.org

Review URL: https://codereview.chromium.org/324403003

git-svn-id: https://v8.googlecode.com/svn/branches/bleeding_edge@21769 ce2b1a6d-e550-0410-aec6-3dcde31c8c00
---
 src/arm/lithium-codegen-arm.cc     | 16 +++++++++-------
 src/arm64/lithium-codegen-arm64.cc | 16 +++++++++-------
 src/ia32/lithium-codegen-ia32.cc   | 13 ++++++++-----
 src/x64/lithium-codegen-x64.cc     | 15 ++++++++-------
 test/mjsunit/mjsunit.status        |  4 ----
 5 files changed, 34 insertions(+), 30 deletions(-)

diff --git a/src/arm/lithium-codegen-arm.cc b/src/arm/lithium-codegen-arm.cc
index 800f67b717..249c22f77b 100644
--- a/src/arm/lithium-codegen-arm.cc
+++ b/src/arm/lithium-codegen-arm.cc
@@ -1469,20 +1469,22 @@ void LCodeGen::DoFlooringDivByPowerOf2I(LFlooringDivByPowerOf2I* instr) {
     DeoptimizeIf(eq, instr->environment());
   }
 
-  // If the negation could not overflow, simply shifting is OK.
-  if (!instr->hydrogen()->CheckFlag(HValue::kLeftCanBeMinInt)) {
-    __ mov(result, Operand(dividend, ASR, shift));
+  // Dividing by -1 is basically negation, unless we overflow.
+  if (divisor == -1) {
+    if (instr->hydrogen()->CheckFlag(HValue::kLeftCanBeMinInt)) {
+      DeoptimizeIf(vs, instr->environment());
+    }
     return;
   }
 
-  // Dividing by -1 is basically negation, unless we overflow.
-  if (divisor == -1) {
-    DeoptimizeIf(vs, instr->environment());
+  // If the negation could not overflow, simply shifting is OK.
+  if (!instr->hydrogen()->CheckFlag(HValue::kLeftCanBeMinInt)) {
+    __ mov(result, Operand(result, ASR, shift));
     return;
   }
 
   __ mov(result, Operand(kMinInt / divisor), LeaveCC, vs);
-  __ mov(result, Operand(dividend, ASR, shift), LeaveCC, vc);
+  __ mov(result, Operand(result, ASR, shift), LeaveCC, vc);
 }
 
 
diff --git a/src/arm64/lithium-codegen-arm64.cc b/src/arm64/lithium-codegen-arm64.cc
index f301c8b765..c86efbd3a4 100644
--- a/src/arm64/lithium-codegen-arm64.cc
+++ b/src/arm64/lithium-codegen-arm64.cc
@@ -3933,19 +3933,21 @@ void LCodeGen::DoFlooringDivByPowerOf2I(LFlooringDivByPowerOf2I* instr) {
     DeoptimizeIf(eq, instr->environment());
   }
 
+  // Dividing by -1 is basically negation, unless we overflow.
+  if (divisor == -1) {
+    if (instr->hydrogen()->CheckFlag(HValue::kLeftCanBeMinInt)) {
+      DeoptimizeIf(vs, instr->environment());
+    }
+    return;
+  }
+
   // If the negation could not overflow, simply shifting is OK.
   if (!instr->hydrogen()->CheckFlag(HValue::kLeftCanBeMinInt)) {
     __ Mov(result, Operand(dividend, ASR, shift));
     return;
   }
 
-  // Dividing by -1 is basically negation, unless we overflow.
-  if (divisor == -1) {
-    DeoptimizeIf(vs, instr->environment());
-    return;
-  }
-
-  __ Asr(result, dividend, shift);
+  __ Asr(result, result, shift);
   __ Csel(result, result, kMinInt / divisor, vc);
 }
 
diff --git a/src/ia32/lithium-codegen-ia32.cc b/src/ia32/lithium-codegen-ia32.cc
index 791faa2660..1e4f7561be 100644
--- a/src/ia32/lithium-codegen-ia32.cc
+++ b/src/ia32/lithium-codegen-ia32.cc
@@ -1353,14 +1353,17 @@ void LCodeGen::DoFlooringDivByPowerOf2I(LFlooringDivByPowerOf2I* instr) {
     DeoptimizeIf(zero, instr->environment());
   }
 
-  if (!instr->hydrogen()->CheckFlag(HValue::kLeftCanBeMinInt)) {
-    __ sar(dividend, shift);
+  // Dividing by -1 is basically negation, unless we overflow.
+  if (divisor == -1) {
+    if (instr->hydrogen()->CheckFlag(HValue::kLeftCanBeMinInt)) {
+      DeoptimizeIf(overflow, instr->environment());
+    }
     return;
   }
 
-  // Dividing by -1 is basically negation, unless we overflow.
-  if (divisor == -1) {
-    DeoptimizeIf(overflow, instr->environment());
+  // If the negation could not overflow, simply shifting is OK.
+  if (!instr->hydrogen()->CheckFlag(HValue::kLeftCanBeMinInt)) {
+    __ sar(dividend, shift);
     return;
   }
 
diff --git a/src/x64/lithium-codegen-x64.cc b/src/x64/lithium-codegen-x64.cc
index 41321340e0..809df5d932 100644
--- a/src/x64/lithium-codegen-x64.cc
+++ b/src/x64/lithium-codegen-x64.cc
@@ -1134,16 +1134,17 @@ void LCodeGen::DoFlooringDivByPowerOf2I(LFlooringDivByPowerOf2I* instr) {
     DeoptimizeIf(zero, instr->environment());
   }
 
-  // If the negation could not overflow, simply shifting is OK.
-  if (!instr->hydrogen()->CheckFlag(HValue::kLeftCanBeMinInt)) {
-    __ sarl(dividend, Immediate(shift));
+  // Dividing by -1 is basically negation, unless we overflow.
+  if (divisor == -1) {
+    if (instr->hydrogen()->CheckFlag(HValue::kLeftCanBeMinInt)) {
+      DeoptimizeIf(overflow, instr->environment());
+    }
     return;
   }
 
-  // Note that we could emit branch-free code, but that would need one more
-  // register.
-  if (divisor == -1) {
-    DeoptimizeIf(overflow, instr->environment());
+  // If the negation could not overflow, simply shifting is OK.
+  if (!instr->hydrogen()->CheckFlag(HValue::kLeftCanBeMinInt)) {
+    __ sarl(dividend, Immediate(shift));
     return;
   }
 
diff --git a/test/mjsunit/mjsunit.status b/test/mjsunit/mjsunit.status
index 70653f657b..3a6b28136b 100644
--- a/test/mjsunit/mjsunit.status
+++ b/test/mjsunit/mjsunit.status
@@ -308,10 +308,6 @@
   # Currently always deopt on minus zero
   'math-floor-of-div-minus-zero': [SKIP],
 
-  # Issue 3259.
-  'math-floor-of-div-nosudiv': [PASS, FAIL],
-  'math-floor-of-div': [PASS, FAIL],
-
   ############################################################################
   # Slow tests.
   'regress/regress-2185-2': [PASS, SLOW],