[v8-dev] Fix FlooringDivByPowerOf2I on ARM/ARM64 (issue 280883003)

Wed, 14 May 2014 02:59:24 -0700 

Reviewers: ulan, jochen,

Description:
Fix FlooringDivByPowerOf2I on ARM/ARM64
Fix for divisor=1, found during code inspection. We can't hit this bug, due to HDiv::Canonicalize() discarding the operation, but it might avoid a future bug 
hunt. Patch also includes a small tidying of the ARM64 code.

BUG=

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

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

Affected files (+14, -10 lines):
  M src/arm/lithium-codegen-arm.cc
  M src/arm64/lithium-codegen-arm64.cc


Index: src/arm/lithium-codegen-arm.cc
diff --git a/src/arm/lithium-codegen-arm.cc b/src/arm/lithium-codegen-arm.cc
index 2879d6411112255da7053dfa1d94c2b71b647beb..86aa0b5d5299930646fcc78d71ca7f55cfb0f2c7 100644 
--- a/src/arm/lithium-codegen-arm.cc
+++ b/src/arm/lithium-codegen-arm.cc
@@ -1435,9 +1435,14 @@ void LCodeGen::DoFlooringDivByPowerOf2I(LFlooringDivByPowerOf2I* instr) { 
   Register result = ToRegister(instr->result());
   int32_t divisor = instr->divisor();

+  // If the divisor is 1, return the dividend.
+  if (divisor == 1) {
+    __ Move(result, dividend);
+    return;
+  }
+
// If the divisor is positive, things are easy: There can be no deopts and we 
   // can simply do an arithmetic right shift.
-  if (divisor == 1) return;
   int32_t shift = WhichPowerOf2Abs(divisor);
   if (divisor > 1) {
     __ mov(result, Operand(dividend, ASR, shift));
Index: src/arm64/lithium-codegen-arm64.cc
diff --git a/src/arm64/lithium-codegen-arm64.cc b/src/arm64/lithium-codegen-arm64.cc index caa902e282db1a3de061ba5b3b8cae7f63f71d50..5b67ac2b63a4604e7072ec31b96b1220a9b98d52 100644 
--- a/src/arm64/lithium-codegen-arm64.cc
+++ b/src/arm64/lithium-codegen-arm64.cc
@@ -3870,9 +3870,14 @@ void LCodeGen::DoFlooringDivByPowerOf2I(LFlooringDivByPowerOf2I* instr) { 
   Register result = ToRegister32(instr->result());
   int32_t divisor = instr->divisor();

+  // If the divisor is 1, return the dividend.
+  if (divisor == 1) {
+    __ Mov(result, dividend, kDiscardForSameWReg);
+    return;
+  }
+
// If the divisor is positive, things are easy: There can be no deopts and we 
   // can simply do an arithmetic right shift.
-  if (divisor == 1) return;
   int32_t shift = WhichPowerOf2Abs(divisor);
   if (divisor > 1) {
     __ Mov(result, Operand(dividend, ASR, shift));
@@ -3897,14 +3902,8 @@ void LCodeGen::DoFlooringDivByPowerOf2I(LFlooringDivByPowerOf2I* instr) { 
     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);
+  __ Asr(result, dividend, shift);
+  __ Csel(result, result, kMinInt / divisor, vc);
 }




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