Skip to content

Commit 5904cfe

Browse files
committed
Addressing many PR comments: Remove ymm and zmm support, cleanup, etc
1 parent fbd2beb commit 5904cfe

File tree

19 files changed

+138
-245
lines changed

19 files changed

+138
-245
lines changed

cranelift/assembler-x64/meta/src/dsl.rs

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -8,14 +8,13 @@ mod encoding;
88
mod features;
99
pub mod format;
1010

11-
pub use encoding::{rex, vex};
1211
pub use encoding::{
13-
rex, vex, Encoding, Group1Prefix, Group2Prefix, Group3Prefix, Group4Prefix, Opcodes, Prefixes,
14-
Rex, Vex, VexLength, VexMMMMM, VexPP,
12+
Encoding, Group1Prefix, Group2Prefix, Group3Prefix, Group4Prefix, Opcodes, Prefixes, Rex, Vex,
13+
VexLength, VexMMMMM, VexPP, rex, vex,
1514
};
16-
pub use features::{Feature, Features, ALL_FEATURES};
17-
pub use format::{align, fmt, implicit, r, rw, sxl, sxq, sxw, w};
15+
pub use features::{ALL_FEATURES, Feature, Features};
1816
pub use format::{Extension, Format, Location, Mutability, Operand, OperandKind, RegClass};
17+
pub use format::{align, fmt, implicit, r, rw, sxl, sxq, sxw, w};
1918

2019
/// Abbreviated constructor for an x64 instruction.
2120
pub fn inst(

cranelift/assembler-x64/meta/src/dsl/encoding.rs

Lines changed: 13 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -36,12 +36,9 @@ pub fn vex(opcode: impl Into<Opcodes>) -> Vex {
3636
Vex {
3737
opcodes: opcode.into(),
3838
w: false,
39-
wig: false,
40-
length: VexLength::default(),
39+
length: VexLength::_128,
4140
mmmmm: VexMMMMM::None,
4241
pp: VexPP::None,
43-
reg: 0x00,
44-
vvvv: None,
4542
imm: None,
4643
}
4744
}
@@ -418,23 +415,6 @@ impl Prefixes {
418415
&& self.group3.is_none()
419416
&& self.group4.is_none()
420417
}
421-
422-
pub fn bits(&self) -> u8 {
423-
let mut bits = 0;
424-
if self.group1.is_some() {
425-
bits |= 0b0001;
426-
}
427-
if self.group2.is_some() {
428-
bits |= 0b0010;
429-
}
430-
if self.group3.is_some() {
431-
bits |= 0b0100;
432-
}
433-
if self.group4.is_some() {
434-
bits |= 0b1000;
435-
}
436-
bits
437-
}
438418
}
439419

440420
pub enum Group1Prefix {
@@ -637,12 +617,9 @@ impl fmt::Display for Imm {
637617
pub struct Vex {
638618
pub opcodes: Opcodes,
639619
pub w: bool,
640-
pub wig: bool,
641620
pub length: VexLength,
642621
pub mmmmm: VexMMMMM,
643622
pub pp: VexPP,
644-
pub reg: u8,
645-
pub vvvv: Option<Register>,
646623
pub imm: Option<u8>,
647624
}
648625

@@ -668,6 +645,18 @@ impl fmt::Display for VexPP {
668645
}
669646
}
670647

648+
pub enum VexLength {
649+
_128,
650+
}
651+
652+
impl fmt::Display for VexLength {
653+
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
654+
match self {
655+
VexLength::_128 => write!(f, "_128"),
656+
}
657+
}
658+
}
659+
671660
#[derive(PartialEq)]
672661
pub enum VexMMMMM {
673662
None,
@@ -689,27 +678,6 @@ impl fmt::Display for VexMMMMM {
689678
}
690679
}
691680

692-
pub enum VexLength {
693-
_128,
694-
_256,
695-
}
696-
697-
impl VexLength {
698-
/// Encode the `L` bit.
699-
pub fn bits(&self) -> u8 {
700-
match self {
701-
Self::_128 => 0b0,
702-
Self::_256 => 0b1,
703-
}
704-
}
705-
}
706-
707-
impl Default for VexLength {
708-
fn default() -> Self {
709-
Self::_128
710-
}
711-
}
712-
713681
/// Describe the register index to use. This wrapper is a type-safe way to pass
714682
/// around the registers defined in `inst/regs.rs`.
715683
#[derive(Debug, Copy, Clone, Default)]
@@ -751,7 +719,6 @@ impl fmt::Display for Vex {
751719
write!(f, "VEX")?;
752720
match self.length {
753721
VexLength::_128 => write!(f, ".128")?,
754-
VexLength::_256 => write!(f, ".256")?,
755722
}
756723
write!(f, " {:#04x}", self.opcodes.primary)?;
757724
Ok(())

cranelift/assembler-x64/meta/src/dsl/format.rs

Lines changed: 6 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -303,8 +303,6 @@ pub enum Location {
303303
xmm_m32,
304304
xmm_m64,
305305
xmm_m128,
306-
ymm_m256,
307-
zmm_m512,
308306

309307
// Memory-only locations.
310308
m8,
@@ -314,12 +312,6 @@ pub enum Location {
314312
xmm1,
315313
xmm2,
316314
xmm3,
317-
ymm1,
318-
ymm2,
319-
ymm3,
320-
zmm1,
321-
zmm2,
322-
zmm3,
323315
}
324316

325317
impl Location {
@@ -333,8 +325,6 @@ impl Location {
333325
eax | edx | imm32 | r32 | rm32 | m32 | xmm_m32 => 32,
334326
rax | rdx | r64 | rm64 | m64 | xmm_m64 => 64,
335327
xmm1 | xmm2 | xmm3 | xmm_m128 => 128,
336-
ymm1 | ymm2 | ymm3 | ymm_m256 => 256,
337-
zmm1 | zmm2 | zmm3 | zmm_m512 => 512,
338328
}
339329
}
340330

@@ -350,9 +340,8 @@ impl Location {
350340
use Location::*;
351341
match self {
352342
al | ax | eax | rax | cl | dx | edx | rdx | imm8 | imm16 | imm32 | r8 | r16 | r32
353-
| r64 | xmm1 | xmm2 | xmm3 | ymm1 | ymm2 | ymm3 | zmm1 | zmm2 | zmm3 => false,
354-
rm8 | rm16 | rm32 | rm64 | xmm_m32 | xmm_m64 | xmm_m128 | m8 | m16 | m32 | m64
355-
| ymm_m256 | zmm_m512 => true,
343+
| r64 | xmm1 | xmm2 | xmm3 => false,
344+
rm8 | rm16 | rm32 | rm64 | m8 | m16 | m32 | m64 | xmm_m32 | xmm_m64 | xmm_m128 => true,
356345
}
357346
}
358347

@@ -365,7 +354,7 @@ impl Location {
365354
imm8 | imm16 | imm32 => false,
366355
al | ax | eax | rax | cl | dx | edx | rdx | r8 | r16 | r32 | r64 | rm8 | rm16
367356
| rm32 | rm64 | m8 | m16 | m32 | m64 | xmm1 | xmm2 | xmm3 | xmm_m32 | xmm_m64
368-
| ymm1 | ymm2 | ymm3 | zmm1 | zmm2 | zmm3 | xmm_m128 | ymm_m256 | zmm_m512 => true,
357+
| xmm_m128 => true,
369358
}
370359
}
371360

@@ -376,12 +365,8 @@ impl Location {
376365
match self {
377366
al | ax | eax | rax | cl | dx | edx | rdx => OperandKind::FixedReg(*self),
378367
imm8 | imm16 | imm32 => OperandKind::Imm(*self),
379-
r8 | r16 | r32 | r64 | xmm1 | xmm2 | xmm3 | ymm1 | ymm2 | ymm3 | zmm1 | zmm2 | zmm3 => {
380-
OperandKind::Reg(*self)
381-
}
382-
rm8 | rm16 | rm32 | rm64 | xmm_m32 | xmm_m64 | xmm_m128 | ymm_m256 | zmm_m512 => {
383-
OperandKind::RegMem(*self)
384-
}
368+
r8 | r16 | r32 | r64 | xmm1 | xmm2 | xmm3 => OperandKind::Reg(*self),
369+
rm8 | rm16 | rm32 | rm64 | xmm_m32 | xmm_m64 | xmm_m128 => OperandKind::RegMem(*self),
385370
m8 | m16 | m32 | m64 => OperandKind::Mem(*self),
386371
}
387372
}
@@ -397,8 +382,7 @@ impl Location {
397382
imm8 | imm16 | imm32 | m8 | m16 | m32 | m64 => None,
398383
al | ax | eax | rax | cl | dx | edx | rdx | r8 | r16 | r32 | r64 | rm8 | rm16
399384
| rm32 | rm64 => Some(RegClass::Gpr),
400-
xmm1 | xmm2 | xmm3 | ymm1 | ymm2 | ymm3 | zmm1 | zmm2 | zmm3 | xmm_m32 | xmm_m64
401-
| xmm_m128 | ymm_m256 | zmm_m512 => Some(RegClass::Xmm),
385+
xmm1 | xmm2 | xmm3 | xmm_m32 | xmm_m64 | xmm_m128 => Some(RegClass::Xmm),
402386
}
403387
}
404388
}
@@ -432,8 +416,6 @@ impl core::fmt::Display for Location {
432416
xmm_m32 => write!(f, "xmm_m32"),
433417
xmm_m64 => write!(f, "xmm_m64"),
434418
xmm_m128 => write!(f, "xmm_m128"),
435-
ymm_m256 => write!(f, "ymm_m256"),
436-
zmm_m512 => write!(f, "zmm_m512"),
437419

438420
m8 => write!(f, "m8"),
439421
m16 => write!(f, "m16"),
@@ -443,14 +425,6 @@ impl core::fmt::Display for Location {
443425
xmm1 => write!(f, "xmm1"),
444426
xmm2 => write!(f, "xmm2"),
445427
xmm3 => write!(f, "xmm3"),
446-
447-
ymm1 => write!(f, "ymm1"),
448-
ymm2 => write!(f, "ymm2"),
449-
ymm3 => write!(f, "ymm3"),
450-
451-
zmm1 => write!(f, "zmm1"),
452-
zmm2 => write!(f, "zmm2"),
453-
zmm3 => write!(f, "zmm3"),
454428
}
455429
}
456430
}

cranelift/assembler-x64/meta/src/generate/format.rs

Lines changed: 29 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,35 @@ impl dsl::Format {
4848
}
4949

5050
pub fn generate_vex_encoding(&self, f: &mut Formatter, vex: &dsl::Vex) {
51-
self.generate_vex(f, vex);
51+
use dsl::OperandKind::{Reg, RegMem};
52+
f.empty_line();
53+
f.comment("Emit New VEX prefix.");
54+
55+
match self.operands_by_kind().as_slice() {
56+
[Reg(xmm1), Reg(xmm2), RegMem(xmm_m128)] => {
57+
fmtln!(
58+
f,
59+
"vex_instruction::<R>(
60+
0x{:0x},
61+
VexVectorLength::{},
62+
VexPP::{},
63+
OpcodeMap::{},
64+
self.{}.enc(),
65+
Some(self.{}.enc()),
66+
Some(self.{}.clone()),
67+
{}).encode(buf, off);",
68+
vex.opcodes.primary,
69+
vex.length.to_string(),
70+
vex.pp.to_string(),
71+
vex.mmmmm.to_string(),
72+
xmm1,
73+
xmm2,
74+
xmm_m128,
75+
"None"
76+
);
77+
}
78+
_ => unimplemented!(),
79+
}
5280
}
5381

5482
/// `buf.put1(...);`
@@ -142,31 +170,6 @@ impl dsl::Format {
142170
fmtln!(f, "rex.encode(buf);");
143171
}
144172

145-
fn generate_vex(&self, f: &mut Formatter, vex: &dsl::Vex) {
146-
f.empty_line();
147-
f.comment("Emit VEX prefix.");
148-
fmtln!(
149-
f,
150-
"let mut vex: VexInstruction<R> = vex_instruction(0x{:0x});",
151-
vex.opcodes.primary
152-
);
153-
fmtln!(f, "vex.reg = self.xmm1.enc();");
154-
f.add_block("match &self.xmm_m128", |f| {
155-
fmtln!(
156-
f,
157-
"XmmMem::Xmm(r) => {{vex.rm = Some(XmmMem::Xmm(r.clone()));}}"
158-
);
159-
fmtln!(
160-
f,
161-
"XmmMem::Mem(m) => {{vex.rm = Some(XmmMem::Mem(m.clone()));}}"
162-
);
163-
});
164-
fmtln!(f, "vex.vvvv = Some(self.xmm2.enc());");
165-
fmtln!(f, "vex.prefix = LegacyPrefix::{};", vex.pp.to_string());
166-
fmtln!(f, "vex.map = OpcodeMap::{};", vex.mmmmm.to_string());
167-
fmtln!(f, "vex.encode(buf, off);");
168-
}
169-
170173
fn generate_modrm_byte(&self, f: &mut Formatter, rex: &dsl::Rex) {
171174
use dsl::OperandKind::{FixedReg, Imm, Mem, Reg, RegMem};
172175

cranelift/assembler-x64/meta/src/generate/operand.rs

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -25,10 +25,10 @@ impl dsl::Operand {
2525
}
2626
r8 | r16 | r32 | r64 => format!("Gpr<R::{mut_}Gpr>"),
2727
rm8 | rm16 | rm32 | rm64 => format!("GprMem<R::{mut_}Gpr, R::ReadGpr>"),
28-
xmm1 | xmm2 | xmm3 | ymm1 | ymm2 | ymm3 | zmm1 | zmm2 | zmm3 => {
28+
xmm1 | xmm2 | xmm3 => {
2929
format!("Xmm<R::{mut_}Xmm>")
3030
}
31-
xmm_m32 | xmm_m64 | xmm_m128 | ymm_m256 | zmm_m512 => {
31+
xmm_m32 | xmm_m64 | xmm_m128 => {
3232
format!("XmmMem<R::{mut_}Xmm, R::ReadGpr>")
3333
}
3434
m8 | m16 | m32 | m64 => format!("Amode<R::ReadGpr>"),
@@ -55,8 +55,7 @@ impl dsl::Location {
5555
Some(size) => format!("self.{self}.to_string({size})"),
5656
None => unreachable!(),
5757
},
58-
xmm_m32 | xmm_m64 | xmm1 | xmm2 | xmm3 | ymm1 | ymm2 | ymm3 | zmm1 | zmm2 | zmm3
59-
| xmm_m128 | ymm_m256 | zmm_m512 | m8 | m16 | m32 | m64 => {
58+
xmm_m32 | xmm_m64 | xmm1 | xmm2 | xmm3 | xmm_m128 | m8 | m16 | m32 | m64 => {
6059
format!("self.{self}.to_string()")
6160
}
6261
}
@@ -75,8 +74,7 @@ impl dsl::Location {
7574
m8 | m16 | m32 | m64 => {
7675
panic!("no need to generate a size for memory-only access")
7776
}
78-
xmm_m32 | xmm_m64 | xmm1 | xmm2 | xmm3 | ymm1 | ymm2 | ymm3 | zmm1 | zmm2 | zmm3
79-
| xmm_m128 | ymm_m256 | zmm_m512 => {
77+
xmm_m32 | xmm_m64 | xmm1 | xmm2 | xmm3 | xmm_m128 => {
8078
panic!("no need to generate a size for XMM-sized access")
8179
}
8280
}

cranelift/assembler-x64/meta/src/instructions.rs

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,6 @@
11
//! Defines x64 instructions using the DSL.
22
33
mod add;
4-
mod addpd;
5-
mod addps;
64
mod and;
75
mod bitmanip;
86
mod cvt;
@@ -36,7 +34,5 @@ pub fn list() -> Vec<Inst> {
3634
all.extend(sqrt::list());
3735
all.extend(sub::list());
3836
all.extend(xor::list());
39-
all.extend(addpd::list());
40-
all.extend(addps::list());
4137
all
4238
}

cranelift/assembler-x64/meta/src/instructions/add.rs

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
1-
use crate::dsl::{Feature::*, Inst, Location::*};
2-
use crate::dsl::{align, fmt, inst, r, rex, rw, sxl, sxq};
1+
use crate::dsl::{align, fmt, inst, r, rex, rw, sxl, sxq, vex, w};
2+
use crate::dsl::{Feature::*, Inst, Location::*, VexLength::*, VexMMMMM::*, VexPP::*};
33

44
#[rustfmt::skip] // Keeps instructions on a single line.
55
pub fn list() -> Vec<Inst> {
@@ -77,5 +77,8 @@ pub fn list() -> Vec<Inst> {
7777
inst("paddusw", fmt("A", [rw(xmm1), r(align(xmm_m128))]), rex([0x66, 0x0F, 0xDD]).r(), _64b | compat | sse2),
7878
inst("phaddw", fmt("A", [rw(xmm1), r(align(xmm_m128))]), rex([0x66, 0x0F, 0x38, 0x01]).r(), _64b | compat | ssse3),
7979
inst("phaddd", fmt("A", [rw(xmm1), r(align(xmm_m128))]), rex([0x66, 0x0F, 0x38, 0x02]).r(), _64b | compat | ssse3),
80+
// Vex instructions
81+
inst("vaddps", fmt("B", [w(xmm1), r(xmm2), r(xmm_m128)]), vex(0x58).length(_128).mmmmm(_OF), _64b | compat | sse2),
82+
inst("vaddpd", fmt("B", [w(xmm1), r(xmm2), r(xmm_m128)]), vex(0x58).length(_128).pp(_66).mmmmm(_OF), _64b | compat | sse2)
8083
]
8184
}

cranelift/assembler-x64/meta/src/instructions/addpd.rs

Lines changed: 0 additions & 12 deletions
This file was deleted.

cranelift/assembler-x64/meta/src/instructions/addps.rs

Lines changed: 0 additions & 10 deletions
This file was deleted.

0 commit comments

Comments
 (0)