Skip to content

Commit

Permalink
Cherry-pick #4332 to 0.38 branch. (#4336)
Browse files Browse the repository at this point in the history
* Cranelift/x64: fix register allocator metadata for 8-bit divides. (#4332)

`idiv` on x86-64 only reads `rdx`/`edx`/`dx`/`dl` for divides with width
greater than 8 bits; for an 8-bit divide, it reads the whole 16-bit
divisor from `ax`, as our CISC ancestors intended. This PR fixes the
metadata to avoid a regalloc panic (due to undefined `rdx`) in this
case. Does not affect Wasmtime or other Wasm-frontend embedders.

* Update RELEASES.md.
  • Loading branch information
cfallin committed Jun 27, 2022
1 parent d421fad commit 94fd5b0
Show file tree
Hide file tree
Showing 4 changed files with 23 additions and 6 deletions.
8 changes: 8 additions & 0 deletions RELEASES.md
Expand Up @@ -13,6 +13,14 @@ Released 2022-06-27.
including Wasmtime.
[regalloc2#60](https://github.com/bytecodealliance/regalloc2/pull/60)

* A bug in the 8-bit lowering of integer division on x86-64 was fixed in
Cranelift that could cause a register allocator panic due to an undefined
value in a register. (The divide instruction does not take a register `rdx`
as a source when 8 bits but the metadata incorrectly claimed it did.) No
impact on Wasm/Wasmtime users, and impact on direct Cranelift embedders
limited to compilation panics.
[#4332](https://github.com/bytecodealliance/wasmtime/pull/4332)

--------------------------------------------------------------------------------

## 0.38.0
Expand Down
6 changes: 4 additions & 2 deletions cranelift/codegen/src/isa/x64/inst/emit.rs
Expand Up @@ -385,13 +385,15 @@ pub(crate) fn emit(
dst_remainder,
} => {
let dividend_lo = allocs.next(dividend_lo.to_reg());
let dividend_hi = allocs.next(dividend_hi.to_reg());
let dst_quotient = allocs.next(dst_quotient.to_reg().to_reg());
let dst_remainder = allocs.next(dst_remainder.to_reg().to_reg());
debug_assert_eq!(dividend_lo, regs::rax());
debug_assert_eq!(dividend_hi, regs::rdx());
debug_assert_eq!(dst_quotient, regs::rax());
debug_assert_eq!(dst_remainder, regs::rdx());
if size.to_bits() > 8 {
let dividend_hi = allocs.next(dividend_hi.to_reg());
debug_assert_eq!(dividend_hi, regs::rdx());
}

let (opcode, prefix) = match size {
OperandSize::Size8 => (0xF6, LegacyPrefixes::None),
Expand Down
4 changes: 2 additions & 2 deletions cranelift/codegen/src/isa/x64/inst/emit_tests.rs
Expand Up @@ -1750,12 +1750,12 @@ fn test_x64_emit() {
insns.push((
Inst::div(OperandSize::Size8, false, RegMem::reg(regs::rax())),
"F6F0",
"div %al, %dl, %al, %al, %dl",
"div %al, (none), %al, %al, %dl",
));
insns.push((
Inst::div(OperandSize::Size8, false, RegMem::reg(regs::rsi())),
"40F6F6",
"div %al, %dl, %sil, %al, %dl",
"div %al, (none), %sil, %al, %dl",
));

// ========================================================
Expand Down
11 changes: 9 additions & 2 deletions cranelift/codegen/src/isa/x64/inst/mod.rs
Expand Up @@ -966,11 +966,15 @@ impl PrettyPrint for Inst {
dst_remainder,
} => {
let dividend_lo = pretty_print_reg(dividend_lo.to_reg(), size.to_bytes(), allocs);
let dividend_hi = pretty_print_reg(dividend_hi.to_reg(), size.to_bytes(), allocs);
let dst_quotient =
pretty_print_reg(dst_quotient.to_reg().to_reg(), size.to_bytes(), allocs);
let dst_remainder =
pretty_print_reg(dst_remainder.to_reg().to_reg(), size.to_bytes(), allocs);
let dividend_hi = if size.to_bits() > 8 {
pretty_print_reg(dividend_hi.to_reg(), size.to_bytes(), allocs)
} else {
"(none)".to_string()
};
let divisor = divisor.pretty_print(size.to_bytes(), allocs);
format!(
"{} {}, {}, {}, {}, {}",
Expand Down Expand Up @@ -1718,12 +1722,15 @@ fn x64_get_operands<F: Fn(VReg) -> VReg>(inst: &Inst, collector: &mut OperandCol
dividend_hi,
dst_quotient,
dst_remainder,
size,
..
} => {
collector.reg_fixed_use(dividend_lo.to_reg(), regs::rax());
collector.reg_fixed_use(dividend_hi.to_reg(), regs::rdx());
collector.reg_fixed_def(dst_quotient.to_writable_reg(), regs::rax());
collector.reg_fixed_def(dst_remainder.to_writable_reg(), regs::rdx());
if size.to_bits() > 8 {
collector.reg_fixed_use(dividend_hi.to_reg(), regs::rdx());
}
divisor.get_operands(collector);
}
Inst::MulHi {
Expand Down

0 comments on commit 94fd5b0

Please sign in to comment.