Skip to content

Initial support for vex encoding for the new assembler. #10695

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 2 commits into from
Closed

Initial support for vex encoding for the new assembler. #10695

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b41bba8
Initial support for vex encoding for the new assembler.
jlb6740 Apr 1, 2025
d962f1c
Remove dependency on meta crate from vex support
jlb6740 Apr 30, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 5 additions & 3 deletions cranelift/assembler-x64/meta/src/dsl.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,12 @@ mod encoding;
mod features;
pub mod format;

pub use encoding::{rex, vex};
pub use encoding::{Encoding, Group1Prefix, Group2Prefix, Group3Prefix, Group4Prefix, Opcodes, Prefixes, Rex};
pub use encoding::{
rex, vex, Encoding, Group1Prefix, Group2Prefix, Group3Prefix, Group4Prefix, Opcodes, Prefixes, Rex, Vex, VexLength,
VexMMMMM, VexPP,
};
pub use features::{Feature, Features, ALL_FEATURES};
pub use format::{align, fmt, r, rw, sxl, sxq, sxw};
pub use format::{align, fmt, r, rw, sxl, sxq, sxw, w};
pub use format::{Extension, Format, Location, Mutability, Operand, OperandKind};

/// Abbreviated constructor for an x64 instruction.
Expand Down
163 changes: 155 additions & 8 deletions cranelift/assembler-x64/meta/src/dsl/encoding.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,20 @@ pub fn rex(opcode: impl Into<Opcodes>) -> Rex {

/// An abbreviated constructor for VEX-encoded instructions.
#[must_use]
pub fn vex() -> Vex {
Vex {}
pub fn vex(opcode: impl Into<Opcodes>) -> Vex {
Vex {
opcodes: opcode.into(),
w: false,
r: false,
wig: false,
rxb: 0,
length: VexLength::default(),
mmmmm: VexMMMMM::None,
pp: VexPP::None,
reg: 0x00,
vvvv: None,
imm: None,
}
}

/// Enumerate the ways x64 encodes instructions.
Expand All @@ -48,7 +60,7 @@ impl Encoding {
pub fn validate(&self, operands: &[Operand]) {
match self {
Encoding::Rex(rex) => rex.validate(operands),
Encoding::Vex(vex) => vex.validate(),
Encoding::Vex(vex) => vex.validate(operands),
}
}
}
Expand All @@ -57,7 +69,7 @@ impl fmt::Display for Encoding {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
match self {
Encoding::Rex(rex) => write!(f, "{rex}"),
Encoding::Vex(_vex) => todo!(),
Encoding::Vex(vex) => write!(f, "{vex}"),
}
}
}
Expand Down Expand Up @@ -383,6 +395,23 @@ impl Prefixes {
pub fn is_empty(&self) -> bool {
self.group1.is_none() && self.group2.is_none() && self.group3.is_none() && self.group4.is_none()
}

pub fn bits(&self) -> u8 {
let mut bits = 0;
if self.group1.is_some() {
bits |= 0b0001;
}
if self.group2.is_some() {
bits |= 0b0010;
}
if self.group3.is_some() {
bits |= 0b0100;
}
if self.group4.is_some() {
bits |= 0b1000;
}
bits
}
}

pub enum Group1Prefix {
Expand Down Expand Up @@ -559,7 +588,7 @@ pub enum Imm {
}

impl Imm {
fn bits(&self) -> u8 {
fn bits(&self) -> u16 {
match self {
Imm::None => 0,
Imm::ib => 8,
Expand All @@ -582,10 +611,128 @@ impl fmt::Display for Imm {
}
}

pub struct Vex {}
pub struct Vex {
pub opcodes: Opcodes,
pub w: bool,
pub r: bool,
pub wig: bool,
pub rxb: u8,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think at the DSL level we may want to have separate booleans for each bit, e.g.:

r: bool,
x: bool,
b: bool,
w: bool,

As it stands, this duplicates the r bit.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I am not positive, but I don't even think any of these bits will need to be set at the DSL level. For now I'll just remove the rxb and add the b and w bits as suggested here (but maybe we can remove later).

pub length: VexLength,
pub mmmmm: VexMMMMM,
pub pp: VexPP,
pub reg: u8,
pub vvvv: Option<Register>,
pub imm: Option<u8>,
}

#[derive(PartialEq)]
pub enum VexPP {
None,
/// Operand size override -- here, denoting "16-bit operation".
_66,
/// REPNE, but no specific meaning here -- is just an opcode extension.
_F2,
/// REP/REPE, but no specific meaning here -- is just an opcode extension.
_F3,
}

impl fmt::Display for VexPP {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
match self {
VexPP::None => write!(f, "None"),
VexPP::_66 => write!(f, "_66"),
VexPP::_F3 => write!(f, "_F3"),
VexPP::_F2 => write!(f, "_F2"),
}
}
}

#[derive(PartialEq)]
pub enum VexMMMMM {
None,
_OF,
/// Operand size override -- here, denoting "16-bit operation".
_OF3A,
/// The lock prefix.
_OF38,
}

impl fmt::Display for VexMMMMM {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
match self {
VexMMMMM::None => write!(f, "None"),
VexMMMMM::_OF => write!(f, "_0F"),
VexMMMMM::_OF3A => write!(f, "_OF3A"),
VexMMMMM::_OF38 => write!(f, "_OF38"),
}
}
}

pub enum VexLength {
_128,
_256,
}

impl VexLength {
/// Encode the `L` bit.
pub fn bits(&self) -> u8 {
match self {
Self::_128 => 0b0,
Self::_256 => 0b1,
}
}
}

impl Default for VexLength {
fn default() -> Self {
Self::_128
}
}

/// Describe the register index to use. This wrapper is a type-safe way to pass
/// around the registers defined in `inst/regs.rs`.
#[derive(Debug, Copy, Clone, Default)]
pub struct Register(u8);
impl From<u8> for Register {
fn from(reg: u8) -> Self {
debug_assert!(reg < 16);
Self(reg)
}
}
impl Into<u8> for Register {
fn into(self) -> u8 {
self.0
}
}

impl Vex {
fn validate(&self) {
todo!()
pub fn length(self, length: VexLength) -> Self {
Self { length, ..self }
}
pub fn pp(self, pp: VexPP) -> Self {
Self { pp, ..self }
}
pub fn mmmmm(self, mmmmm: VexMMMMM) -> Self {
Self { mmmmm, ..self }
}

fn validate(&self, _operands: &[Operand]) {}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It will be helpful to check here for invalid instruction definitions: that way we can find them early, at codegen time.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes agreed .. can we implement this validate in the next PR where we turn the plumbing on and we can see that the validation is actually doing the right thing against those first few instructions or would you like to see a little more of this filled out now?

}

impl From<Vex> for Encoding {
fn from(vex: Vex) -> Encoding {
Encoding::Vex(vex)
}
}

impl fmt::Display for Vex {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
write!(f, "VEX")?;
match self.length {
VexLength::_128 => write!(f, ".128")?,
VexLength::_256 => write!(f, ".256")?,
}
write!(f, " {:#04x}", self.opcodes.primary)?;
Ok(())
}
}
74 changes: 58 additions & 16 deletions cranelift/assembler-x64/meta/src/dsl/format.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,16 @@ pub fn r(op: impl Into<Operand>) -> Operand {
op
}

#[must_use]
pub fn w(location: Location) -> Operand {
Operand {
location,
mutability: Mutability::Write,
extension: Extension::None,
align: false,
}
}

/// An abbreviated constructor for a memory operand that requires alignment.
pub fn align(location: Location) -> Operand {
assert!(location.uses_memory());
Expand Down Expand Up @@ -236,8 +246,6 @@ pub enum Location {
r32,
r64,

xmm,

rm8,
rm16,
rm32,
Expand All @@ -248,35 +256,51 @@ pub enum Location {
m16,
m32,
m64,
xmm1,
xmm2,
xmm3,
ymm1,
ymm2,
ymm3,
zmm1,
zmm2,
zmm3,

xmm_m128,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the same as rm128... I'm not convinced adding these unused variants is necessary at this point, either.

Copy link
Contributor Author

@jlb6740 jlb6740 May 1, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was wanting to be consistent with the manual here:

ADDPD xmm1, xmm2/m128
VADDPD xmm1, xmm2, xmm3/m128

and writing something like this:

inst(
            "vaddpd",
            fmt("B", [w(xmm1), r(xmm2), r(xmm_m128)]),
            vex(0x58).length(_128).pp(_66).mmmmm(_OF),
            _64b | compat | sse,
        ),

instead of like this:

inst(
            "vaddpd",
            fmt("B", [w(xmm1), r(xmm2), r(rm128)]),
            vex(0x58).length(_128).pp(_66).mmmmm(_OF),
            _64b | compat | sse,
        ),

I kept the rm128 because I wasn't sure of opinions of this during review that I replaced rm128 with xmm_m128 in all the rex encoded instructions just yet (maybe a separate patch), but using the naming xmm_m128 is more consistent with the manual. What do you think?

ymm_m256,
zmm_m512,
}

impl Location {
/// Return the number of bits accessed.
#[must_use]
pub fn bits(&self) -> u8 {
pub fn bits(&self) -> u16 {
use Location::*;
match self {
al | cl | imm8 | r8 | rm8 | m8 => 8,
ax | imm16 | r16 | rm16 | m16 => 16,
eax | imm32 | r32 | rm32 | m32 => 32,
rax | r64 | rm64 | m64 => 64,
xmm | rm128 => 128,
rm128 | xmm1 | xmm2 | xmm3 | xmm_m128 => 128,
ymm1 | ymm2 | ymm3 | ymm_m256 => 256,
zmm1 | zmm2 | zmm3 | zmm_m512 => 512,
}
}

/// Return the number of bytes accessed, for convenience.
#[must_use]
pub fn bytes(&self) -> u8 {
self.bits() / 8
pub fn bytes(&self) -> u16 {
self.bits() / 16
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not right:

Suggested change
self.bits() / 16
self.bits() / 8

}

/// Return `true` if the location accesses memory; `false` otherwise.
#[must_use]
pub fn uses_memory(&self) -> bool {
use Location::*;
match self {
al | cl | ax | eax | rax | imm8 | imm16 | imm32 | r8 | r16 | r32 | r64 | xmm => false,
rm8 | rm16 | rm32 | rm64 | rm128 | m8 | m16 | m32 | m64 => true,
al | cl | ax | eax | rax | imm8 | imm16 | imm32 | r8 | r16 | r32 | r64 | xmm1 | xmm2 | xmm3 | ymm1
| ymm2 | ymm3 | zmm1 | zmm2 | zmm3 => false,
rm8 | rm16 | rm32 | rm64 | rm128 | m8 | m16 | m32 | m64 | xmm_m128 | ymm_m256 | zmm_m512 => true,
}
}

Expand All @@ -286,9 +310,9 @@ impl Location {
pub fn uses_register(&self) -> bool {
use Location::*;
match self {
imm8 | imm16 | imm32 => false,
al | ax | eax | rax | cl | r8 | r16 | r32 | r64 | xmm | rm8 | rm16 | rm32 | rm64 | rm128 | m8 | m16
| m32 | m64 => true,
cl | imm8 | imm16 | imm32 => false,
al | ax | eax | rax | r8 | r16 | r32 | r64 | rm8 | rm16 | rm32 | rm64 | rm128 | m8 | m16 | m32 | m64
| xmm1 | xmm2 | xmm3 | ymm1 | ymm2 | ymm3 | zmm1 | zmm2 | zmm3 | xmm_m128 | ymm_m256 | zmm_m512 => true,
}
}

Expand All @@ -299,8 +323,10 @@ impl Location {
match self {
al | ax | eax | rax | cl => OperandKind::FixedReg(*self),
imm8 | imm16 | imm32 => OperandKind::Imm(*self),
r8 | r16 | r32 | r64 | xmm => OperandKind::Reg(*self),
rm8 | rm16 | rm32 | rm64 | rm128 => OperandKind::RegMem(*self),
r8 | r16 | r32 | r64 | xmm1 | xmm2 | xmm3 | ymm1 | ymm2 | ymm3 | zmm1 | zmm2 | zmm3 => {
OperandKind::Reg(*self)
}
rm8 | rm16 | rm32 | rm64 | rm128 | xmm_m128 | ymm_m256 | zmm_m512 => OperandKind::RegMem(*self),
m8 | m16 | m32 | m64 => OperandKind::Mem(*self),
}
}
Expand All @@ -326,8 +352,6 @@ impl core::fmt::Display for Location {
r32 => write!(f, "r32"),
r64 => write!(f, "r64"),

xmm => write!(f, "xmm"),

rm8 => write!(f, "rm8"),
rm16 => write!(f, "rm16"),
rm32 => write!(f, "rm32"),
Expand All @@ -338,6 +362,21 @@ impl core::fmt::Display for Location {
m16 => write!(f, "m16"),
m32 => write!(f, "m32"),
m64 => write!(f, "m64"),
xmm1 => write!(f, "xmm1"),
xmm2 => write!(f, "xmm2"),
xmm3 => write!(f, "xmm3"),

ymm1 => write!(f, "ymm1"),
ymm2 => write!(f, "ymm2"),
ymm3 => write!(f, "ymm3"),

zmm1 => write!(f, "zmm1"),
zmm2 => write!(f, "zmm2"),
zmm3 => write!(f, "zmm3"),

xmm_m128 => write!(f, "xmm_m128"),
ymm_m256 => write!(f, "ymm_m256"),
zmm_m512 => write!(f, "zmm_m512"),
}
}
}
Expand Down Expand Up @@ -368,6 +407,7 @@ pub enum OperandKind {
pub enum Mutability {
Read,
ReadWrite,
Write,
}

impl Mutability {
Expand All @@ -377,6 +417,7 @@ impl Mutability {
pub fn is_read(&self) -> bool {
match self {
Mutability::Read | Mutability::ReadWrite => true,
Mutability::Write => false,
}
}

Expand All @@ -386,7 +427,7 @@ impl Mutability {
pub fn is_write(&self) -> bool {
match self {
Mutability::Read => false,
Mutability::ReadWrite => true,
Mutability::ReadWrite | Mutability::Write => true,
}
}
}
Expand All @@ -402,6 +443,7 @@ impl core::fmt::Display for Mutability {
match self {
Self::Read => write!(f, "r"),
Self::ReadWrite => write!(f, "rw"),
Self::Write => write!(f, "w"),
}
}
}
Expand Down
Loading
Loading