Skip to content
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

Jump to bottom

Cranelift: Improve codegen of store_imm on x64 #6979

Merged
merged 3 commits into from
Sep 11, 2023
Merged

Cranelift: Improve codegen of store_imm on x64 #6979

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
918e309
Improve lowering of store_imm on x64
mchesser Sep 8, 2023
45f3131
Ensure that the MovImmM operand fits in an i32 and add tests.
mchesser Sep 10, 2023
832448f
Update winch to handle MovImmM change
mchesser Sep 10, 2023
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
7 changes: 6 additions & 1 deletion cranelift/codegen/src/isa/x64/inst.isle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,7 @@

;; Immediate store.
(MovImmM (size OperandSize)
(simm64 u64)
(simm32 i32)
(dst SyntheticAmode))

;; Integer stores: mov (b w l q) reg addr.
Expand Down Expand Up @@ -2368,6 +2368,11 @@
(let ((size OperandSize (raw_operand_size_of_type ty)))
(SideEffectNoResult.Inst (MInst.MovRM size data addr))))

(decl x64_movimm_m (Type SyntheticAmode i32) SideEffectNoResult)
(rule (x64_movimm_m ty addr imm)
(let ((size OperandSize (raw_operand_size_of_type ty)))
(SideEffectNoResult.Inst (MInst.MovImmM size imm addr))))

(decl xmm_movrm (SseOpcode SyntheticAmode Xmm) SideEffectNoResult)
(rule (xmm_movrm op addr data)
(SideEffectNoResult.Inst (MInst.XmmMovRM op data addr)))
Expand Down
12 changes: 3 additions & 9 deletions cranelift/codegen/src/isa/x64/inst/emit.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -797,7 +797,7 @@ pub(crate) fn emit(
}
}

Inst::MovImmM { size, simm64, dst } => {
Inst::MovImmM { size, simm32, dst } => {
let dst = &dst.finalize(state, sink).with_allocs(allocs);
let default_rex = RexFlags::clear_w();
let default_opcode = 0xC7;
Expand All @@ -810,13 +810,7 @@ pub(crate) fn emit(
// operand is a memory operand, not a possibly 8-bit register.
OperandSize::Size8 => (0xC6, default_rex, bytes, prefix),
OperandSize::Size16 => (0xC7, default_rex, bytes, LegacyPrefixes::_66),
OperandSize::Size64 => {
if !low32_will_sign_extend_to_64(*simm64) {
panic!("Immediate-to-memory moves require immediate operand to sign-extend to 64 bits.");
}

(default_opcode, RexFlags::from(*size), bytes, prefix)
}
OperandSize::Size64 => (default_opcode, RexFlags::from(*size), bytes, prefix),

_ => (default_opcode, default_rex, bytes, prefix),
};
Expand All @@ -826,7 +820,7 @@ pub(crate) fn emit(
// 32-bit C7 /0 id
// 64-bit REX.W C7 /0 id
emit_std_enc_mem(sink, prefix, opcode, 1, /*subopcode*/ 0, dst, rex, 0);
emit_simm(sink, size, *simm64 as u32);
emit_simm(sink, size, *simm32 as u32);
}

Inst::MovRR { size, src, dst } => {
Expand Down
24 changes: 12 additions & 12 deletions cranelift/codegen/src/isa/x64/inst/emit_tests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2856,7 +2856,7 @@ fn test_x64_emit() {
insns.push((
Inst::MovImmM {
size: OperandSize::Size8,
simm64: i8::MIN as u64,
simm32: i8::MIN as i32,
dst: Amode::imm_reg(99, rax).into(),
},
"C6406380",
Expand All @@ -2866,7 +2866,7 @@ fn test_x64_emit() {
insns.push((
Inst::MovImmM {
size: OperandSize::Size8,
simm64: i8::MAX as u64,
simm32: i8::MAX as i32,
dst: Amode::imm_reg(99, r8).into(),
},
"41C640637F",
Expand All @@ -2876,7 +2876,7 @@ fn test_x64_emit() {
insns.push((
Inst::MovImmM {
size: OperandSize::Size16,
simm64: i16::MIN as u64,
simm32: i16::MIN as i32,
dst: Amode::imm_reg(99, rcx).into(),
},
"66C741630080",
Expand All @@ -2886,7 +2886,7 @@ fn test_x64_emit() {
insns.push((
Inst::MovImmM {
size: OperandSize::Size16,
simm64: i16::MAX as u64,
simm32: i16::MAX as i32,
dst: Amode::imm_reg(99, r9).into(),
},
"6641C74163FF7F",
Expand All @@ -2896,7 +2896,7 @@ fn test_x64_emit() {
insns.push((
Inst::MovImmM {
size: OperandSize::Size32,
simm64: i32::MIN as u64,
simm32: i32::MIN,
dst: Amode::imm_reg(99, rdx).into(),
},
"C7426300000080",
Expand All @@ -2906,7 +2906,7 @@ fn test_x64_emit() {
insns.push((
Inst::MovImmM {
size: OperandSize::Size32,
simm64: i32::MAX as u64,
simm32: i32::MAX,
dst: Amode::imm_reg(99, r10).into(),
},
"41C74263FFFFFF7F",
Expand All @@ -2916,7 +2916,7 @@ fn test_x64_emit() {
insns.push((
Inst::MovImmM {
size: OperandSize::Size64,
simm64: i32::MIN as u64,
simm32: i32::MIN,
dst: Amode::imm_reg(99, rbx).into(),
},
"48C7436300000080",
Expand All @@ -2926,7 +2926,7 @@ fn test_x64_emit() {
insns.push((
Inst::MovImmM {
size: OperandSize::Size64,
simm64: i32::MAX as u64,
simm32: i32::MAX,
dst: Amode::imm_reg(99, r11).into(),
},
"49C74363FFFFFF7F",
Expand All @@ -2936,7 +2936,7 @@ fn test_x64_emit() {
insns.push((
Inst::MovImmM {
size: OperandSize::Size8,
simm64: 0u64,
simm32: 0i32,
dst: Amode::imm_reg(99, rsp).into(),
},
"C644246300",
Expand All @@ -2946,7 +2946,7 @@ fn test_x64_emit() {
insns.push((
Inst::MovImmM {
size: OperandSize::Size16,
simm64: 0u64,
simm32: 0i32,
dst: Amode::imm_reg(99, r12).into(),
},
"6641C74424630000",
Expand All @@ -2956,7 +2956,7 @@ fn test_x64_emit() {
insns.push((
Inst::MovImmM {
size: OperandSize::Size32,
simm64: 0u64,
simm32: 0i32,
dst: Amode::imm_reg(99, rbp).into(),
},
"C7456300000000",
Expand All @@ -2966,7 +2966,7 @@ fn test_x64_emit() {
insns.push((
Inst::MovImmM {
size: OperandSize::Size64,
simm64: 0u64,
simm32: 0i32,
dst: Amode::imm_reg(99, r13).into(),
},
"49C7456300000000",
Expand Down
10 changes: 5 additions & 5 deletions cranelift/codegen/src/isa/x64/inst/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1369,14 +1369,14 @@ impl PrettyPrint for Inst {
}
}

Inst::MovImmM { size, simm64, dst } => {
Inst::MovImmM { size, simm32, dst } => {
let dst = dst.pretty_print(size.to_bytes(), allocs);
let suffix = suffix_bwlq(*size);
let imm = match *size {
OperandSize::Size8 => ((*simm64 as u8) as i8).to_string(),
OperandSize::Size16 => ((*simm64 as u16) as i16).to_string(),
OperandSize::Size32 => ((*simm64 as u32) as i32).to_string(),
OperandSize::Size64 => (*simm64 as i64).to_string(),
OperandSize::Size8 => ((*simm32 as u8) as i8).to_string(),
OperandSize::Size16 => ((*simm32 as u16) as i16).to_string(),
OperandSize::Size32 => simm32.to_string(),
OperandSize::Size64 => (*simm32 as i64).to_string(),
};
let op = ljustify2("mov".to_string(), suffix);
format!("{op} ${imm}, {dst}")
Expand Down
5 changes: 5 additions & 0 deletions cranelift/codegen/src/isa/x64/lower.isle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2912,6 +2912,11 @@
(side_effect
(x64_movrm $I32 (to_amode flags address offset) value)))

;; IMM stores
(rule 2 (lower (store flags (has_type (fits_in_64 ty) (iconst (simm32 value))) address offset))
(side_effect
(x64_movimm_m ty (to_amode flags address offset) value)))

;; F32 stores of values in XMM registers.
(rule 1 (lower (store flags
value @ (value_type $F32)
Expand Down
201 changes: 201 additions & 0 deletions cranelift/filetests/filetests/isa/x64/store-imm.clif
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,201 @@
test compile precise-output
target x86_64

function %store_imm8(i64 sret) {
block0(v0: i64):
v1 = iconst.i8 0x12
store v1, v0
return
}

; VCode:
; pushq %rbp
; movq %rsp, %rbp
; block0:
; movb $18, 0(%rdi)
; movq %rdi, %rax
; movq %rbp, %rsp
; popq %rbp
; ret
;
; Disassembled:
; block0: ; offset 0x0
; pushq %rbp
; movq %rsp, %rbp
; block1: ; offset 0x4
; movb $0x12, (%rdi) ; trap: heap_oob
; movq %rdi, %rax
; movq %rbp, %rsp
; popq %rbp
; retq

function %store_imm16(i64 sret) {
block0(v0: i64):
v1 = iconst.i16 0x1234
store v1, v0
return
}

; VCode:
; pushq %rbp
; movq %rsp, %rbp
; block0:
; movw $4660, 0(%rdi)
; movq %rdi, %rax
; movq %rbp, %rsp
; popq %rbp
; ret
;
; Disassembled:
; block0: ; offset 0x0
; pushq %rbp
; movq %rsp, %rbp
; block1: ; offset 0x4
; movw $0x1234, (%rdi) ; trap: heap_oob
; movq %rdi, %rax
; movq %rbp, %rsp
; popq %rbp
; retq

function %store_imm32(i64 sret) {
block0(v0: i64):
v1 = iconst.i32 0x1234_5678
store v1, v0
return
}

; VCode:
; pushq %rbp
; movq %rsp, %rbp
; block0:
; movl $305419896, 0(%rdi)
; movq %rdi, %rax
; movq %rbp, %rsp
; popq %rbp
; ret
;
; Disassembled:
; block0: ; offset 0x0
; pushq %rbp
; movq %rsp, %rbp
; block1: ; offset 0x4
; movl $0x12345678, (%rdi) ; trap: heap_oob
; movq %rdi, %rax
; movq %rbp, %rsp
; popq %rbp
; retq

function %store_imm64(i64 sret) {
block0(v0: i64):
v1 = iconst.i64 0x1234_5678
store v1, v0
return
}

; VCode:
; pushq %rbp
; movq %rsp, %rbp
; block0:
; movq $305419896, 0(%rdi)
; movq %rdi, %rax
; movq %rbp, %rsp
; popq %rbp
; ret
;
; Disassembled:
; block0: ; offset 0x0
; pushq %rbp
; movq %rsp, %rbp
; block1: ; offset 0x4
; movq $0x12345678, (%rdi) ; trap: heap_oob
; movq %rdi, %rax
; movq %rbp, %rsp
; popq %rbp
; retq

function %store_max_i32_imm64(i64 sret) {
block0(v0: i64):
v1 = iconst.i64 0x7fff_ffff
store v1, v0
return
}

; VCode:
; pushq %rbp
; movq %rsp, %rbp
; block0:
; movq $2147483647, 0(%rdi)
; movq %rdi, %rax
; movq %rbp, %rsp
; popq %rbp
; ret
;
; Disassembled:
; block0: ; offset 0x0
; pushq %rbp
; movq %rsp, %rbp
; block1: ; offset 0x4
; movq $0x7fffffff, (%rdi) ; trap: heap_oob
; movq %rdi, %rax
; movq %rbp, %rsp
; popq %rbp
; retq

function %store_min_i32_imm64(i64 sret) {
block0(v0: i64):
v1 = iconst.i64 -2_147_483_648
store v1, v0
return
}

; VCode:
; pushq %rbp
; movq %rsp, %rbp
; block0:
; movq $-2147483648, 0(%rdi)
; movq %rdi, %rax
; movq %rbp, %rsp
; popq %rbp
; ret
;
; Disassembled:
; block0: ; offset 0x0
; pushq %rbp
; movq %rsp, %rbp
; block1: ; offset 0x4
; movq $18446744071562067968, (%rdi) ; trap: heap_oob
; movq %rdi, %rax
; movq %rbp, %rsp
; popq %rbp
; retq

function %store_max_i64_imm64(i64 sret) {
block0(v0: i64):
v1 = iconst.i64 0x7fff_ffff_ffff_ffff
store v1, v0
return
}

; VCode:
; pushq %rbp
; movq %rsp, %rbp
; block0:
; movabsq $9223372036854775807, %rax
; movq %rax, 0(%rdi)
; movq %rdi, %rax
; movq %rbp, %rsp
; popq %rbp
; ret
;
; Disassembled:
; block0: ; offset 0x0
; pushq %rbp
; movq %rsp, %rbp
; block1: ; offset 0x4
; movabsq $0x7fffffffffffffff, %rax
; movq %rax, (%rdi) ; trap: heap_oob
; movq %rdi, %rax
; movq %rbp, %rsp
; popq %rbp
; retq

Loading