Commit 30a29fa2 authored by Pierre Langlois's avatar Pierre Langlois Committed by Commit Bot

[arm] Cleanup addrmod1 encoding and Operand class

This cleanup is the result of trying to modify the `Assembler::addrmod1` method
and realising it's very easy to break it. It handles three groups of
instructions with different operands and uses `r0` when a register is not used:

- General case:            rd, rn, (rm|rm shift #imm|rm shift rs)
- Comparison instructions:     rn, (rm|rm shift #imm|rm shift rs)
- Move instructions        rd,     (rm|rm shift #imm|rm shift rs)

Let's use `no_reg` instead of `r0` with explicit checks and assertions so that
it's clear this method is used with multiple types of instructions.
Additionaly, keep the order of operands as "rd", "rn", "rm".

As drive-by fixes, I've taken the opportunity to add a few helper methods to the
`Operand` class.

Bug: 
Change-Id: If8140d804bc90dea1d3c186b3cee54297f91462a
Reviewed-on: https://chromium-review.googlesource.com/531284
Commit-Queue: Georg Neis <neis@chromium.org>
Reviewed-by: 's avatarJaroslav Sevcik <jarin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#45949}
parent 51a6789b
This diff is collapsed.
...@@ -527,12 +527,20 @@ class Operand BASE_EMBEDDED { ...@@ -527,12 +527,20 @@ class Operand BASE_EMBEDDED {
static Operand EmbeddedNumber(double value); // Smi or HeapNumber static Operand EmbeddedNumber(double value); // Smi or HeapNumber
// Return true if this is a register operand. // Return true if this is a register operand.
INLINE(bool is_reg() const) { bool IsRegister() const {
return rm_.is_valid() && return rm_.is_valid() &&
rs_.is(no_reg) && rs_.is(no_reg) &&
shift_op_ == LSL && shift_op_ == LSL &&
shift_imm_ == 0; shift_imm_ == 0;
} }
// Return true if this is a register operand shifted with an immediate.
bool IsImmediateShiftedRegister() const {
return rm_.is_valid() && !rs_.is_valid();
}
// Return true if this is a register operand shifted with a register.
bool IsRegisterShiftedRegister() const {
return rm_.is_valid() && rs_.is_valid();
}
// Return the number of actual instructions required to implement the given // Return the number of actual instructions required to implement the given
// instruction for this particular operand. This can be a single instruction, // instruction for this particular operand. This can be a single instruction,
...@@ -545,28 +553,31 @@ class Operand BASE_EMBEDDED { ...@@ -545,28 +553,31 @@ class Operand BASE_EMBEDDED {
// //
// The value returned is only valid as long as no entries are added to the // The value returned is only valid as long as no entries are added to the
// constant pool between this call and the actual instruction being emitted. // constant pool between this call and the actual instruction being emitted.
int instructions_required(const Assembler* assembler, Instr instr = 0) const; int InstructionsRequired(const Assembler* assembler, Instr instr = 0) const;
bool must_output_reloc_info(const Assembler* assembler) const; bool MustOutputRelocInfo(const Assembler* assembler) const;
inline int32_t immediate() const { inline int32_t immediate() const {
DCHECK(!rm_.is_valid()); DCHECK(IsImmediate());
DCHECK(!is_heap_number()); DCHECK(!IsHeapNumber());
return value_.immediate; return value_.immediate;
} }
bool IsImmediate() const {
return !rm_.is_valid();
}
double heap_number() const { double heap_number() const {
DCHECK(is_heap_number()); DCHECK(IsHeapNumber());
return value_.heap_number; return value_.heap_number;
} }
bool IsHeapNumber() const {
DCHECK_IMPLIES(is_heap_number_, IsImmediate());
DCHECK_IMPLIES(is_heap_number_, rmode_ == RelocInfo::EMBEDDED_OBJECT);
return is_heap_number_;
}
Register rm() const { return rm_; } Register rm() const { return rm_; }
Register rs() const { return rs_; } Register rs() const { return rs_; }
ShiftOp shift_op() const { return shift_op_; } ShiftOp shift_op() const { return shift_op_; }
bool is_heap_number() const {
DCHECK_IMPLIES(is_heap_number_, !rm_.is_valid());
DCHECK_IMPLIES(is_heap_number_, rmode_ == RelocInfo::EMBEDDED_OBJECT);
return is_heap_number_;
}
private: private:
...@@ -1844,16 +1855,21 @@ class Assembler : public AssemblerBase { ...@@ -1844,16 +1855,21 @@ class Assembler : public AssemblerBase {
void GrowBuffer(); void GrowBuffer();
// 32-bit immediate values // 32-bit immediate values
void move_32_bit_immediate(Register rd, void Move32BitImmediate(Register rd, const Operand& x, Condition cond = al);
const Operand& x,
Condition cond = al);
// Instruction generation // Instruction generation
void addrmod1(Instr instr, Register rn, Register rd, const Operand& x); void AddrMode1(Instr instr, Register rd, Register rn, const Operand& x);
void addrmod2(Instr instr, Register rd, const MemOperand& x); // Attempt to encode operand |x| for instruction |instr| and return true on
void addrmod3(Instr instr, Register rd, const MemOperand& x); // success. The result will be encoded in |instr| directly. This method may
void addrmod4(Instr instr, Register rn, RegList rl); // change the opcode if deemed beneficial, for instance, MOV may be turned
void addrmod5(Instr instr, CRegister crd, const MemOperand& x); // into MVN, ADD into SUB, AND into BIC, ...etc. The only reason this method
// may fail is that the operand is an immediate that cannot be encoded.
bool AddrMode1TryEncodeOperand(Instr* instr, const Operand& x);
void AddrMode2(Instr instr, Register rd, const MemOperand& x);
void AddrMode3(Instr instr, Register rd, const MemOperand& x);
void AddrMode4(Instr instr, Register rn, RegList rl);
void AddrMode5(Instr instr, CRegister crd, const MemOperand& x);
// Labels // Labels
void print(Label* L); void print(Label* L);
......
...@@ -88,7 +88,7 @@ int MacroAssembler::CallSize( ...@@ -88,7 +88,7 @@ int MacroAssembler::CallSize(
Instr mov_instr = cond | MOV | LeaveCC; Instr mov_instr = cond | MOV | LeaveCC;
Operand mov_operand = Operand(reinterpret_cast<intptr_t>(target), rmode); Operand mov_operand = Operand(reinterpret_cast<intptr_t>(target), rmode);
return kInstrSize + return kInstrSize +
mov_operand.instructions_required(this, mov_instr) * kInstrSize; mov_operand.InstructionsRequired(this, mov_instr) * kInstrSize;
} }
...@@ -318,12 +318,11 @@ void MacroAssembler::Mls(Register dst, Register src1, Register src2, ...@@ -318,12 +318,11 @@ void MacroAssembler::Mls(Register dst, Register src1, Register src2,
void MacroAssembler::And(Register dst, Register src1, const Operand& src2, void MacroAssembler::And(Register dst, Register src1, const Operand& src2,
Condition cond) { Condition cond) {
if (!src2.is_reg() && if (!src2.IsRegister() && !src2.MustOutputRelocInfo(this) &&
!src2.must_output_reloc_info(this) &&
src2.immediate() == 0) { src2.immediate() == 0) {
mov(dst, Operand::Zero(), LeaveCC, cond); mov(dst, Operand::Zero(), LeaveCC, cond);
} else if (!(src2.instructions_required(this) == 1) && } else if (!(src2.InstructionsRequired(this) == 1) &&
!src2.must_output_reloc_info(this) && !src2.MustOutputRelocInfo(this) &&
CpuFeatures::IsSupported(ARMv7) && CpuFeatures::IsSupported(ARMv7) &&
base::bits::IsPowerOfTwo32(src2.immediate() + 1)) { base::bits::IsPowerOfTwo32(src2.immediate() + 1)) {
CpuFeatureScope scope(this, ARMv7); CpuFeatureScope scope(this, ARMv7);
...@@ -1992,7 +1991,7 @@ void MacroAssembler::Allocate(int object_size, ...@@ -1992,7 +1991,7 @@ void MacroAssembler::Allocate(int object_size,
object_size -= bits; object_size -= bits;
shift += 8; shift += 8;
Operand bits_operand(bits); Operand bits_operand(bits);
DCHECK(bits_operand.instructions_required(this) == 1); DCHECK(bits_operand.InstructionsRequired(this) == 1);
add(result_end, source, bits_operand); add(result_end, source, bits_operand);
source = result_end; source = result_end;
} }
...@@ -2202,7 +2201,7 @@ void MacroAssembler::FastAllocate(int object_size, Register result, ...@@ -2202,7 +2201,7 @@ void MacroAssembler::FastAllocate(int object_size, Register result,
object_size -= bits; object_size -= bits;
shift += 8; shift += 8;
Operand bits_operand(bits); Operand bits_operand(bits);
DCHECK(bits_operand.instructions_required(this) == 1); DCHECK(bits_operand.InstructionsRequired(this) == 1);
add(result_end, source, bits_operand); add(result_end, source, bits_operand);
source = result_end; source = result_end;
} }
......
...@@ -182,7 +182,7 @@ class MacroAssembler: public Assembler { ...@@ -182,7 +182,7 @@ class MacroAssembler: public Assembler {
void Move(Register dst, Register src, Condition cond = al); void Move(Register dst, Register src, Condition cond = al);
void Move(Register dst, const Operand& src, SBit sbit = LeaveCC, void Move(Register dst, const Operand& src, SBit sbit = LeaveCC,
Condition cond = al) { Condition cond = al) {
if (!src.is_reg() || !src.rm().is(dst) || sbit != LeaveCC) { if (!src.IsRegister() || !src.rm().is(dst) || sbit != LeaveCC) {
mov(dst, src, sbit, cond); mov(dst, src, sbit, cond);
} }
} }
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment