Cranelift/x64: fix register allocator metadata for 8-bit divides. (by…

…tecodealliance#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.
cfallin · Jun 27, 2022 · 2f5bebd · 2f5bebd
1 parent d421fad
commit 2f5bebd
Show file tree

Hide file tree

Showing 3 changed files with 15 additions and 6 deletions.
diff --git a/cranelift/codegen/src/isa/x64/inst/emit.rs b/cranelift/codegen/src/isa/x64/inst/emit.rs
@@ -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),

diff --git a/cranelift/codegen/src/isa/x64/inst/emit_tests.rs b/cranelift/codegen/src/isa/x64/inst/emit_tests.rs
@@ -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",
     ));
 
     // ========================================================

diff --git a/cranelift/codegen/src/isa/x64/inst/mod.rs b/cranelift/codegen/src/isa/x64/inst/mod.rs
@@ -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!(
                     "{} {}, {}, {}, {}, {}",
@@ -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 {