Address review comments;

cranelift/codegen/src/isa/x64/abi.rs

@@ -94,10 +94,10 @@ fn in_vec_reg(ty: types::Type) -> bool {
 }
 
 fn get_intreg_for_arg_systemv(call_conv: &CallConv, idx: usize) -> Option<Reg> {
-    assert!(match call_conv {
-        CallConv::SystemV | CallConv::BaldrdashSystemV => true,
-        _ => false,
-    });
+    match call_conv {
+        CallConv::Fast | CallConv::Cold | CallConv::SystemV | CallConv::BaldrdashSystemV => {}
+        _ => panic!("int args only supported for SysV calling convention"),
+    };
     match idx {
         0 => Some(regs::rdi()),
         1 => Some(regs::rsi()),
@@ -110,10 +110,10 @@ fn get_intreg_for_arg_systemv(call_conv: &CallConv, idx: usize) -> Option<Reg> {
 }
 
 fn get_fltreg_for_arg_systemv(call_conv: &CallConv, idx: usize) -> Option<Reg> {
-    assert!(match call_conv {
-        CallConv::SystemV | CallConv::BaldrdashSystemV => true,
-        _ => false,
-    });
+    match call_conv {
+        CallConv::Fast | CallConv::Cold | CallConv::SystemV | CallConv::BaldrdashSystemV => {}
+        _ => panic!("float args only supported for SysV calling convention"),
+    };
     match idx {
         0 => Some(regs::xmm0()),
         1 => Some(regs::xmm1()),

cranelift/codegen/src/isa/x64/inst/args.rs

@@ -282,32 +282,32 @@ impl fmt::Debug for AluRmiROpcode {
     }
 }
 
-impl ToString for AluRmiROpcode {
-    fn to_string(&self) -> String {
-        format!("{:?}", self)
+impl fmt::Display for AluRmiROpcode {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        fmt::Debug::fmt(self, f)
     }
 }
 
 #[derive(Clone, PartialEq)]
-pub enum ReadOnlyGprRmROpcode {
+pub enum UnaryRmROpcode {
     /// Bit-scan reverse.
     Bsr,
     /// Bit-scan forward.
     Bsf,
 }
 
-impl fmt::Debug for ReadOnlyGprRmROpcode {
+impl fmt::Debug for UnaryRmROpcode {
     fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result {
         match self {
-            ReadOnlyGprRmROpcode::Bsr => write!(fmt, "bsr"),
-            ReadOnlyGprRmROpcode::Bsf => write!(fmt, "bsf"),
+            UnaryRmROpcode::Bsr => write!(fmt, "bsr"),
+            UnaryRmROpcode::Bsf => write!(fmt, "bsf"),
         }
     }
 }
 
-impl ToString for ReadOnlyGprRmROpcode {
-    fn to_string(&self) -> String {
-        format!("{:?}", self)
+impl fmt::Display for UnaryRmROpcode {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        fmt::Debug::fmt(self, f)
     }
 }
 
@@ -468,9 +468,9 @@ impl fmt::Debug for SseOpcode {
     }
 }
 
-impl ToString for SseOpcode {
-    fn to_string(&self) -> String {
-        format!("{:?}", self)
+impl fmt::Display for SseOpcode {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        fmt::Debug::fmt(self, f)
     }
 }
 
@@ -519,38 +519,65 @@ impl fmt::Debug for ExtMode {
     }
 }
 
-impl ToString for ExtMode {
-    fn to_string(&self) -> String {
-        format!("{:?}", self)
+impl fmt::Display for ExtMode {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        fmt::Debug::fmt(self, f)
     }
 }
 
-/// These indicate the form of a scalar shift: left, signed right, unsigned right.
+/// These indicate the form of a scalar shift/rotate: left, signed right, unsigned right.
 #[derive(Clone)]
 pub enum ShiftKind {
-    Left,
-    RightZ,
-    RightS,
+    ShiftLeft,
+    /// Inserts zeros in the most significant bits.
+    ShiftRightLogical,
+    /// Replicates the sign bit in the most significant bits.
+    ShiftRightArithmetic,
     RotateLeft,
     RotateRight,
 }
 
 impl fmt::Debug for ShiftKind {
     fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result {
         let name = match self {
-            ShiftKind::Left => "shl",
-            ShiftKind::RightZ => "shr",
-            ShiftKind::RightS => "sar",
+            ShiftKind::ShiftLeft => "shl",
+            ShiftKind::ShiftRightLogical => "shr",
+            ShiftKind::ShiftRightArithmetic => "sar",
             ShiftKind::RotateLeft => "rol",
             ShiftKind::RotateRight => "ror",
         };
         write!(fmt, "{}", name)
     }
 }
 
-impl ToString for ShiftKind {
-    fn to_string(&self) -> String {
-        format!("{:?}", self)
+impl fmt::Display for ShiftKind {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        fmt::Debug::fmt(self, f)
+    }
+}
+
+/// What kind of division or remainer instruction this is?
+#[derive(Clone)]
+pub enum DivOrRemKind {
+    SignedDiv,
+    UnsignedDiv,
+    SignedRem,
+    UnsignedRem,
+}
+
+impl DivOrRemKind {
+    pub(crate) fn is_signed(&self) -> bool {
+        match self {
+            DivOrRemKind::SignedDiv | DivOrRemKind::SignedRem => true,
+            _ => false,
+        }
+    }
+
+    pub(crate) fn is_div(&self) -> bool {
+        match self {
+            DivOrRemKind::SignedDiv | DivOrRemKind::UnsignedDiv => true,
+            _ => false,
+        }
     }
 }
 
@@ -665,9 +692,9 @@ impl fmt::Debug for CC {
     }
 }
 
-impl ToString for CC {
-    fn to_string(&self) -> String {
-        format!("{:?}", self)
+impl fmt::Display for CC {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        fmt::Debug::fmt(self, f)
     }
 }
 

cranelift/codegen/src/isa/x64/inst/emit.rs

@@ -556,7 +556,7 @@ pub(crate) fn emit(
             }
         }
 
-        Inst::ReadOnly_Gpr_Rm_R { size, op, src, dst } => {
+        Inst::UnaryRmR { size, op, src, dst } => {
             let (prefix, rex_flags) = match size {
                 2 => (LegacyPrefix::_66, RexFlags::clear_w()),
                 4 => (LegacyPrefix::None, RexFlags::clear_w()),
@@ -565,8 +565,8 @@ pub(crate) fn emit(
             };
 
             let (opcode, num_opcodes) = match op {
-                ReadOnlyGprRmROpcode::Bsr => (0x0fbd, 2),
-                ReadOnlyGprRmROpcode::Bsf => (0x0fbc, 2),
+                UnaryRmROpcode::Bsr => (0x0fbd, 2),
+                UnaryRmROpcode::Bsf => (0x0fbc, 2),
             };
 
             match src {
@@ -661,8 +661,7 @@ pub(crate) fn emit(
         }
 
         Inst::CheckedDivOrRemSeq {
-            is_div,
-            is_signed,
+            kind,
             size,
             divisor,
             loc,
@@ -704,7 +703,7 @@ pub(crate) fn emit(
             let inst = Inst::trap_if(CC::Z, TrapCode::IntegerDivisionByZero, *loc);
             inst.emit(sink, flags, state);
 
-            let (do_op, done_label) = if *is_signed {
+            let (do_op, done_label) = if kind.is_signed() {
                 // Now check if the divisor is -1.
                 let inst = Inst::cmp_rmi_r(*size, RegMemImm::imm(0xffffffff), *divisor);
                 inst.emit(sink, flags, state);
@@ -715,7 +714,7 @@ pub(crate) fn emit(
                 one_way_jmp(sink, CC::NZ, do_op);
 
                 // Here, divisor == -1.
-                if !*is_div {
+                if !kind.is_div() {
                     // x % -1 = 0; put the result into the destination, $rdx.
                     let done_label = sink.get_label();
 
@@ -756,7 +755,7 @@ pub(crate) fn emit(
             }
 
             // Fill in the high parts:
-            if *is_signed {
+            if kind.is_signed() {
                 // sign-extend the sign-bit of rax into rdx, for signed opcodes.
                 let inst = Inst::sign_extend_rax_to_rdx(*size);
                 inst.emit(sink, flags, state);
@@ -766,7 +765,7 @@ pub(crate) fn emit(
                 inst.emit(sink, flags, state);
             }
 
-            let inst = Inst::div(*size, *is_signed, RegMem::reg(*divisor), *loc);
+            let inst = Inst::div(*size, kind.is_signed(), RegMem::reg(*divisor), *loc);
             inst.emit(sink, flags, state);
 
             // Lowering takes care of moving the result back into the right register, see comment
@@ -1047,9 +1046,9 @@ pub(crate) fn emit(
             let subopcode = match kind {
                 ShiftKind::RotateLeft => 0,
                 ShiftKind::RotateRight => 1,
-                ShiftKind::Left => 4,
-                ShiftKind::RightZ => 5,
-                ShiftKind::RightS => 7,
+                ShiftKind::ShiftLeft => 4,
+                ShiftKind::ShiftRightLogical => 5,
+                ShiftKind::ShiftRightArithmetic => 7,
             };
 
             let rex = if *is_64 {
@@ -1550,6 +1549,7 @@ pub(crate) fn emit(
             srcloc,
         } => {
             // The full address can be encoded in the register, with a relocation.
+            // Generates: movabsq $name, %dst
             let enc_dst = int_reg_enc(dst.to_reg());
             sink.put1(0x48 | ((enc_dst >> 3) & 1));
             sink.put1(0xB8 | (enc_dst & 7));