MIPS: HRor clean-up
This is a follow up to
https://android-review.googlesource.com/#/c/194590/.
Change-Id: Ia37faa02736e5dd54c1e71fd2a4d94e074746757
diff --git a/compiler/optimizing/code_generator_mips.cc b/compiler/optimizing/code_generator_mips.cc
index 75f5fb3..fdca0b3 100644
--- a/compiler/optimizing/code_generator_mips.cc
+++ b/compiler/optimizing/code_generator_mips.cc
@@ -1539,8 +1539,10 @@
bool use_imm = rhs_location.IsConstant();
Register rhs_reg = use_imm ? ZERO : rhs_location.AsRegister<Register>();
int64_t rhs_imm = use_imm ? CodeGenerator::GetInt64ValueOf(rhs_location.GetConstant()) : 0;
- uint32_t shift_mask = (type == Primitive::kPrimInt) ? kMaxIntShiftValue : kMaxLongShiftValue;
- uint32_t shift_value = rhs_imm & shift_mask;
+ const uint32_t shift_mask = (type == Primitive::kPrimInt)
+ ? kMaxIntShiftValue
+ : kMaxLongShiftValue;
+ const uint32_t shift_value = rhs_imm & shift_mask;
// Are the INS (Insert Bit Field) and ROTR instructions supported?
bool has_ins_rotr = codegen_->GetInstructionSetFeatures().IsMipsIsaRevGreaterThanEqual2();
@@ -1580,6 +1582,11 @@
__ Rotrv(dst, lhs, rhs_reg);
} else {
__ Subu(TMP, ZERO, rhs_reg);
+ // 32-bit shift instructions use the 5 least significant bits of the shift count, so
+ // shifting by `-rhs_reg` is equivalent to shifting by `(32 - rhs_reg) & 31`. The case
+ // when `rhs_reg & 31 == 0` is OK even though we don't shift `lhs` left all the way out
+ // by 32, because the result in this case is computed as `(lhs >> 0) | (lhs << 0)`,
+ // IOW, the OR'd values are equal.
__ Sllv(TMP, lhs, TMP);
__ Srlv(dst, lhs, rhs_reg);
__ Or(dst, dst, TMP);
@@ -1643,33 +1650,33 @@
}
}
} else {
- shift_value -= kMipsBitsPerWord;
+ const uint32_t shift_value_high = shift_value - kMipsBitsPerWord;
if (instr->IsShl()) {
- __ Sll(dst_high, lhs_low, shift_value);
+ __ Sll(dst_high, lhs_low, shift_value_high);
__ Move(dst_low, ZERO);
} else if (instr->IsShr()) {
- __ Sra(dst_low, lhs_high, shift_value);
+ __ Sra(dst_low, lhs_high, shift_value_high);
__ Sra(dst_high, dst_low, kMipsBitsPerWord - 1);
} else if (instr->IsUShr()) {
- __ Srl(dst_low, lhs_high, shift_value);
+ __ Srl(dst_low, lhs_high, shift_value_high);
__ Move(dst_high, ZERO);
} else {
- if (shift_value == 0) {
+ if (shift_value == kMipsBitsPerWord) {
// 64-bit rotation by 32 is just a swap.
__ Move(dst_low, lhs_high);
__ Move(dst_high, lhs_low);
} else {
if (has_ins_rotr) {
- __ Srl(dst_low, lhs_high, shift_value);
- __ Ins(dst_low, lhs_low, kMipsBitsPerWord - shift_value, shift_value);
- __ Srl(dst_high, lhs_low, shift_value);
- __ Ins(dst_high, lhs_high, kMipsBitsPerWord - shift_value, shift_value);
+ __ Srl(dst_low, lhs_high, shift_value_high);
+ __ Ins(dst_low, lhs_low, kMipsBitsPerWord - shift_value_high, shift_value_high);
+ __ Srl(dst_high, lhs_low, shift_value_high);
+ __ Ins(dst_high, lhs_high, kMipsBitsPerWord - shift_value_high, shift_value_high);
} else {
- __ Sll(TMP, lhs_low, kMipsBitsPerWord - shift_value);
- __ Srl(dst_low, lhs_high, shift_value);
+ __ Sll(TMP, lhs_low, kMipsBitsPerWord - shift_value_high);
+ __ Srl(dst_low, lhs_high, shift_value_high);
__ Or(dst_low, dst_low, TMP);
- __ Sll(TMP, lhs_high, kMipsBitsPerWord - shift_value);
- __ Srl(dst_high, lhs_low, shift_value);
+ __ Sll(TMP, lhs_high, kMipsBitsPerWord - shift_value_high);
+ __ Srl(dst_high, lhs_low, shift_value_high);
__ Or(dst_high, dst_high, TMP);
}
}