[v8-dev] [v8] r20544 committed - Fixed flooring division by -1 on ARM....

Mon, 07 Apr 2014 05:09:25 -0700 

Revision: 20544
Author:   [email protected]
Date:     Mon Apr  7 12:08:40 2014 UTC
Log:      Fixed flooring division by -1 on ARM.

We should avoid ASR #0 on ARM. Simplified and improved code a bit
while we're there on all platforms. Human brains are very bad at
understanding nested structures...

BUG=v8:3259
LOG=y
[email protected]

Review URL: https://codereview.chromium.org/227553003
http://code.google.com/p/v8/source/detail?r=20544

Modified:
 /branches/bleeding_edge/src/arm/lithium-codegen-arm.cc
 /branches/bleeding_edge/src/arm64/lithium-codegen-arm64.cc
 /branches/bleeding_edge/src/ia32/lithium-codegen-ia32.cc
 /branches/bleeding_edge/src/x64/lithium-codegen-x64.cc

=======================================
--- /branches/bleeding_edge/src/arm/lithium-codegen-arm.cc Fri Apr 4 16:18:59 2014 UTC +++ /branches/bleeding_edge/src/arm/lithium-codegen-arm.cc Mon Apr 7 12:08:40 2014 UTC 
@@ -1469,19 +1469,21 @@
   if (instr->hydrogen()->CheckFlag(HValue::kBailoutOnMinusZero)) {
     DeoptimizeIf(eq, instr->environment());
   }
-  if (instr->hydrogen()->CheckFlag(HValue::kLeftCanBeMinInt)) {
- // Note that we could emit branch-free code, but that would need one more 
-    // register.
-    if (divisor == -1) {
-      DeoptimizeIf(vs, instr->environment());
-      __ mov(result, Operand(dividend, ASR, shift));
-    } else {
-      __ mov(result, Operand(kMinInt / divisor), LeaveCC, vs);
-      __ mov(result, Operand(dividend, ASR, shift), LeaveCC, vc);
-    }
-  } else {
+
+  // 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;
   }
+
+  __ mov(result, Operand(kMinInt / divisor), LeaveCC, vs);
+  __ mov(result, Operand(dividend, ASR, shift), LeaveCC, vc);
 }


=======================================
--- /branches/bleeding_edge/src/arm64/lithium-codegen-arm64.cc Fri Apr 4 16:18:59 2014 UTC +++ /branches/bleeding_edge/src/arm64/lithium-codegen-arm64.cc Mon Apr 7 12:08:40 2014 UTC 
@@ -3851,22 +3851,28 @@
   }

   // If the divisor is negative, we have to negate and handle edge cases.
-  Label not_kmin_int, done;
   __ Negs(result, dividend);
   if (instr->hydrogen()->CheckFlag(HValue::kBailoutOnMinusZero)) {
     DeoptimizeIf(eq, instr->environment());
   }
-  if (instr->hydrogen()->CheckFlag(HValue::kLeftCanBeMinInt)) {
- // Note that we could emit branch-free code, but that would need one more 
-    // register.
-    if (divisor == -1) {
-      DeoptimizeIf(vs, instr->environment());
-    } else {
-      __ B(vc, &not_kmin_int);
-      __ Mov(result, kMinInt / divisor);
-      __ B(&done);
-    }
+
+  // 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;
   }
+
+ // Using a conditional data processing instruction would need 1 more register. 
+  Label not_kmin_int, done;
+  __ B(vc, &not_kmin_int);
+  __ Mov(result, kMinInt / divisor);
+  __ B(&done);
   __ bind(&not_kmin_int);
   __ Mov(result, Operand(dividend, ASR, shift));
   __ bind(&done);
=======================================
--- /branches/bleeding_edge/src/ia32/lithium-codegen-ia32.cc Fri Apr 4 16:18:59 2014 UTC +++ /branches/bleeding_edge/src/ia32/lithium-codegen-ia32.cc Mon Apr 7 12:08:40 2014 UTC 
@@ -1622,22 +1622,26 @@
   }

   // If the divisor is negative, we have to negate and handle edge cases.
-  Label not_kmin_int, done;
   __ neg(dividend);
   if (instr->hydrogen()->CheckFlag(HValue::kBailoutOnMinusZero)) {
     DeoptimizeIf(zero, instr->environment());
   }
-  if (instr->hydrogen()->CheckFlag(HValue::kLeftCanBeMinInt)) {
- // Note that we could emit branch-free code, but that would need one more 
-    // register.
-    if (divisor == -1) {
-      DeoptimizeIf(overflow, instr->environment());
-    } else {
-      __ j(no_overflow, &not_kmin_int, Label::kNear);
-      __ mov(dividend, Immediate(kMinInt / divisor));
-      __ jmp(&done, Label::kNear);
-    }
+
+  if (!instr->hydrogen()->CheckFlag(HValue::kLeftCanBeMinInt)) {
+    __ sar(dividend, shift);
+    return;
+  }
+
+  // Dividing by -1 is basically negation, unless we overflow.
+  if (divisor == -1) {
+    DeoptimizeIf(overflow, instr->environment());
+    return;
   }
+
+  Label not_kmin_int, done;
+  __ j(no_overflow, &not_kmin_int, Label::kNear);
+  __ mov(dividend, Immediate(kMinInt / divisor));
+  __ jmp(&done, Label::kNear);
   __ bind(&not_kmin_int);
   __ sar(dividend, shift);
   __ bind(&done);
=======================================
--- /branches/bleeding_edge/src/x64/lithium-codegen-x64.cc Fri Apr 4 16:18:59 2014 UTC +++ /branches/bleeding_edge/src/x64/lithium-codegen-x64.cc Mon Apr 7 12:08:40 2014 UTC 
@@ -1139,22 +1139,28 @@
   }

   // If the divisor is negative, we have to negate and handle edge cases.
-  Label not_kmin_int, done;
   __ negl(dividend);
   if (instr->hydrogen()->CheckFlag(HValue::kBailoutOnMinusZero)) {
     DeoptimizeIf(zero, instr->environment());
   }
-  if (instr->hydrogen()->CheckFlag(HValue::kLeftCanBeMinInt)) {
- // Note that we could emit branch-free code, but that would need one more 
-    // register.
-    __ j(no_overflow, &not_kmin_int, Label::kNear);
-    if (divisor == -1) {
-      DeoptimizeIf(no_condition, instr->environment());
-    } else {
-      __ movl(dividend, Immediate(kMinInt / divisor));
-      __ jmp(&done, Label::kNear);
-    }
+
+  // If the negation could not overflow, simply shifting is OK.
+  if (!instr->hydrogen()->CheckFlag(HValue::kLeftCanBeMinInt)) {
+    __ sarl(dividend, Immediate(shift));
+    return;
+  }
+
+  // Note that we could emit branch-free code, but that would need one more
+  // register.
+  if (divisor == -1) {
+    DeoptimizeIf(overflow, instr->environment());
+    return;
   }
+
+  Label not_kmin_int, done;
+  __ j(no_overflow, &not_kmin_int, Label::kNear);
+  __ movl(dividend, Immediate(kMinInt / divisor));
+  __ jmp(&done, Label::kNear);
   __ bind(&not_kmin_int);
   __ sarl(dividend, Immediate(shift));
   __ bind(&done);

--
--
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