From 861a66fb674389e882e491d04a8c14e18f11a463 Mon Sep 17 00:00:00 2001 From: "whesse@chromium.org" Date: Thu, 4 Nov 2010 15:30:04 +0000 Subject: [PATCH] Fix a potential error in Add() macro-instruction on ARM. Review URL: http://codereview.chromium.org/4247004 git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@5769 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- src/arm/assembler-arm.cc | 17 ++++++++--------- src/arm/assembler-arm.h | 1 + src/arm/macro-assembler-arm.cc | 24 ++++++++++++------------ 3 files changed, 21 insertions(+), 21 deletions(-) diff --git a/src/arm/assembler-arm.cc b/src/arm/assembler-arm.cc index d32547ce69..72835ba3c4 100644 --- a/src/arm/assembler-arm.cc +++ b/src/arm/assembler-arm.cc @@ -749,15 +749,15 @@ static bool fits_shifter(uint32_t imm32, // if they can be encoded in the ARM's 12 bits of immediate-offset instruction // space. There is no guarantee that the relocated location can be similarly // encoded. -static bool MustUseConstantPool(RelocInfo::Mode rmode) { - if (rmode == RelocInfo::EXTERNAL_REFERENCE) { +bool Operand::must_use_constant_pool() const { + if (rmode_ == RelocInfo::EXTERNAL_REFERENCE) { #ifdef DEBUG if (!Serializer::enabled()) { Serializer::TooLateToEnableNow(); } #endif // def DEBUG return Serializer::enabled(); - } else if (rmode == RelocInfo::NONE) { + } else if (rmode_ == RelocInfo::NONE) { return false; } return true; @@ -766,7 +766,7 @@ static bool MustUseConstantPool(RelocInfo::Mode rmode) { bool Operand::is_single_instruction() const { if (rm_.is_valid()) return true; - if (MustUseConstantPool(rmode_)) return false; + if (must_use_constant_pool()) return false; uint32_t dummy1, dummy2; return fits_shifter(imm32_, &dummy1, &dummy2, NULL); } @@ -782,7 +782,7 @@ void Assembler::addrmod1(Instr instr, // Immediate. uint32_t rotate_imm; uint32_t immed_8; - if (MustUseConstantPool(x.rmode_) || + if (x.must_use_constant_pool() || !fits_shifter(x.imm32_, &rotate_imm, &immed_8, &instr)) { // The immediate operand cannot be encoded as a shifter operand, so load // it first to register ip and change the original instruction to use ip. @@ -791,8 +791,7 @@ void Assembler::addrmod1(Instr instr, CHECK(!rn.is(ip)); // rn should never be ip, or will be trashed Condition cond = static_cast(instr & CondMask); if ((instr & ~CondMask) == 13*B21) { // mov, S not set - if (MustUseConstantPool(x.rmode_) || - !CpuFeatures::IsSupported(ARMv7)) { + if (x.must_use_constant_pool() || !CpuFeatures::IsSupported(ARMv7)) { RecordRelocInfo(x.rmode_, x.imm32_); ldr(rd, MemOperand(pc, 0), cond); } else { @@ -803,7 +802,7 @@ void Assembler::addrmod1(Instr instr, } else { // If this is not a mov or mvn instruction we may still be able to avoid // a constant pool entry by using mvn or movw. - if (!MustUseConstantPool(x.rmode_) && + if (!x.must_use_constant_pool() && (instr & kMovMvnMask) != kMovMvnPattern) { mov(ip, x, LeaveCC, cond); } else { @@ -1336,7 +1335,7 @@ void Assembler::msr(SRegisterFieldMask fields, const Operand& src, // Immediate. uint32_t rotate_imm; uint32_t immed_8; - if (MustUseConstantPool(src.rmode_) || + if (src.must_use_constant_pool() || !fits_shifter(src.imm32_, &rotate_imm, &immed_8, NULL)) { // Immediate operand cannot be encoded, load it first to register ip. RecordRelocInfo(src.rmode_, src.imm32_); diff --git a/src/arm/assembler-arm.h b/src/arm/assembler-arm.h index de9dbf3a84..c9ca3550ad 100644 --- a/src/arm/assembler-arm.h +++ b/src/arm/assembler-arm.h @@ -448,6 +448,7 @@ class Operand BASE_EMBEDDED { // Return true of this operand fits in one instruction so that no // 2-instruction solution with a load into the ip register is necessary. bool is_single_instruction() const; + bool must_use_constant_pool() const; inline int32_t immediate() const { ASSERT(!rm_.is_valid()); diff --git a/src/arm/macro-assembler-arm.cc b/src/arm/macro-assembler-arm.cc index c7d7c05f4b..d2c22af53d 100644 --- a/src/arm/macro-assembler-arm.cc +++ b/src/arm/macro-assembler-arm.cc @@ -220,20 +220,20 @@ void MacroAssembler::Move(Register dst, Register src) { void MacroAssembler::And(Register dst, Register src1, const Operand& src2, Condition cond) { - if (!CpuFeatures::IsSupported(ARMv7) || src2.is_single_instruction()) { - and_(dst, src1, src2, LeaveCC, cond); - return; - } - int32_t immediate = src2.immediate(); - if (immediate == 0) { + if (!src2.is_reg() && + !src2.must_use_constant_pool() && + src2.immediate() == 0) { mov(dst, Operand(0, RelocInfo::NONE), LeaveCC, cond); - return; + + } else if (!src2.is_single_instruction() && + !src2.must_use_constant_pool() && + CpuFeatures::IsSupported(ARMv7) && + IsPowerOf2(src2.immediate() + 1)) { + ubfx(dst, src1, 0, WhichPowerOf2(src2.immediate() + 1), cond); + + } else { + and_(dst, src1, src2, LeaveCC, cond); } - if (IsPowerOf2(immediate + 1) && ((immediate & 1) != 0)) { - ubfx(dst, src1, 0, WhichPowerOf2(immediate + 1), cond); - return; - } - and_(dst, src1, src2, LeaveCC, cond); }