Skip to content

x64: use Rust types for assembler immediates #10302

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

Merged
abrown merged 2 commits into from
Mar 3, 2025
Merged

x64: use Rust types for assembler immediates #10302

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
fd4b39f
x64: use Rust types for assembler immediates
abrown Feb 27, 2025
642aef1
Merge branch 'main' into assembler-hide-imm
abrown Mar 3, 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
13 changes: 0 additions & 13 deletions cranelift/assembler-x64/meta/src/generate.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,19 +52,6 @@ pub fn isle_macro(f: &mut Formatter, insts: &[dsl::Inst]) {
/// Generate the ISLE definitions that match the `isle_assembler_methods!` macro
/// above.
pub fn isle_definitions(f: &mut Formatter, insts: &[dsl::Inst]) {
f.line("(type AssemblerImm8 extern (enum))", None);
f.line("(type AssemblerSimm8 extern (enum))", None);
f.line("(type AssemblerImm16 extern (enum))", None);
f.line("(type AssemblerSimm16 extern (enum))", None);
f.line("(type AssemblerImm32 extern (enum))", None);
f.line("(type AssemblerSimm32 extern (enum))", None);
f.line("(type AssemblerReadGpr extern (enum))", None);
f.line("(type AssemblerReadWriteGpr extern (enum))", None);
f.line("(type AssemblerReadGprMem extern (enum))", None);
f.line("(type AssemblerReadWriteGprMem extern (enum))", None);
f.line("(type AssemblerInst extern (enum))", None);
f.empty_line();

f.line("(type AssemblerOutputs (enum", None);
f.line(" ;; Used for instructions that have ISLE `SideEffect`s (memory stores, traps,", None);
f.line(" ;; etc.) and do not return a `Value`.", None);
Expand Down
66 changes: 17 additions & 49 deletions cranelift/assembler-x64/meta/src/generate/operand.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,39 +20,16 @@ impl dsl::Operand {
}
}

#[must_use]
pub fn generate_mut_ty(&self, read_ty: &str, read_write_ty: &str) -> Option<String> {
use dsl::Mutability::*;
use dsl::OperandKind::*;
let pick_ty = match self.mutability {
Read => read_ty,
ReadWrite => read_write_ty,
};
match self.location.kind() {
FixedReg(_) => None,
Imm(loc) => {
let bits = loc.bits();
if self.extension.is_sign_extended() {
Some(format!("Simm{bits}"))
} else {
Some(format!("Imm{bits}"))
}
}
Reg(_) => Some(format!("Gpr<{pick_ty}>")),
RegMem(_) => Some(format!("GprMem<{pick_ty}, {read_ty}>")),
}
}

/// Returns the type of this operand in ISLE as part of the
/// `IsleConstructorRaw` variants.
/// Returns the type of this operand in ISLE as a part of the ISLE "raw"
/// constructors.
pub fn isle_param_raw(&self) -> String {
match self.location.kind() {
OperandKind::Imm(loc) => {
let bits = loc.bits();
if self.extension.is_sign_extended() {
format!("AssemblerSimm{bits}")
format!("i{bits}")
} else {
format!("AssemblerImm{bits}")
format!("u{bits}")
}
}
OperandKind::Reg(_) => "Gpr".to_string(),
Expand Down Expand Up @@ -85,9 +62,9 @@ impl dsl::Operand {
OperandKind::Imm(loc) => {
let bits = loc.bits();
if self.extension.is_sign_extended() {
format!("&cranelift_assembler_x64::Simm{bits}")
format!("i{bits}")
} else {
format!("&cranelift_assembler_x64::Imm{bits}")
format!("u{bits}")
}
}
OperandKind::RegMem(_) => "&GprMem".to_string(),
Expand All @@ -108,30 +85,21 @@ impl dsl::Operand {
Mutability::Read => "self.convert_gpr_mem_to_assembler_read_gpr_mem",
Mutability::ReadWrite => "self.convert_gpr_mem_to_assembler_read_write_gpr_mem",
}),
OperandKind::FixedReg(_) | OperandKind::Imm(_) => None,
OperandKind::Imm(loc) => match (self.extension.is_sign_extended(), loc.bits()) {
(true, 8) => Some("cranelift_assembler_x64::Simm8::new"),
(true, 16) => Some("cranelift_assembler_x64::Simm16::new"),
(true, 32) => Some("cranelift_assembler_x64::Simm32::new"),
(false, 8) => Some("cranelift_assembler_x64::Imm8::new"),
(false, 16) => Some("cranelift_assembler_x64::Imm16::new"),
(false, 32) => Some("cranelift_assembler_x64::Imm32::new"),
_ => None,
},
OperandKind::FixedReg(_) => None,
}
}
}

impl dsl::Location {
/// `<operand type>`, if the operand has a type (i.e., not fixed registers).
#[must_use]
pub fn generate_type(&self, generic: Option<String>) -> Option<String> {
use dsl::Location::*;
let generic = match generic {
Some(ty) => format!("<{ty}>"),
None => String::new(),
};
match self {
al | ax | eax | rax | cl => None,
imm8 => Some("Imm8".into()),
imm16 => Some("Imm16".into()),
imm32 => Some("Imm32".into()),
r8 | r16 | r32 | r64 => Some(format!("Gpr{generic}")),
rm8 | rm16 | rm32 | rm64 => Some(format!("GprMem{generic}")),
}
}

/// `self.<operand>.to_string(...)`
#[must_use]
pub fn generate_to_string(&self, extension: dsl::Extension) -> String {
Expand Down Expand Up @@ -159,7 +127,7 @@ impl dsl::Location {

/// `Size::<operand size>`
#[must_use]
pub fn generate_size(&self) -> Option<&str> {
fn generate_size(&self) -> Option<&str> {
use dsl::Location::*;
match self {
al | ax | eax | rax | cl | imm8 | imm16 | imm32 => None,
Expand Down
20 changes: 8 additions & 12 deletions cranelift/codegen/src/isa/x64/inst.isle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -827,6 +827,8 @@
;; An instruction assembled outside of cranelift-codegen.
(External (inst AssemblerInst))))

(type AssemblerInst extern (enum))

(type OperandSize extern
(enum Size8
Size16
Expand Down Expand Up @@ -2946,29 +2948,23 @@
dst_lo)))

;; Helpers for matching operands, extracting them into their assembler types.
(decl is_imm8 (AssemblerImm8) GprMemImm)
(decl is_imm8 (u8) GprMemImm)
(extern extractor is_imm8 is_imm8)
(decl is_simm8 (AssemblerSimm8) GprMemImm)
(decl is_simm8 (i8) GprMemImm)
(extern extractor is_simm8 is_simm8)
(decl is_imm16 (AssemblerImm16) GprMemImm)
(decl is_imm16 (u16) GprMemImm)
(extern extractor is_imm16 is_imm16)
(decl is_simm16 (AssemblerSimm16) GprMemImm)
(decl is_simm16 (i16) GprMemImm)
(extern extractor is_simm16 is_simm16)
(decl is_imm32 (AssemblerImm32) GprMemImm)
(decl is_imm32 (u32) GprMemImm)
(extern extractor is_imm32 is_imm32)
(decl is_simm32 (AssemblerSimm32) GprMemImm)
(decl is_simm32 (i32) GprMemImm)
(extern extractor is_simm32 is_simm32)
(decl is_gpr (Gpr) GprMemImm)
(extern extractor is_gpr is_gpr)
(decl is_gpr_mem (GprMem) GprMemImm)
(extern extractor is_gpr_mem is_gpr_mem)

;; Helpers to auto-convert to and from assembler types.

(decl u8_to_assembler_imm8 (u8) AssemblerImm8)
(extern constructor u8_to_assembler_imm8 u8_to_assembler_imm8)
(convert u8 AssemblerImm8 u8_to_assembler_imm8)

;; Helper for emitting `and` instructions.
(decl x64_and (Type Gpr GprMemImm) Gpr)

Expand Down
46 changes: 12 additions & 34 deletions cranelift/codegen/src/isa/x64/lower/isle.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,12 +42,6 @@ type BoxSyntheticAmode = Box<SyntheticAmode>;
/// When interacting with the external assembler (see `external.rs`), we
/// need to fix the types we'll use.
type AssemblerInst = asm::Inst<CraneliftRegisters>;
type AssemblerImm8 = asm::Imm8;
type AssemblerSimm8 = asm::Simm8;
type AssemblerImm16 = asm::Imm16;
type AssemblerSimm16 = asm::Simm16;
type AssemblerImm32 = asm::Imm32;
type AssemblerSimm32 = asm::Simm32;

pub struct SinkableLoad {
inst: Inst,
Expand Down Expand Up @@ -958,56 +952,44 @@ impl Context for IsleContext<'_, '_, MInst, X64Backend> {
///// External assembler methods.
////////////////////////////////////////////////////////////////////////////

fn is_imm8(&mut self, src: &GprMemImm) -> Option<AssemblerImm8> {
fn is_imm8(&mut self, src: &GprMemImm) -> Option<u8> {
match src.clone().to_reg_mem_imm() {
RegMemImm::Imm { simm32 } => {
let imm = u8::try_from(simm32).ok()?;
Some(AssemblerImm8::new(imm))
}
RegMemImm::Imm { simm32 } => Some(u8::try_from(simm32).ok()?),
_ => None,
}
}

fn is_simm8(&mut self, src: &GprMemImm) -> Option<AssemblerSimm8> {
fn is_simm8(&mut self, src: &GprMemImm) -> Option<i8> {
match src.clone().to_reg_mem_imm() {
RegMemImm::Imm { simm32 } => {
let imm = i8::try_from(simm32).ok()?;
Some(AssemblerSimm8::new(imm))
}
RegMemImm::Imm { simm32 } => Some(i8::try_from(simm32).ok()?),
_ => None,
}
}

fn is_imm16(&mut self, src: &GprMemImm) -> Option<AssemblerImm16> {
fn is_imm16(&mut self, src: &GprMemImm) -> Option<u16> {
match src.clone().to_reg_mem_imm() {
RegMemImm::Imm { simm32 } => {
let imm = u16::try_from(simm32).ok()?;
Some(AssemblerImm16::new(imm))
}
RegMemImm::Imm { simm32 } => Some(u16::try_from(simm32).ok()?),
_ => None,
}
}

fn is_simm16(&mut self, src: &GprMemImm) -> Option<AssemblerSimm16> {
fn is_simm16(&mut self, src: &GprMemImm) -> Option<i16> {
match src.clone().to_reg_mem_imm() {
RegMemImm::Imm { simm32 } => {
let imm = i16::try_from(simm32).ok()?;
Some(AssemblerSimm16::new(imm))
}
RegMemImm::Imm { simm32 } => Some(i16::try_from(simm32).ok()?),
_ => None,
}
}

fn is_imm32(&mut self, src: &GprMemImm) -> Option<AssemblerImm32> {
fn is_imm32(&mut self, src: &GprMemImm) -> Option<u32> {
match src.clone().to_reg_mem_imm() {
RegMemImm::Imm { simm32 } => Some(AssemblerImm32::new(simm32)),
RegMemImm::Imm { simm32 } => Some(simm32),
_ => None,
}
}

fn is_simm32(&mut self, src: &GprMemImm) -> Option<AssemblerSimm32> {
fn is_simm32(&mut self, src: &GprMemImm) -> Option<i32> {
match src.clone().to_reg_mem_imm() {
RegMemImm::Imm { simm32 } => Some(AssemblerSimm32::new(simm32 as i32)),
RegMemImm::Imm { simm32 } => Some(simm32 as i32),
_ => None,
}
}
Expand All @@ -1026,10 +1008,6 @@ impl Context for IsleContext<'_, '_, MInst, X64Backend> {
_ => None,
}
}

fn u8_to_assembler_imm8(&mut self, val: u8) -> AssemblerImm8 {
AssemblerImm8::new(val)
}
}

impl IsleContext<'_, '_, MInst, X64Backend> {
Expand Down
Loading