Follow up of "div/rem on x86 and x86_64", to tidy up the code a little.
Change-Id: Ibf39cbc8ac1d773599d70be2cb1e941674b60f1d
diff --git a/compiler/optimizing/code_generator_utils.cc b/compiler/optimizing/code_generator_utils.cc
index 26cab2f..921c1d8 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 @@
* 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 @@
// 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 @@
*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 742d675..59b495c 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 dac7221..007e25a 100644
--- a/compiler/optimizing/code_generator_x86.cc
+++ b/compiler/optimizing/code_generator_x86.cc
@@ -2229,10 +2229,11 @@
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);
@@ -2247,16 +2248,14 @@
}
-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));
@@ -2365,15 +2364,15 @@
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);
@@ -2444,7 +2443,7 @@
// 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;
@@ -2518,7 +2517,7 @@
// 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 a78f564..a5489d2 100644
--- a/compiler/optimizing/code_generator_x86.h
+++ b/compiler/optimizing/code_generator_x86.h
@@ -164,7 +164,7 @@
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 1f24046..2bb0349 100644
--- a/compiler/optimizing/code_generator_x86_64.cc
+++ b/compiler/optimizing/code_generator_x86_64.cc
@@ -2213,12 +2213,7 @@
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);
@@ -2248,25 +2243,18 @@
}
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)));
@@ -2327,7 +2315,7 @@
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();
@@ -2391,7 +2379,7 @@
__ imulq(numerator);
if (imm > 0 && magic < 0) {
- // RDX += numeratorerator
+ // RDX += numerator
__ addq(rdx, numerator);
} else if (imm < 0 && magic > 0) {
// RDX -= numerator
@@ -2441,19 +2429,14 @@
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 03bf0b8..f6fbc2e 100644
--- a/compiler/optimizing/code_generator_x86_64.h
+++ b/compiler/optimizing/code_generator_x86_64.h
@@ -174,7 +174,7 @@
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);