Commit ecb77978 authored by Clemens Hammacher's avatar Clemens Hammacher Committed by Commit Bot

Reland "[Assembler][x64] Make Operand immutable"

This is a reland of e7f9fb4a.

Original change's description:
> [Assembler][x64] Make Operand immutable
> 
> This CL removes all setters from the Operand and removes the friendship
> relation between Assembler and Operand. All data fields of the Operand
> are set exactly once in the constructor, the Operand is immutable
> afterwards.
> In order to construct the data of an Operand easily, the OperandBuilder
> is introduced. After building an Operand, the data is copied to the
> const field of the Operand.
> 
> R=mstarzinger@chromium.org
> 
> Bug: v8:7310
> Change-Id: I1628052b8a0c47cbfbc3645dfdac5a0e9705977b
> Reviewed-on: https://chromium-review.googlesource.com/936741
> Reviewed-by: Michael Starzinger <mstarzinger@chromium.org>
> Commit-Queue: Clemens Hammacher <clemensh@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#51563}

Bug: v8:7310
Change-Id: I84df5e11b1811585fbba7309e3bb9c6b17e18c0b
Reviewed-on: https://chromium-review.googlesource.com/936628Reviewed-by: 's avatarMichael Starzinger <mstarzinger@chromium.org>
Commit-Queue: Clemens Hammacher <clemensh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#51573}
parent 0ad8033b
......@@ -3382,11 +3382,9 @@ void CodeGenerator::AssembleSwap(InstructionOperand* source,
frame_access_state()->IncreaseSPDelta(1);
unwinding_info_writer_.MaybeIncreaseBaseOffsetAt(__ pc_offset(),
kPointerSize);
Operand dst = g.ToOperand(destination);
__ movq(src, dst);
__ movq(src, g.ToOperand(destination));
frame_access_state()->IncreaseSPDelta(-1);
dst = g.ToOperand(destination);
__ popq(dst);
__ popq(g.ToOperand(destination));
unwinding_info_writer_.MaybeIncreaseBaseOffsetAt(__ pc_offset(),
-kPointerSize);
} else {
......
......@@ -39,6 +39,21 @@ inline Operand GetContextOperand() { return Operand(rbp, -16); }
// stack for a call to C.
static constexpr Register kCCallLastArgAddrReg = rax;
inline Operand GetMemOp(LiftoffAssembler* assm, Register addr, Register offset,
uint32_t offset_imm, LiftoffRegList pinned) {
if (offset_imm > kMaxInt) {
// The immediate can not be encoded in the operand. Load it to a register
// first.
Register total_offset = assm->GetUnusedRegister(kGpReg, pinned).gp();
assm->movl(total_offset, Immediate(offset_imm));
if (offset != no_reg) {
assm->emit_ptrsize_add(total_offset, addr, offset);
}
return Operand(addr, total_offset, times_1, 0);
}
if (offset == no_reg) return Operand(addr, offset_imm);
return Operand(addr, offset, times_1, offset_imm);
}
} // namespace liftoff
uint32_t LiftoffAssembler::PrepareStackFrame() {
......@@ -110,19 +125,8 @@ void LiftoffAssembler::Load(LiftoffRegister dst, Register src_addr,
Register offset_reg, uint32_t offset_imm,
LoadType type, LiftoffRegList pinned,
uint32_t* protected_load_pc) {
Operand src_op = offset_reg == no_reg
? Operand(src_addr, offset_imm)
: Operand(src_addr, offset_reg, times_1, offset_imm);
if (offset_imm > kMaxInt) {
// The immediate can not be encoded in the operand. Load it to a register
// first.
Register src = GetUnusedRegister(kGpReg, pinned).gp();
movl(src, Immediate(offset_imm));
if (offset_reg != no_reg) {
emit_ptrsize_add(src, src, offset_reg);
}
src_op = Operand(src_addr, src, times_1, 0);
}
Operand src_op =
liftoff::GetMemOp(this, src_addr, offset_reg, offset_imm, pinned);
if (protected_load_pc) *protected_load_pc = pc_offset();
switch (type.value()) {
case LoadType::kI32Load8U:
......@@ -170,19 +174,8 @@ void LiftoffAssembler::Store(Register dst_addr, Register offset_reg,
uint32_t offset_imm, LiftoffRegister src,
StoreType type, LiftoffRegList pinned,
uint32_t* protected_store_pc) {
Operand dst_op = offset_reg == no_reg
? Operand(dst_addr, offset_imm)
: Operand(dst_addr, offset_reg, times_1, offset_imm);
if (offset_imm > kMaxInt) {
// The immediate can not be encoded in the operand. Load it to a register
// first.
Register dst = GetUnusedRegister(kGpReg, pinned).gp();
movl(dst, Immediate(offset_imm));
if (offset_reg != no_reg) {
emit_ptrsize_add(dst, dst, offset_reg);
}
dst_op = Operand(dst_addr, dst, times_1, 0);
}
Operand dst_op =
liftoff::GetMemOp(this, dst_addr, offset_reg, offset_imm, pinned);
if (protected_store_pc) *protected_store_pc = pc_offset();
switch (type.value()) {
case StoreType::kI32Store8:
......
......@@ -93,11 +93,11 @@ void Assembler::emit_rex_64(Register reg, XMMRegister rm_reg) {
}
void Assembler::emit_rex_64(Register reg, Operand op) {
emit(0x48 | reg.high_bit() << 2 | op.rex_);
emit(0x48 | reg.high_bit() << 2 | op.data().rex);
}
void Assembler::emit_rex_64(XMMRegister reg, Operand op) {
emit(0x48 | (reg.code() & 0x8) >> 1 | op.rex_);
emit(0x48 | (reg.code() & 0x8) >> 1 | op.data().rex);
}
......@@ -106,14 +106,14 @@ void Assembler::emit_rex_64(Register rm_reg) {
emit(0x48 | rm_reg.high_bit());
}
void Assembler::emit_rex_64(Operand op) { emit(0x48 | op.rex_); }
void Assembler::emit_rex_64(Operand op) { emit(0x48 | op.data().rex); }
void Assembler::emit_rex_32(Register reg, Register rm_reg) {
emit(0x40 | reg.high_bit() << 2 | rm_reg.high_bit());
}
void Assembler::emit_rex_32(Register reg, Operand op) {
emit(0x40 | reg.high_bit() << 2 | op.rex_);
emit(0x40 | reg.high_bit() << 2 | op.data().rex);
}
......@@ -121,7 +121,7 @@ void Assembler::emit_rex_32(Register rm_reg) {
emit(0x40 | rm_reg.high_bit());
}
void Assembler::emit_rex_32(Operand op) { emit(0x40 | op.rex_); }
void Assembler::emit_rex_32(Operand op) { emit(0x40 | op.data().rex); }
void Assembler::emit_optional_rex_32(Register reg, Register rm_reg) {
byte rex_bits = reg.high_bit() << 2 | rm_reg.high_bit();
......@@ -129,12 +129,12 @@ void Assembler::emit_optional_rex_32(Register reg, Register rm_reg) {
}
void Assembler::emit_optional_rex_32(Register reg, Operand op) {
byte rex_bits = reg.high_bit() << 2 | op.rex_;
byte rex_bits = reg.high_bit() << 2 | op.data().rex;
if (rex_bits != 0) emit(0x40 | rex_bits);
}
void Assembler::emit_optional_rex_32(XMMRegister reg, Operand op) {
byte rex_bits = (reg.code() & 0x8) >> 1 | op.rex_;
byte rex_bits = (reg.code() & 0x8) >> 1 | op.data().rex;
if (rex_bits != 0) emit(0x40 | rex_bits);
}
......@@ -166,7 +166,7 @@ void Assembler::emit_optional_rex_32(XMMRegister rm_reg) {
}
void Assembler::emit_optional_rex_32(Operand op) {
if (op.rex_ != 0) emit(0x40 | op.rex_);
if (op.data().rex != 0) emit(0x40 | op.data().rex);
}
......@@ -180,7 +180,7 @@ void Assembler::emit_vex3_byte1(XMMRegister reg, XMMRegister rm,
// byte 1 of 3-byte VEX
void Assembler::emit_vex3_byte1(XMMRegister reg, Operand rm, LeadingOpcode m) {
byte rxb = ~((reg.high_bit() << 2) | rm.rex_) << 5;
byte rxb = ~((reg.high_bit() << 2) | rm.data().rex) << 5;
emit(rxb | m);
}
......@@ -226,7 +226,7 @@ void Assembler::emit_vex_prefix(Register reg, Register vreg, Register rm,
void Assembler::emit_vex_prefix(XMMRegister reg, XMMRegister vreg, Operand rm,
VectorLength l, SIMDPrefix pp, LeadingOpcode mm,
VexW w) {
if (rm.rex_ || mm != k0F || w != kW0) {
if (rm.data().rex || mm != k0F || w != kW0) {
emit_vex3_byte0();
emit_vex3_byte1(reg, rm, mm);
emit_vex3_byte2(w, vreg, l, pp);
......@@ -413,49 +413,6 @@ void RelocInfo::Visit(ObjectVisitor* visitor) {
}
}
// -----------------------------------------------------------------------------
// Implementation of Operand
void Operand::set_modrm(int mod, Register rm_reg) {
DCHECK(is_uint2(mod));
buf_[0] = mod << 6 | rm_reg.low_bits();
// Set REX.B to the high bit of rm.code().
rex_ |= rm_reg.high_bit();
}
void Operand::set_sib(ScaleFactor scale, Register index, Register base) {
DCHECK_EQ(len_, 1);
DCHECK(is_uint2(scale));
// Use SIB with no index register only for base rsp or r12. Otherwise we
// would skip the SIB byte entirely.
DCHECK(index != rsp || base == rsp || base == r12);
buf_[1] = (scale << 6) | (index.low_bits() << 3) | base.low_bits();
rex_ |= index.high_bit() << 1 | base.high_bit();
len_ = 2;
}
void Operand::set_disp8(int disp) {
DCHECK(is_int8(disp));
DCHECK(len_ == 1 || len_ == 2);
int8_t* p = reinterpret_cast<int8_t*>(&buf_[len_]);
*p = disp;
len_ += sizeof(int8_t);
}
void Operand::set_disp32(int disp) {
DCHECK(len_ == 1 || len_ == 2);
int32_t* p = reinterpret_cast<int32_t*>(&buf_[len_]);
*p = disp;
len_ += sizeof(int32_t);
}
void Operand::set_disp64(int64_t disp) {
DCHECK_EQ(1, len_);
int64_t* p = reinterpret_cast<int64_t*>(&buf_[len_]);
*p = disp;
len_ += sizeof(disp);
}
} // namespace internal
} // namespace v8
......
This diff is collapsed.
......@@ -333,6 +333,13 @@ enum ScaleFactor : int8_t {
class Operand {
public:
struct Data {
byte rex = 0;
byte buf[9];
byte len = 1; // number of bytes of buf_ in use.
int8_t addend; // for rip + offset + addend.
};
// [base + disp/r]
Operand(Register base, int32_t disp);
......@@ -355,46 +362,37 @@ class Operand {
// [rip + disp/r]
explicit Operand(Label* label, int addend = 0);
Operand(const Operand&) = default;
// Checks whether either base or index register is the given register.
// Does not check the "reg" part of the Operand.
bool AddressUsesRegister(Register reg) const;
// Queries related to the size of the generated instruction.
// Whether the generated instruction will have a REX prefix.
bool requires_rex() const { return rex_ != 0; }
bool requires_rex() const { return data_.rex != 0; }
// Size of the ModR/M, SIB and displacement parts of the generated
// instruction.
int operand_size() const { return len_; }
private:
byte rex_;
byte buf_[9];
// The number of bytes of buf_ in use.
byte len_;
int8_t addend_; // for rip + offset + addend
// Set the ModR/M byte without an encoded 'reg' register. The
// register is encoded later as part of the emit_operand operation.
// set_modrm can be called before or after set_sib and set_disp*.
inline void set_modrm(int mod, Register rm);
int operand_size() const { return data_.len; }
// Set the SIB byte if one is needed. Sets the length to 2 rather than 1.
inline void set_sib(ScaleFactor scale, Register index, Register base);
const Data& data() const { return data_; }
// Adds operand displacement fields (offsets added to the memory address).
// Needs to be called after set_sib, not before it.
inline void set_disp8(int disp);
inline void set_disp32(int disp);
inline void set_disp64(int64_t disp); // for labels.
// TODO(clemensh): Get rid of this friendship, or make Operand immutable.
friend class Assembler;
private:
const Data data_;
};
static_assert(sizeof(Operand) <= 2 * kPointerSize,
"Operand must be small enough to pass it by value");
// Unfortunately, MSVC 2015 is broken in that both is_trivially_destructible and
// is_trivially_copy_constructible are true, but is_trivially_copyable is false.
// (status at 2018-02-26, observed on the msvc waterfall bot).
#if V8_CC_MSVC
static_assert(std::is_trivially_copy_constructible<Operand>::value &&
std::is_trivially_destructible<Operand>::value,
"Operand must be trivially copyable to pass it by value");
#else
static_assert(IS_TRIVIALLY_COPYABLE(Operand),
"Operand must be trivially copyable to pass it by value");
#endif
#define ASSEMBLER_INSTRUCTION_LIST(V) \
V(add) \
......
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