Merge "Follow up of "div/rem on x86 and x86_64", to tidy up the code a little."

6 files changed, 37 insertions, 46 deletions

diff --git a/compiler/optimizing/code_generator_utils.cc b/compiler/optimizing/code_generator_utils.cc
index 26cab2ff09..921c1d86c2 100644
--- a/compiler/optimizing/code_generator_utils.cc
+++ b/compiler/optimizing/code_generator_utils.cc
@@ -18,13 +18,15 @@
 
 #include "base/logging.h"
 
+namespace art {
+
 void CalculateMagicAndShiftForDivRem(int64_t divisor, bool is_long,
                                      int64_t* magic, int* shift) {
   // It does not make sense to calculate magic and shift for zero divisor.
   DCHECK_NE(divisor, 0);
 
-  /* According to implementation from H.S.Warren's "Hacker's Delight" (Addison Wesley, 2002)
-   * Chapter 10 and T,Grablund, P.L.Montogomery's "Division by Invariant Integers Using
+  /* Implementation according to H.S.Warren's "Hacker's Delight" (Addison Wesley, 2002)
+   * Chapter 10 and T.Grablund, P.L.Montogomery's "Division by Invariant Integers Using
    * Multiplication" (PLDI 1994).
    * The magic number M and shift S can be calculated in the following way:
    * Let nc be the most positive value of numerator(n) such that nc = kd - 1,
@@ -39,11 +41,11 @@ void CalculateMagicAndShiftForDivRem(int64_t divisor, bool is_long,
    * 2^p > nc * (d - 2^p % d), where d >= 2
    * 2^p > nc * (d + 2^p % d), where d <= -2.
    *
-   * The magic number M is calcuated by
+   * The magic number M is calculated by
    * M = (2^p + d - 2^p % d) / d, where d >= 2
    * M = (2^p - d - 2^p % d) / d, where d <= -2.
    *
-   * Notice that p is always bigger than or equal to 32 (resp. 64), so we just return 32-p
+   * Notice that p is always bigger than or equal to 32 (resp. 64), so we just return 32 - p
    * (resp. 64 - p) as the shift number S.
    */
 
@@ -52,9 +54,10 @@ void CalculateMagicAndShiftForDivRem(int64_t divisor, bool is_long,
 
   // Initialize the computations.
   uint64_t abs_d = (divisor >= 0) ? divisor : -divisor;
-  uint64_t tmp = exp + (is_long ? static_cast<uint64_t>(divisor) >> 63 :
-                                    static_cast<uint32_t>(divisor) >> 31);
-  uint64_t abs_nc = tmp - 1 - tmp % abs_d;
+  uint64_t sign_bit = is_long ? static_cast<uint64_t>(divisor) >> 63 :
+                                static_cast<uint32_t>(divisor) >> 31;
+  uint64_t tmp = exp + sign_bit;
+  uint64_t abs_nc = tmp - 1 - (tmp % abs_d);
   uint64_t quotient1 = exp / abs_nc;
   uint64_t remainder1 = exp % abs_nc;
   uint64_t quotient2 = exp / abs_d;
@@ -91,3 +94,4 @@ void CalculateMagicAndShiftForDivRem(int64_t divisor, bool is_long,
   *shift = is_long ? p - 64 : p - 32;
 }
 
+}  // namespace art
diff --git a/compiler/optimizing/code_generator_utils.h b/compiler/optimizing/code_generator_utils.h
index 742d67565a..59b495c2c9 100644
--- a/compiler/optimizing/code_generator_utils.h
+++ b/compiler/optimizing/code_generator_utils.h
@@ -19,7 +19,12 @@
 
 #include <cstdint>
 
-// Computes the magic number and the shift needed in the div/rem by constant algorithm
+namespace art {
+
+// Computes the magic number and the shift needed in the div/rem by constant algorithm, as out
+// arguments `magic` and `shift`
 void CalculateMagicAndShiftForDivRem(int64_t divisor, bool is_long, int64_t* magic, int* shift);
 
+}  // namespace art
+
 #endif  // ART_COMPILER_OPTIMIZING_CODE_GENERATOR_UTILS_H_
diff --git a/compiler/optimizing/code_generator_x86.cc b/compiler/optimizing/code_generator_x86.cc
index 183453ded3..a6fb07fa98 100644
--- a/compiler/optimizing/code_generator_x86.cc
+++ b/compiler/optimizing/code_generator_x86.cc
@@ -2304,10 +2304,11 @@ void InstructionCodeGeneratorX86::DivRemOneOrMinusOne(HBinaryOperation* instruct
 
   LocationSummary* locations = instruction->GetLocations();
   DCHECK(locations->InAt(1).IsConstant());
+  DCHECK(locations->InAt(1).GetConstant()->IsIntConstant());
 
   Register out_register = locations->Out().AsRegister<Register>();
   Register input_register = locations->InAt(0).AsRegister<Register>();
-  int imm = locations->InAt(1).GetConstant()->AsIntConstant()->GetValue();
+  int32_t imm = locations->InAt(1).GetConstant()->AsIntConstant()->GetValue();
 
   DCHECK(imm == 1 || imm == -1);
 
@@ -2322,16 +2323,14 @@ void InstructionCodeGeneratorX86::DivRemOneOrMinusOne(HBinaryOperation* instruct
 }
 
 
-void InstructionCodeGeneratorX86::DivByPowerOfTwo(HBinaryOperation* instruction) {
-  DCHECK(instruction->IsDiv());
-
+void InstructionCodeGeneratorX86::DivByPowerOfTwo(HDiv* instruction) {
   LocationSummary* locations = instruction->GetLocations();
 
   Register out_register = locations->Out().AsRegister<Register>();
   Register input_register = locations->InAt(0).AsRegister<Register>();
-  int imm = locations->InAt(1).GetConstant()->AsIntConstant()->GetValue();
+  int32_t imm = locations->InAt(1).GetConstant()->AsIntConstant()->GetValue();
 
-  DCHECK(instruction->IsDiv() && IsPowerOfTwo(std::abs(imm)));
+  DCHECK(IsPowerOfTwo(std::abs(imm)));
   Register num = locations->GetTemp(0).AsRegister<Register>();
 
   __ leal(num, Address(input_register, std::abs(imm) - 1));
@@ -2440,15 +2439,15 @@ void InstructionCodeGeneratorX86::GenerateDivRemIntegral(HBinaryOperation* instr
       DCHECK_EQ(EAX, first.AsRegister<Register>());
       DCHECK_EQ(is_div ? EAX : EDX, out.AsRegister<Register>());
 
-      if (second.IsConstant()) {
-        int imm = second.GetConstant()->AsIntConstant()->GetValue();
+      if (instruction->InputAt(1)->IsIntConstant()) {
+        int32_t imm = second.GetConstant()->AsIntConstant()->GetValue();
 
         if (imm == 0) {
           // Do not generate anything for 0. DivZeroCheck would forbid any generated code.
         } else if (imm == 1 || imm == -1) {
           DivRemOneOrMinusOne(instruction);
         } else if (is_div && IsPowerOfTwo(std::abs(imm))) {
-          DivByPowerOfTwo(instruction);
+          DivByPowerOfTwo(instruction->AsDiv());
         } else {
           DCHECK(imm <= -2 || imm >= 2);
           GenerateDivRemWithAnyConstant(instruction);
@@ -2519,7 +2518,7 @@ void LocationsBuilderX86::VisitDiv(HDiv* div) {
       // We need to save the numerator while we tweak eax and edx. As we are using imul in a way
       // which enforces results to be in EAX and EDX, things are simpler if we use EAX also as
       // output and request another temp.
-      if (div->InputAt(1)->IsConstant()) {
+      if (div->InputAt(1)->IsIntConstant()) {
         locations->AddTemp(Location::RequiresRegister());
       }
       break;
@@ -2593,7 +2592,7 @@ void LocationsBuilderX86::VisitRem(HRem* rem) {
       // We need to save the numerator while we tweak eax and edx. As we are using imul in a way
       // which enforces results to be in EAX and EDX, things are simpler if we use EDX also as
       // output and request another temp.
-      if (rem->InputAt(1)->IsConstant()) {
+      if (rem->InputAt(1)->IsIntConstant()) {
         locations->AddTemp(Location::RequiresRegister());
       }
       break;
diff --git a/compiler/optimizing/code_generator_x86.h b/compiler/optimizing/code_generator_x86.h
index b2420e4076..8c56e35329 100644
--- a/compiler/optimizing/code_generator_x86.h
+++ b/compiler/optimizing/code_generator_x86.h
@@ -165,7 +165,7 @@ class InstructionCodeGeneratorX86 : public HGraphVisitor {
   void HandleBitwiseOperation(HBinaryOperation* instruction);
   void GenerateDivRemIntegral(HBinaryOperation* instruction);
   void DivRemOneOrMinusOne(HBinaryOperation* instruction);
-  void DivByPowerOfTwo(HBinaryOperation* instruction);
+  void DivByPowerOfTwo(HDiv* instruction);
   void GenerateDivRemWithAnyConstant(HBinaryOperation* instruction);
   void GenerateRemFP(HRem *rem);
   void HandleShift(HBinaryOperation* instruction);
diff --git a/compiler/optimizing/code_generator_x86_64.cc b/compiler/optimizing/code_generator_x86_64.cc
index f714214e8a..01b24ea33f 100644
--- a/compiler/optimizing/code_generator_x86_64.cc
+++ b/compiler/optimizing/code_generator_x86_64.cc
@@ -2348,12 +2348,7 @@ void InstructionCodeGeneratorX86_64::DivRemOneOrMinusOne(HBinaryOperation* instr
 
   CpuRegister output_register = locations->Out().AsRegister<CpuRegister>();
   CpuRegister input_register = locations->InAt(0).AsRegister<CpuRegister>();
-  int64_t imm;
-  if (second.GetConstant()->IsLongConstant()) {
-    imm = second.GetConstant()->AsLongConstant()->GetValue();
-  } else {
-    imm = second.GetConstant()->AsIntConstant()->GetValue();
-  }
+  int64_t imm = Int64FromConstant(second.GetConstant());
 
   DCHECK(imm == 1 || imm == -1);
 
@@ -2383,25 +2378,18 @@ void InstructionCodeGeneratorX86_64::DivRemOneOrMinusOne(HBinaryOperation* instr
     }
 
     default:
-      LOG(FATAL) << "Unreachable";
+      LOG(FATAL) << "Unexpected type for div by (-)1 " << instruction->GetResultType();
   }
 }
 
-void InstructionCodeGeneratorX86_64::DivByPowerOfTwo(HBinaryOperation* instruction) {
-  DCHECK(instruction->IsDiv());
-
+void InstructionCodeGeneratorX86_64::DivByPowerOfTwo(HDiv* instruction) {
   LocationSummary* locations = instruction->GetLocations();
   Location second = locations->InAt(1);
 
   CpuRegister output_register = locations->Out().AsRegister<CpuRegister>();
   CpuRegister numerator = locations->InAt(0).AsRegister<CpuRegister>();
 
-  int64_t imm;
-  if (instruction->GetResultType() == Primitive::kPrimLong) {
-    imm = second.GetConstant()->AsLongConstant()->GetValue();
-  } else {
-    imm = second.GetConstant()->AsIntConstant()->GetValue();
-  }
+  int64_t imm = Int64FromConstant(second.GetConstant());
 
   DCHECK(IsPowerOfTwo(std::abs(imm)));
 
@@ -2462,7 +2450,7 @@ void InstructionCodeGeneratorX86_64::GenerateDivRemWithAnyConstant(HBinaryOperat
   int64_t magic;
   int shift;
 
-  // TODO: can these branch be written as one?
+  // TODO: can these branches be written as one?
   if (instruction->GetResultType() == Primitive::kPrimInt) {
     int imm = second.GetConstant()->AsIntConstant()->GetValue();
 
@@ -2526,7 +2514,7 @@ void InstructionCodeGeneratorX86_64::GenerateDivRemWithAnyConstant(HBinaryOperat
     __ imulq(numerator);
 
     if (imm > 0 && magic < 0) {
-      // RDX += numeratorerator
+      // RDX += numerator
       __ addq(rdx, numerator);
     } else if (imm < 0 && magic > 0) {
       // RDX -= numerator
@@ -2576,19 +2564,14 @@ void InstructionCodeGeneratorX86_64::GenerateDivRemIntegral(HBinaryOperation* in
   DCHECK_EQ(is_div ? RAX : RDX, out.AsRegister());
 
   if (second.IsConstant()) {
-    int64_t imm;
-    if (second.GetConstant()->AsLongConstant()) {
-      imm = second.GetConstant()->AsLongConstant()->GetValue();
-    } else {
-      imm = second.GetConstant()->AsIntConstant()->GetValue();
-    }
+    int64_t imm = Int64FromConstant(second.GetConstant());
 
     if (imm == 0) {
       // Do not generate anything. DivZeroCheck would prevent any code to be executed.
     } else if (imm == 1 || imm == -1) {
       DivRemOneOrMinusOne(instruction);
     } else if (instruction->IsDiv() && IsPowerOfTwo(std::abs(imm))) {
-      DivByPowerOfTwo(instruction);
+      DivByPowerOfTwo(instruction->AsDiv());
     } else {
       DCHECK(imm <= -2 || imm >= 2);
       GenerateDivRemWithAnyConstant(instruction);
diff --git a/compiler/optimizing/code_generator_x86_64.h b/compiler/optimizing/code_generator_x86_64.h
index a1bbe47991..61bf6ac71d 100644
--- a/compiler/optimizing/code_generator_x86_64.h
+++ b/compiler/optimizing/code_generator_x86_64.h
@@ -175,7 +175,7 @@ class InstructionCodeGeneratorX86_64 : public HGraphVisitor {
   void HandleBitwiseOperation(HBinaryOperation* operation);
   void GenerateRemFP(HRem *rem);
   void DivRemOneOrMinusOne(HBinaryOperation* instruction);
-  void DivByPowerOfTwo(HBinaryOperation* instruction);
+  void DivByPowerOfTwo(HDiv* instruction);
   void GenerateDivRemWithAnyConstant(HBinaryOperation* instruction);
   void GenerateDivRemIntegral(HBinaryOperation* instruction);
   void HandleShift(HBinaryOperation* operation);