diff --git a/Userland/DevTools/UserspaceEmulator/SoftCPU.cpp b/Userland/DevTools/UserspaceEmulator/SoftCPU.cpp index 6cfaebbd57..784311d8cd 100644 --- a/Userland/DevTools/UserspaceEmulator/SoftCPU.cpp +++ b/Userland/DevTools/UserspaceEmulator/SoftCPU.cpp @@ -1721,7 +1721,7 @@ void SoftCPU::FCMOVB(const X86::Instruction& insn) { VERIFY(insn.modrm().is_register()); if (cf()) - fpu_set(0, fpu_get(insn.rm() & 7)); + fpu_set(0, fpu_get(insn.modrm().rm())); } void SoftCPU::FIMUL_RM32(const X86::Instruction& insn) @@ -1736,7 +1736,7 @@ void SoftCPU::FCMOVE(const X86::Instruction& insn) { VERIFY(insn.modrm().is_register()); if (zf()) - fpu_set(0, fpu_get(insn.rm() & 7)); + fpu_set(0, fpu_get(insn.modrm().rm())); } void SoftCPU::FICOM_RM32(const X86::Instruction&) { TODO_INSN(); } @@ -1744,7 +1744,7 @@ void SoftCPU::FICOM_RM32(const X86::Instruction&) { TODO_INSN(); } void SoftCPU::FCMOVBE(const X86::Instruction& insn) { if (evaluate_condition(6)) - fpu_set(0, fpu_get(insn.rm() & 7)); + fpu_set(0, fpu_get(insn.modrm().rm())); } void SoftCPU::FICOMP_RM32(const X86::Instruction&) { TODO_INSN(); } @@ -1832,7 +1832,7 @@ void SoftCPU::FIST_RM32(const X86::Instruction& insn) void SoftCPU::FCMOVNBE(const X86::Instruction& insn) { if (evaluate_condition(7)) - fpu_set(0, fpu_get(insn.rm() & 7)); + fpu_set(0, fpu_get(insn.modrm().rm())); } void SoftCPU::FISTP_RM32(const X86::Instruction& insn) @@ -1869,7 +1869,7 @@ void SoftCPU::FLD_RM80(const X86::Instruction& insn) void SoftCPU::FUCOMI(const X86::Instruction& insn) { - auto i = insn.rm() & 7; + auto i = insn.modrm().rm(); // FIXME: Unordered comparison checks. // FIXME: QNaN / exception handling. // FIXME: Set C0, C2, C3 in FPU status word. @@ -1890,7 +1890,7 @@ void SoftCPU::FUCOMI(const X86::Instruction& insn) void SoftCPU::FCOMI(const X86::Instruction& insn) { - auto i = insn.rm() & 7; + auto i = insn.modrm().rm(); // FIXME: QNaN / exception handling. // FIXME: Set C0, C2, C3 in FPU status word. set_zf(fpu_get(0) == fpu_get(i)); diff --git a/Userland/Libraries/LibX86/Instruction.cpp b/Userland/Libraries/LibX86/Instruction.cpp index 2e0fac7059..aba6379e6e 100644 --- a/Userland/Libraries/LibX86/Instruction.cpp +++ b/Userland/Libraries/LibX86/Instruction.cpp @@ -1037,7 +1037,7 @@ String MemoryOrRegisterReference::to_string_a16() const String base; bool hasDisplacement = false; - switch (m_rm & 7) { + switch (rm()) { case 0: base = "bx+si"; break; @@ -1060,16 +1060,16 @@ String MemoryOrRegisterReference::to_string_a16() const base = "bx"; break; case 6: - if ((m_rm & 0xc0) == 0) + if (mod() == 0) base = String::formatted("{:#04x}", m_displacement16); else base = "bp"; break; } - switch (m_rm & 0xc0) { - case 0x40: - case 0x80: + switch (mod()) { + case 0b01: + case 0b10: hasDisplacement = true; } @@ -1178,16 +1178,16 @@ String MemoryOrRegisterReference::to_string_a32() const return register_name(static_cast(m_register_index)); bool has_displacement = false; - switch (m_rm & 0xc0) { - case 0x40: - case 0x80: + switch (mod()) { + case 0b01: + case 0b10: has_displacement = true; } if (m_has_sib && (m_sib & 7) == 5) has_displacement = true; String base; - switch (m_rm & 7) { + switch (rm()) { case 0: base = "eax"; break; @@ -1207,13 +1207,13 @@ String MemoryOrRegisterReference::to_string_a32() const base = "edi"; break; case 5: - if ((m_rm & 0xc0) == 0) + if (mod() == 0) base = String::formatted("{:#08x}", m_displacement32); else base = "ebp"; break; case 4: - base = sib_to_string(m_rm, m_sib); + base = sib_to_string(m_rm_byte, m_sib); break; } @@ -1766,7 +1766,7 @@ void Instruction::to_string_internal(StringBuilder& builder, u32 origin, const S break; case OP_reg32_CR: append_mnemonic_space(); - builder.append(register_name(static_cast(rm() & 7))); + builder.append(register_name(static_cast(modrm().rm()))); append(", "); append_creg(); break; @@ -1774,11 +1774,11 @@ void Instruction::to_string_internal(StringBuilder& builder, u32 origin, const S append_mnemonic_space(); append_creg(); append(", "); - builder.append(register_name(static_cast(rm() & 7))); + builder.append(register_name(static_cast(modrm().rm()))); break; case OP_reg32_DR: append_mnemonic_space(); - builder.append(register_name(static_cast(rm() & 7))); + builder.append(register_name(static_cast(modrm().rm()))); append(", "); append_dreg(); break; @@ -1786,7 +1786,7 @@ void Instruction::to_string_internal(StringBuilder& builder, u32 origin, const S append_mnemonic_space(); append_dreg(); append(", "); - builder.append(register_name(static_cast(rm() & 7))); + builder.append(register_name(static_cast(modrm().rm()))); break; case OP_short_imm8: append_mnemonic_space(); diff --git a/Userland/Libraries/LibX86/Instruction.h b/Userland/Libraries/LibX86/Instruction.h index 8985f92e39..e515165cd2 100644 --- a/Userland/Libraries/LibX86/Instruction.h +++ b/Userland/Libraries/LibX86/Instruction.h @@ -374,6 +374,11 @@ public: RegisterIndex8 reg8() const { return static_cast(register_index()); } FpuRegisterIndex reg_fpu() const { return static_cast(register_index()); } + // helpers to get the parts by name as in the spec + u8 mod() const { return m_rm_byte >> 6; } + u8 reg() const { return m_rm_byte >> 3 & 0b111; } + u8 rm() const { return m_rm_byte & 0b111; } + template void write8(CPU&, const Instruction&, T); template @@ -429,7 +434,7 @@ private: u16 m_displacement16; }; - u8 m_rm { 0 }; + u8 m_rm_byte { 0 }; u8 m_sib { 0 }; u8 m_displacement_bytes { 0 }; u8 m_register_index : 7 { 0x7f }; @@ -467,8 +472,8 @@ public: String mnemonic() const; u8 op() const { return m_op; } - u8 rm() const { return m_modrm.m_rm; } - u8 slash() const { return (rm() >> 3) & 7; } + u8 modrm_byte() const { return m_modrm.m_rm_byte; } + u8 slash() const { return (modrm_byte() >> 3) & 7; } u8 imm8() const { return m_imm1; } u16 imm16() const { return m_imm1; } @@ -535,7 +540,7 @@ ALWAYS_INLINE LogicalAddress MemoryOrRegisterReference::resolve16(const CPU& cpu auto default_segment = SegmentRegister::DS; u16 offset = 0; - switch (m_rm & 7) { + switch (rm()) { case 0: offset = cpu.bx().value() + cpu.si().value() + m_displacement16; break; @@ -557,7 +562,7 @@ ALWAYS_INLINE LogicalAddress MemoryOrRegisterReference::resolve16(const CPU& cpu offset = cpu.di().value() + m_displacement16; break; case 6: - if ((m_rm & 0xc0) == 0) + if (mod() == 0) offset = m_displacement16; else { default_segment = SegmentRegister::SS; @@ -579,16 +584,16 @@ ALWAYS_INLINE LogicalAddress MemoryOrRegisterReference::resolve32(const CPU& cpu auto default_segment = SegmentRegister::DS; u32 offset = 0; - switch (m_rm & 0x07) { + switch (rm()) { case 0 ... 3: case 6 ... 7: - offset = cpu.const_gpr32((RegisterIndex32)(m_rm & 0x07)).value() + m_displacement32; + offset = cpu.const_gpr32((RegisterIndex32)(rm())).value() + m_displacement32; break; case 4: offset = evaluate_sib(cpu, default_segment); break; default: // 5 - if ((m_rm & 0xc0) == 0x00) { + if (mod() == 0) { offset = m_displacement32; break; } else { @@ -628,7 +633,7 @@ ALWAYS_INLINE u32 MemoryOrRegisterReference::evaluate_sib(const CPU& cpu, Segmen base += cpu.esp().value(); break; default: // 5 - switch ((m_rm >> 6) & 3) { + switch (mod()) { case 0: break; case 1: @@ -846,7 +851,7 @@ ALWAYS_INLINE Instruction::Instruction(InstructionStreamType& stream, bool o32, if (m_descriptor->has_rm) { // Consume ModR/M (may include SIB and displacement.) m_modrm.decode(stream, m_a32); - m_register_index = (m_modrm.m_rm >> 3) & 7; + m_register_index = m_modrm.reg(); } else { if (has_sub_op()) m_register_index = m_sub_op & 7; @@ -857,8 +862,8 @@ ALWAYS_INLINE Instruction::Instruction(InstructionStreamType& stream, bool o32, bool has_slash = m_descriptor->format == MultibyteWithSlash; if (has_slash) { m_descriptor = &m_descriptor->slashes[slash()]; - if ((rm() & 0xc0) == 0xc0 && m_descriptor->slashes) - m_descriptor = &m_descriptor->slashes[rm() & 7]; + if ((modrm_byte() & 0xc0) == 0xc0 && m_descriptor->slashes) + m_descriptor = &m_descriptor->slashes[modrm_byte() & 7]; } if (!m_descriptor->mnemonic) { @@ -924,7 +929,7 @@ ALWAYS_INLINE Instruction::Instruction(InstructionStreamType& stream, bool o32, template ALWAYS_INLINE void MemoryOrRegisterReference::decode(InstructionStreamType& stream, bool a32) { - m_rm = stream.read8(); + m_rm_byte = stream.read8(); if (a32) { decode32(stream); @@ -962,21 +967,21 @@ ALWAYS_INLINE void MemoryOrRegisterReference::decode(InstructionStreamType& stre template ALWAYS_INLINE void MemoryOrRegisterReference::decode16(InstructionStreamType&) { - switch (m_rm & 0xc0) { - case 0: - if ((m_rm & 0x07) == 6) + switch (mod()) { + case 0b00: + if (rm() == 6) m_displacement_bytes = 2; else VERIFY(m_displacement_bytes == 0); break; - case 0x40: + case 0b01: m_displacement_bytes = 1; break; - case 0x80: + case 0b10: m_displacement_bytes = 2; break; - case 0xc0: - m_register_index = m_rm & 7; + case 0b11: + m_register_index = rm(); break; } } @@ -984,34 +989,34 @@ ALWAYS_INLINE void MemoryOrRegisterReference::decode16(InstructionStreamType&) template ALWAYS_INLINE void MemoryOrRegisterReference::decode32(InstructionStreamType& stream) { - switch (m_rm & 0xc0) { - case 0: - if ((m_rm & 0x07) == 5) + switch (mod()) { + case 0b00: + if (rm() == 5) m_displacement_bytes = 4; break; - case 0x40: + case 0b01: m_displacement_bytes = 1; break; - case 0x80: + case 0b10: m_displacement_bytes = 4; break; - case 0xc0: - m_register_index = m_rm & 7; + case 0b11: + m_register_index = rm(); return; } - m_has_sib = (m_rm & 0x07) == 4; + m_has_sib = rm() == 4; if (m_has_sib) { m_sib = stream.read8(); if ((m_sib & 0x07) == 5) { - switch ((m_rm >> 6) & 0x03) { - case 0: + switch (mod()) { + case 0b00: m_displacement_bytes = 4; break; - case 1: + case 0b01: m_displacement_bytes = 1; break; - case 2: + case 0b10: m_displacement_bytes = 4; break; default: