From ecb77978c70a9db7052e7a3d4dc77f92efe3910f Mon Sep 17 00:00:00 2001 From: Clemens Hammacher Date: Mon, 26 Feb 2018 15:24:38 +0100 Subject: [PATCH] Reland "[Assembler][x64] Make Operand immutable" This is a reland of e7f9fb4a0db1d6eba8d5825e82ec4f435593d536. 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 > Commit-Queue: Clemens Hammacher > Cr-Commit-Position: refs/heads/master@{#51563} Bug: v8:7310 Change-Id: I84df5e11b1811585fbba7309e3bb9c6b17e18c0b Reviewed-on: https://chromium-review.googlesource.com/936628 Reviewed-by: Michael Starzinger Commit-Queue: Clemens Hammacher Cr-Commit-Position: refs/heads/master@{#51573} --- src/compiler/x64/code-generator-x64.cc | 6 +- src/wasm/baseline/x64/liftoff-assembler-x64.h | 45 ++- src/x64/assembler-x64-inl.h | 63 +--- src/x64/assembler-x64.cc | 274 +++++++++++------- src/x64/assembler-x64.h | 48 ++- 5 files changed, 220 insertions(+), 216 deletions(-) diff --git a/src/compiler/x64/code-generator-x64.cc b/src/compiler/x64/code-generator-x64.cc index 12e84896e6..cc6d758a9a 100644 --- a/src/compiler/x64/code-generator-x64.cc +++ b/src/compiler/x64/code-generator-x64.cc @@ -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 { diff --git a/src/wasm/baseline/x64/liftoff-assembler-x64.h b/src/wasm/baseline/x64/liftoff-assembler-x64.h index aadfebd327..05690cfb22 100644 --- a/src/wasm/baseline/x64/liftoff-assembler-x64.h +++ b/src/wasm/baseline/x64/liftoff-assembler-x64.h @@ -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: diff --git a/src/x64/assembler-x64-inl.h b/src/x64/assembler-x64-inl.h index 849117a07b..eef4158f53 100644 --- a/src/x64/assembler-x64-inl.h +++ b/src/x64/assembler-x64-inl.h @@ -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(&buf_[len_]); - *p = disp; - len_ += sizeof(int8_t); -} - -void Operand::set_disp32(int disp) { - DCHECK(len_ == 1 || len_ == 2); - int32_t* p = reinterpret_cast(&buf_[len_]); - *p = disp; - len_ += sizeof(int32_t); -} - -void Operand::set_disp64(int64_t disp) { - DCHECK_EQ(1, len_); - int64_t* p = reinterpret_cast(&buf_[len_]); - *p = disp; - len_ += sizeof(disp); -} } // namespace internal } // namespace v8 diff --git a/src/x64/assembler-x64.cc b/src/x64/assembler-x64.cc index 2ddd87e998..0ec50147fd 100644 --- a/src/x64/assembler-x64.cc +++ b/src/x64/assembler-x64.cc @@ -157,132 +157,189 @@ Address RelocInfo::js_to_wasm_address() const { // ----------------------------------------------------------------------------- // Implementation of Operand -Operand::Operand(Register base, int32_t disp) : rex_(0) { - len_ = 1; - if (base == rsp || base == r12) { - // SIB byte is needed to encode (rsp + offset) or (r12 + offset). - set_sib(times_1, rsp, base); +namespace { +class OperandBuilder { + public: + OperandBuilder(Register base, int32_t disp) { + if (base == rsp || base == r12) { + // SIB byte is needed to encode (rsp + offset) or (r12 + offset). + set_sib(times_1, rsp, base); + } + + if (disp == 0 && base != rbp && base != r13) { + set_modrm(0, base); + } else if (is_int8(disp)) { + set_modrm(1, base); + set_disp8(disp); + } else { + set_modrm(2, base); + set_disp32(disp); + } } - if (disp == 0 && base != rbp && base != r13) { - set_modrm(0, base); - } else if (is_int8(disp)) { - set_modrm(1, base); - set_disp8(disp); - } else { - set_modrm(2, base); - set_disp32(disp); + OperandBuilder(Register base, Register index, ScaleFactor scale, + int32_t disp) { + DCHECK(index != rsp); + set_sib(scale, index, base); + if (disp == 0 && base != rbp && base != r13) { + // This call to set_modrm doesn't overwrite the REX.B (or REX.X) bits + // possibly set by set_sib. + set_modrm(0, rsp); + } else if (is_int8(disp)) { + set_modrm(1, rsp); + set_disp8(disp); + } else { + set_modrm(2, rsp); + set_disp32(disp); + } } -} - -Operand::Operand(Register base, - Register index, - ScaleFactor scale, - int32_t disp) : rex_(0) { - DCHECK(index != rsp); - len_ = 1; - set_sib(scale, index, base); - if (disp == 0 && base != rbp && base != r13) { - // This call to set_modrm doesn't overwrite the REX.B (or REX.X) bits - // possibly set by set_sib. + OperandBuilder(Register index, ScaleFactor scale, int32_t disp) { + DCHECK(index != rsp); set_modrm(0, rsp); - } else if (is_int8(disp)) { - set_modrm(1, rsp); - set_disp8(disp); - } else { - set_modrm(2, rsp); + set_sib(scale, index, rbp); set_disp32(disp); } -} - -Operand::Operand(Register index, - ScaleFactor scale, - int32_t disp) : rex_(0) { - DCHECK(index != rsp); - len_ = 1; - set_modrm(0, rsp); - set_sib(scale, index, rbp); - set_disp32(disp); -} - -Operand::Operand(Label* label, int addend) : rex_(0), len_(1), addend_(addend) { - DCHECK_NOT_NULL(label); - DCHECK(addend == 0 || (is_int8(addend) && label->is_bound())); - set_modrm(0, rbp); - set_disp64(reinterpret_cast(label)); -} - -Operand::Operand(Operand operand, int32_t offset) { - DCHECK_GE(operand.len_, 1); - // Operand encodes REX ModR/M [SIB] [Disp]. - byte modrm = operand.buf_[0]; - DCHECK_LT(modrm, 0xC0); // Disallow mode 3 (register target). - bool has_sib = ((modrm & 0x07) == 0x04); - byte mode = modrm & 0xC0; - int disp_offset = has_sib ? 2 : 1; - int base_reg = (has_sib ? operand.buf_[1] : modrm) & 0x07; - // Mode 0 with rbp/r13 as ModR/M or SIB base register always has a 32-bit - // displacement. - bool is_baseless = (mode == 0) && (base_reg == 0x05); // No base or RIP base. - int32_t disp_value = 0; - if (mode == 0x80 || is_baseless) { - // Mode 2 or mode 0 with rbp/r13 as base: Word displacement. - disp_value = *bit_cast(&operand.buf_[disp_offset]); - } else if (mode == 0x40) { - // Mode 1: Byte displacement. - disp_value = static_cast(operand.buf_[disp_offset]); + OperandBuilder(Label* label, int addend) { + data_.addend = addend; + DCHECK_NOT_NULL(label); + DCHECK(addend == 0 || (is_int8(addend) && label->is_bound())); + set_modrm(0, rbp); + set_disp64(reinterpret_cast(label)); } - // Write new operand with same registers, but with modified displacement. - DCHECK(offset >= 0 ? disp_value + offset > disp_value - : disp_value + offset < disp_value); // No overflow. - disp_value += offset; - rex_ = operand.rex_; - if (!is_int8(disp_value) || is_baseless) { - // Need 32 bits of displacement, mode 2 or mode 1 with register rbp/r13. - buf_[0] = (modrm & 0x3F) | (is_baseless ? 0x00 : 0x80); - len_ = disp_offset + 4; - Memory::int32_at(&buf_[disp_offset]) = disp_value; - } else if (disp_value != 0 || (base_reg == 0x05)) { - // Need 8 bits of displacement. - buf_[0] = (modrm & 0x3F) | 0x40; // Mode 1. - len_ = disp_offset + 1; - buf_[disp_offset] = static_cast(disp_value); - } else { - // Need no displacement. - buf_[0] = (modrm & 0x3F); // Mode 0. - len_ = disp_offset; - } - if (has_sib) { - buf_[1] = operand.buf_[1]; - } -} + OperandBuilder(Operand operand, int32_t offset) { + DCHECK_GE(operand.data().len, 1); + // Operand encodes REX ModR/M [SIB] [Disp]. + byte modrm = operand.data().buf[0]; + DCHECK_LT(modrm, 0xC0); // Disallow mode 3 (register target). + bool has_sib = ((modrm & 0x07) == 0x04); + byte mode = modrm & 0xC0; + int disp_offset = has_sib ? 2 : 1; + int base_reg = (has_sib ? operand.data().buf[1] : modrm) & 0x07; + // Mode 0 with rbp/r13 as ModR/M or SIB base register always has a 32-bit + // displacement. + bool is_baseless = + (mode == 0) && (base_reg == 0x05); // No base or RIP base. + int32_t disp_value = 0; + if (mode == 0x80 || is_baseless) { + // Mode 2 or mode 0 with rbp/r13 as base: Word displacement. + disp_value = *bit_cast(&operand.data().buf[disp_offset]); + } else if (mode == 0x40) { + // Mode 1: Byte displacement. + disp_value = static_cast(operand.data().buf[disp_offset]); + } + // Write new operand with same registers, but with modified displacement. + DCHECK(offset >= 0 ? disp_value + offset > disp_value + : disp_value + offset < disp_value); // No overflow. + disp_value += offset; + data_.rex = operand.data().rex; + if (!is_int8(disp_value) || is_baseless) { + // Need 32 bits of displacement, mode 2 or mode 1 with register rbp/r13. + data_.buf[0] = (modrm & 0x3F) | (is_baseless ? 0x00 : 0x80); + data_.len = disp_offset + 4; + Memory::int32_at(&data_.buf[disp_offset]) = disp_value; + } else if (disp_value != 0 || (base_reg == 0x05)) { + // Need 8 bits of displacement. + data_.buf[0] = (modrm & 0x3F) | 0x40; // Mode 1. + data_.len = disp_offset + 1; + data_.buf[disp_offset] = static_cast(disp_value); + } else { + // Need no displacement. + data_.buf[0] = (modrm & 0x3F); // Mode 0. + data_.len = disp_offset; + } + if (has_sib) { + data_.buf[1] = operand.data().buf[1]; + } + } + + void set_modrm(int mod, Register rm_reg) { + DCHECK(is_uint2(mod)); + data_.buf[0] = mod << 6 | rm_reg.low_bits(); + // Set REX.B to the high bit of rm.code(). + data_.rex |= rm_reg.high_bit(); + } + + void set_sib(ScaleFactor scale, Register index, Register base) { + DCHECK_EQ(data_.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); + data_.buf[1] = (scale << 6) | (index.low_bits() << 3) | base.low_bits(); + data_.rex |= index.high_bit() << 1 | base.high_bit(); + data_.len = 2; + } + + void set_disp8(int disp) { + DCHECK(is_int8(disp)); + DCHECK(data_.len == 1 || data_.len == 2); + int8_t* p = reinterpret_cast(&data_.buf[data_.len]); + *p = disp; + data_.len += sizeof(int8_t); + } + + void set_disp32(int disp) { + DCHECK(data_.len == 1 || data_.len == 2); + int32_t* p = reinterpret_cast(&data_.buf[data_.len]); + *p = disp; + data_.len += sizeof(int32_t); + } + + void set_disp64(int64_t disp) { + DCHECK_EQ(1, data_.len); + int64_t* p = reinterpret_cast(&data_.buf[data_.len]); + *p = disp; + data_.len += sizeof(disp); + } + + const Operand::Data& data() const { return data_; } + + private: + Operand::Data data_; +}; +} // namespace + +Operand::Operand(Register base, int32_t disp) + : data_(OperandBuilder(base, disp).data()) {} + +Operand::Operand(Register base, Register index, ScaleFactor scale, int32_t disp) + : data_(OperandBuilder(base, index, scale, disp).data()) {} + +Operand::Operand(Register index, ScaleFactor scale, int32_t disp) + : data_(OperandBuilder(index, scale, disp).data()) {} + +Operand::Operand(Label* label, int addend) + : data_(OperandBuilder(label, addend).data()) {} + +Operand::Operand(Operand operand, int32_t offset) + : data_(OperandBuilder(operand, offset).data()) {} bool Operand::AddressUsesRegister(Register reg) const { int code = reg.code(); - DCHECK_NE(buf_[0] & 0xC0, 0xC0); // Always a memory operand. - // Start with only low three bits of base register. Initial decoding doesn't - // distinguish on the REX.B bit. - int base_code = buf_[0] & 0x07; + DCHECK_NE(data_.buf[0] & 0xC0, 0xC0); // Always a memory operand. + // Start with only low three bits of base register. Initial decoding + // doesn't distinguish on the REX.B bit. + int base_code = data_.buf[0] & 0x07; if (base_code == rsp.code()) { // SIB byte present in buf_[1]. // Check the index register from the SIB byte + REX.X prefix. - int index_code = ((buf_[1] >> 3) & 0x07) | ((rex_ & 0x02) << 2); + int index_code = ((data_.buf[1] >> 3) & 0x07) | ((data_.rex & 0x02) << 2); // Index code (including REX.X) of 0x04 (rsp) means no index register. if (index_code != rsp.code() && index_code == code) return true; // Add REX.B to get the full base register code. - base_code = (buf_[1] & 0x07) | ((rex_ & 0x01) << 3); + base_code = (data_.buf[1] & 0x07) | ((data_.rex & 0x01) << 3); // A base register of 0x05 (rbp) with mod = 0 means no base register. - if (base_code == rbp.code() && ((buf_[0] & 0xC0) == 0)) return false; + if (base_code == rbp.code() && ((data_.buf[0] & 0xC0) == 0)) return false; return code == base_code; } else { // A base register with low bits of 0x05 (rbp or r13) and mod = 0 means // no base register. - if (base_code == rbp.code() && ((buf_[0] & 0xC0) == 0)) return false; - base_code |= ((rex_ & 0x01) << 3); + if (base_code == rbp.code() && ((data_.buf[0] & 0xC0) == 0)) return false; + base_code |= ((data_.rex & 0x01) << 3); return code == base_code; } } @@ -528,19 +585,20 @@ void Assembler::GrowBuffer() { void Assembler::emit_operand(int code, Operand adr) { DCHECK(is_uint3(code)); - const unsigned length = adr.len_; + const unsigned length = adr.data().len; DCHECK_GT(length, 0); // Emit updated ModR/M byte containing the given register. - DCHECK_EQ(adr.buf_[0] & 0x38, 0); - *pc_++ = adr.buf_[0] | code << 3; + DCHECK_EQ(adr.data().buf[0] & 0x38, 0); + *pc_++ = adr.data().buf[0] | code << 3; // Recognize RIP relative addressing. - if (adr.buf_[0] == 5) { + if (adr.data().buf[0] == 5) { DCHECK_EQ(9u, length); - Label* label = *bit_cast(&adr.buf_[1]); + Label* label = *bit_cast(&adr.data().buf[1]); if (label->is_bound()) { - int offset = label->pos() - pc_offset() - sizeof(int32_t) + adr.addend_; + int offset = + label->pos() - pc_offset() - sizeof(int32_t) + adr.data().addend; DCHECK_GE(0, offset); emitl(offset); } else if (label->is_linked()) { @@ -554,7 +612,7 @@ void Assembler::emit_operand(int code, Operand adr) { } } else { // Emit the rest of the encoded operand. - for (unsigned i = 1; i < length; i++) *pc_++ = adr.buf_[i]; + for (unsigned i = 1; i < length; i++) *pc_++ = adr.data().buf[i]; } } diff --git a/src/x64/assembler-x64.h b/src/x64/assembler-x64.h index 67af1b73ac..a532729d15 100644 --- a/src/x64/assembler-x64.h +++ b/src/x64/assembler-x64.h @@ -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_; } + int operand_size() const { return data_.len; } + + const Data& data() const { return data_; } 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); - - // 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); - - // 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; + 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::value && + std::is_trivially_destructible::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) \