From b97df837524de92ef91aa9b1639e1d02101769ff Mon Sep 17 00:00:00 2001 From: thakis Date: Tue, 3 Mar 2015 21:53:05 -0800 Subject: [PATCH] Use Rotate*() functions instead of doing this manually. Shouldn't make a difference in practice, but it's a bit more readable and it gets the case of a 0 shift correct without undefined behavior. BUG=463436 LOG=N Review URL: https://codereview.chromium.org/975283002 Cr-Commit-Position: refs/heads/master@{#26975} --- src/arm/assembler-arm.cc | 5 ++--- src/arm/disasm-arm.cc | 3 ++- src/arm/simulator-arm.cc | 3 ++- src/base/bits.h | 13 +++++++++++++ src/mips/simulator-mips.cc | 4 ++-- src/mips64/simulator-mips64.cc | 7 +++---- src/ppc/simulator-ppc.cc | 22 ++++++++-------------- 7 files changed, 32 insertions(+), 25 deletions(-) diff --git a/src/arm/assembler-arm.cc b/src/arm/assembler-arm.cc index 2bf48e58dc..bbc766bed3 100644 --- a/src/arm/assembler-arm.cc +++ b/src/arm/assembler-arm.cc @@ -1011,8 +1011,7 @@ static bool fits_shifter(uint32_t imm32, Instr* instr) { // imm32 must be unsigned. for (int rot = 0; rot < 16; rot++) { - uint32_t imm8 = - rot == 0 ? imm32 : (imm32 << 2 * rot) | (imm32 >> (32 - 2 * rot)); + uint32_t imm8 = base::bits::RotateLeft32(imm32, 2 * rot); if ((imm8 <= 0xff)) { *rotate_imm = rot; *immed_8 = imm8; @@ -3325,7 +3324,7 @@ Instr Assembler::PatchMovwImmediate(Instr instruction, uint32_t immediate) { int Assembler::DecodeShiftImm(Instr instr) { int rotate = Instruction::RotateValue(instr) * 2; int immed8 = Instruction::Immed8Value(instr); - return (immed8 >> rotate) | (immed8 << (32 - rotate)); + return base::bits::RotateRight32(immed8, rotate); } diff --git a/src/arm/disasm-arm.cc b/src/arm/disasm-arm.cc index 4e631b08a3..19f8c2f8e2 100644 --- a/src/arm/disasm-arm.cc +++ b/src/arm/disasm-arm.cc @@ -33,6 +33,7 @@ #if V8_TARGET_ARCH_ARM #include "src/arm/constants-arm.h" +#include "src/base/bits.h" #include "src/base/platform/platform.h" #include "src/disasm.h" #include "src/macro-assembler.h" @@ -226,7 +227,7 @@ void Decoder::PrintShiftRm(Instruction* instr) { void Decoder::PrintShiftImm(Instruction* instr) { int rotate = instr->RotateValue() * 2; int immed8 = instr->Immed8Value(); - int imm = (immed8 >> rotate) | (immed8 << (32 - rotate)); + int imm = base::bits::RotateRight32(immed8, rotate); out_buffer_pos_ += SNPrintF(out_buffer_ + out_buffer_pos_, "#%d", imm); } diff --git a/src/arm/simulator-arm.cc b/src/arm/simulator-arm.cc index 209b5d2ae8..4c681ae764 100644 --- a/src/arm/simulator-arm.cc +++ b/src/arm/simulator-arm.cc @@ -13,6 +13,7 @@ #include "src/arm/constants-arm.h" #include "src/arm/simulator-arm.h" #include "src/assembler.h" +#include "src/base/bits.h" #include "src/codegen.h" #include "src/disasm.h" @@ -1506,7 +1507,7 @@ int32_t Simulator::GetShiftRm(Instruction* instr, bool* carry_out) { int32_t Simulator::GetImm(Instruction* instr, bool* carry_out) { int rotate = instr->RotateValue() * 2; int immed8 = instr->Immed8Value(); - int imm = (immed8 >> rotate) | (immed8 << (32 - rotate)); + int imm = base::bits::RotateRight32(immed8, rotate); *carry_out = (rotate == 0) ? c_flag_ : (imm < 0); return imm; } diff --git a/src/base/bits.h b/src/base/bits.h index 0f4d4c712b..5c7cd74c61 100644 --- a/src/base/bits.h +++ b/src/base/bits.h @@ -148,17 +148,30 @@ inline uint32_t RoundDownToPowerOfTwo32(uint32_t value) { } +// Precondition: 0 <= shift < 32 inline uint32_t RotateRight32(uint32_t value, uint32_t shift) { if (shift == 0) return value; return (value >> shift) | (value << (32 - shift)); } +// Precondition: 0 <= shift < 32 +inline uint32_t RotateLeft32(uint32_t value, uint32_t shift) { + if (shift == 0) return value; + return (value << shift) | (value >> (32 - shift)); +} +// Precondition: 0 <= shift < 64 inline uint64_t RotateRight64(uint64_t value, uint64_t shift) { if (shift == 0) return value; return (value >> shift) | (value << (64 - shift)); } +// Precondition: 0 <= shift < 64 +inline uint64_t RotateLeft64(uint64_t value, uint64_t shift) { + if (shift == 0) return value; + return (value << shift) | (value >> (64 - shift)); +} + // SignedAddOverflow32(lhs,rhs,val) performs a signed summation of |lhs| and // |rhs| and stores the result into the variable pointed to by |val| and diff --git a/src/mips/simulator-mips.cc b/src/mips/simulator-mips.cc index 79f337d3df..7ea3841935 100644 --- a/src/mips/simulator-mips.cc +++ b/src/mips/simulator-mips.cc @@ -1935,7 +1935,7 @@ void Simulator::ConfigureTypeRegister(Instruction* instr, // Logical right-rotate of a word by a fixed number of bits. This // is special case of SRL instruction, added in MIPS32 Release 2. // RS field is equal to 00001. - *alu_out = (rt_u >> sa) | (rt_u << (32 - sa)); + *alu_out = base::bits::RotateRight32(rt_u, sa); } break; case SRA: @@ -1953,7 +1953,7 @@ void Simulator::ConfigureTypeRegister(Instruction* instr, // Logical right-rotate of a word by a variable number of bits. // This is special case od SRLV instruction, added in MIPS32 // Release 2. SA field is equal to 00001. - *alu_out = (rt_u >> rs_u) | (rt_u << (32 - rs_u)); + *alu_out = base::bits::RotateRight32(rt_u, rs_u); } break; case SRAV: diff --git a/src/mips64/simulator-mips64.cc b/src/mips64/simulator-mips64.cc index bb39b97cca..95b9fce7fc 100644 --- a/src/mips64/simulator-mips64.cc +++ b/src/mips64/simulator-mips64.cc @@ -2012,7 +2012,7 @@ void Simulator::ConfigureTypeRegister(Instruction* instr, // Logical right-rotate of a word by a fixed number of bits. This // is special case of SRL instruction, added in MIPS32 Release 2. // RS field is equal to 00001. - *alu_out = ((uint32_t)rt_u >> sa) | ((uint32_t)rt_u << (32 - sa)); + *alu_out = base::bits::RotateRight32((uint32_t)rt_u, sa); } break; case DSRL: @@ -2045,8 +2045,7 @@ void Simulator::ConfigureTypeRegister(Instruction* instr, // Logical right-rotate of a word by a variable number of bits. // This is special case od SRLV instruction, added in MIPS32 // Release 2. SA field is equal to 00001. - *alu_out = - ((uint32_t)rt_u >> rs_u) | ((uint32_t)rt_u << (32 - rs_u)); + *alu_out = base::bits::RotateRight32(((uint32_t)rt_u, rs_u); } break; case DSRLV: @@ -2058,7 +2057,7 @@ void Simulator::ConfigureTypeRegister(Instruction* instr, // Logical right-rotate of a word by a variable number of bits. // This is special case od SRLV instruction, added in MIPS32 // Release 2. SA field is equal to 00001. - *alu_out = (rt_u >> rs_u) | (rt_u << (32 - rs_u)); + *alu_out = base::bits::RotateRight32(rt_u, rs_u); } break; case SRAV: diff --git a/src/ppc/simulator-ppc.cc b/src/ppc/simulator-ppc.cc index 0bb2da05ff..9b29004f3d 100644 --- a/src/ppc/simulator-ppc.cc +++ b/src/ppc/simulator-ppc.cc @@ -11,6 +11,7 @@ #if V8_TARGET_ARCH_PPC #include "src/assembler.h" +#include "src/base/bits.h" #include "src/codegen.h" #include "src/disasm.h" #include "src/ppc/constants-ppc.h" @@ -2998,8 +2999,7 @@ void Simulator::ExecuteExt5(Instruction* instr) { int mb = (instr->Bits(10, 6) | (instr->Bit(5) << 5)); DCHECK(sh >= 0 && sh <= 63); DCHECK(mb >= 0 && mb <= 63); - // rotate left - uintptr_t result = (rs_val << sh) | (rs_val >> (64 - sh)); + uintptr_t result = base::bits::RotateLeft64(rs_val, sh); uintptr_t mask = 0xffffffffffffffff >> mb; result &= mask; set_register(ra, result); @@ -3016,8 +3016,7 @@ void Simulator::ExecuteExt5(Instruction* instr) { int me = (instr->Bits(10, 6) | (instr->Bit(5) << 5)); DCHECK(sh >= 0 && sh <= 63); DCHECK(me >= 0 && me <= 63); - // rotate left - uintptr_t result = (rs_val << sh) | (rs_val >> (64 - sh)); + uintptr_t result = base::bits::RotateLeft64(rs_val, sh); uintptr_t mask = 0xffffffffffffffff << (63 - me); result &= mask; set_register(ra, result); @@ -3034,8 +3033,7 @@ void Simulator::ExecuteExt5(Instruction* instr) { int mb = (instr->Bits(10, 6) | (instr->Bit(5) << 5)); DCHECK(sh >= 0 && sh <= 63); DCHECK(mb >= 0 && mb <= 63); - // rotate left - uintptr_t result = (rs_val << sh) | (rs_val >> (64 - sh)); + uintptr_t result = base::bits::RotateLeft64(rs_val, sh); uintptr_t mask = (0xffffffffffffffff >> mb) & (0xffffffffffffffff << sh); result &= mask; set_register(ra, result); @@ -3052,8 +3050,7 @@ void Simulator::ExecuteExt5(Instruction* instr) { int sh = (instr->Bits(15, 11) | (instr->Bit(1) << 5)); int mb = (instr->Bits(10, 6) | (instr->Bit(5) << 5)); int me = 63 - sh; - // rotate left - uintptr_t result = (rs_val << sh) | (rs_val >> (64 - sh)); + uintptr_t result = base::bits::RotateLeft64(rs_val, sh); uintptr_t mask = 0; if (mb < me + 1) { uintptr_t bit = 0x8000000000000000 >> mb; @@ -3092,8 +3089,7 @@ void Simulator::ExecuteExt5(Instruction* instr) { int mb = (instr->Bits(10, 6) | (instr->Bit(5) << 5)); DCHECK(sh >= 0 && sh <= 63); DCHECK(mb >= 0 && mb <= 63); - // rotate left - uintptr_t result = (rs_val << sh) | (rs_val >> (64 - sh)); + uintptr_t result = base::bits::RotateLeft64(rs_val, sh); uintptr_t mask = 0xffffffffffffffff >> mb; result &= mask; set_register(ra, result); @@ -3268,8 +3264,7 @@ void Simulator::ExecuteGeneric(Instruction* instr) { int sh = instr->Bits(15, 11); int mb = instr->Bits(10, 6); int me = instr->Bits(5, 1); - // rotate left - uint32_t result = (rs_val << sh) | (rs_val >> (32 - sh)); + uint32_t result = base::bits::RotateLeft32(rs_val, sh); int mask = 0; if (mb < me + 1) { int bit = 0x80000000 >> mb; @@ -3311,8 +3306,7 @@ void Simulator::ExecuteGeneric(Instruction* instr) { } int mb = instr->Bits(10, 6); int me = instr->Bits(5, 1); - // rotate left - uint32_t result = (rs_val << sh) | (rs_val >> (32 - sh)); + uint32_t result = base::bits::RotateLeft32(rs_val, sh); int mask = 0; if (mb < me + 1) { int bit = 0x80000000 >> mb;