Expand x64 OperandSize to support 8 and 16-bit operands.

This is in preparation for refactoring all x64::Inst arms to use OperandSize. Current uses of OperandSize fall into two categories: 1. XMM operations which require 32/64 bit operands 2. Immediates which only care about 64-bit or not. Adds assertions to existing Inst constructors to check that they are passed valid sizes. This change also removes the implicit widening of 1 and 2 byte values to 4 bytes. from_bytes() is only used by category 2, so removing this behavior will not change any visible behavior. Overall this change should be a no-op.
2021-01-27 21:57:43 -08:00
parent 7aecd6dac9
commit b12d41bfe9
3 changed files with 50 additions and 48 deletions
--- a/cranelift/codegen/src/isa/x64/inst/args.rs
+++ b/cranelift/codegen/src/isa/x64/inst/args.rs
@@ -1323,6 +1323,8 @@ impl RoundImm {
 /// An operand's size in bits.
 #[derive(Clone, Copy, PartialEq)]
 pub enum OperandSize {
+    Size8,
+    Size16,
    Size32,
    Size64,
 }
@@ -1330,24 +1332,35 @@ pub enum OperandSize {
 impl OperandSize {
    pub(crate) fn from_bytes(num_bytes: u32) -> Self {
        match num_bytes {
-            1 | 2 | 4 => OperandSize::Size32,
+            1 => OperandSize::Size8,
+            2 => OperandSize::Size16,
+            4 => OperandSize::Size32,
            8 => OperandSize::Size64,
            _ => unreachable!(),
        }
    }

+    // Check that the value of self is one of the allowed sizes.
+    pub(crate) fn is_size(&self, sizes: &[Self]) -> bool {
+        for val in sizes.iter() {
+            if *self == *val {
+                return true;
+            }
+        }
+        false
+    }
+
    pub(crate) fn to_bytes(&self) -> u8 {
        match self {
+            Self::Size8 => 1,
+            Self::Size16 => 2,
            Self::Size32 => 4,
            Self::Size64 => 8,
        }
    }

    pub(crate) fn to_bits(&self) -> u8 {
-        match self {
-            Self::Size32 => 32,
-            Self::Size64 => 64,
-        }
+        self.to_bytes() * 8
    }
 }

--- a/cranelift/codegen/src/isa/x64/inst/emit.rs
+++ b/cranelift/codegen/src/isa/x64/inst/emit.rs
@@ -2063,6 +2063,7 @@ pub(crate) fn emit(
                        SseOpcode::Maxsd
                    },
                ),
+                _ => unreachable!(),
            };

            let inst = Inst::xmm_cmp_rm_r(cmp_op, RegMem::reg(*lhs), rhs_dst.to_reg());
@@ -2224,6 +2225,7 @@ pub(crate) fn emit(
            let rex = match dst_size {
                OperandSize::Size32 => RexFlags::clear_w(),
                OperandSize::Size64 => RexFlags::set_w(),
+                _ => unreachable!(),
            };

            let (src, dst) = if dst_first {
@@ -2252,6 +2254,7 @@ pub(crate) fn emit(
            let rex = match *src_size {
                OperandSize::Size32 => RexFlags::clear_w(),
                OperandSize::Size64 => RexFlags::set_w(),
+                _ => unreachable!(),
            };
            match src_e {
                RegMem::Reg { reg: reg_e } => {
@@ -2450,6 +2453,7 @@ pub(crate) fn emit(
            let (cast_op, cmp_op, trunc_op) = match src_size {
                OperandSize::Size64 => (SseOpcode::Movq, SseOpcode::Ucomisd, SseOpcode::Cvttsd2si),
                OperandSize::Size32 => (SseOpcode::Movd, SseOpcode::Ucomiss, SseOpcode::Cvttss2si),
+                _ => unreachable!(),
            };

            let done = sink.get_label();
@@ -2542,6 +2546,7 @@ pub(crate) fn emit(
                        let inst = Inst::imm(OperandSize::Size64, cst.bits(), *tmp_gpr);
                        inst.emit(sink, info, state);
                    }
+                    _ => unreachable!(),
                }

                let inst =
@@ -2622,28 +2627,28 @@ pub(crate) fn emit(

            assert_ne!(tmp_xmm, src, "tmp_xmm clobbers src!");

-            let (sub_op, cast_op, cmp_op, trunc_op) = if *src_size == OperandSize::Size64 {
-                (
-                    SseOpcode::Subsd,
-                    SseOpcode::Movq,
-                    SseOpcode::Ucomisd,
-                    SseOpcode::Cvttsd2si,
-                )
-            } else {
-                (
+            let (sub_op, cast_op, cmp_op, trunc_op) = match src_size {
+                OperandSize::Size32 => (
                    SseOpcode::Subss,
                    SseOpcode::Movd,
                    SseOpcode::Ucomiss,
                    SseOpcode::Cvttss2si,
-                )
+                ),
+                OperandSize::Size64 => (
+                    SseOpcode::Subsd,
+                    SseOpcode::Movq,
+                    SseOpcode::Ucomisd,
+                    SseOpcode::Cvttsd2si,
+                ),
+                _ => unreachable!(),
            };

            let done = sink.get_label();

-            let cst = if *src_size == OperandSize::Size64 {
-                Ieee64::pow2(dst_size.to_bits() - 1).bits()
-            } else {
-                Ieee32::pow2(dst_size.to_bits() - 1).bits() as u64
+            let cst = match src_size {
+                OperandSize::Size32 => Ieee32::pow2(dst_size.to_bits() - 1).bits() as u64,
+                OperandSize::Size64 => Ieee64::pow2(dst_size.to_bits() - 1).bits(),
+                _ => unreachable!(),
            };

            let inst = Inst::imm(*src_size, cst, *tmp_gpr);
--- a/cranelift/codegen/src/isa/x64/inst/mod.rs
+++ b/cranelift/codegen/src/isa/x64/inst/mod.rs
@@ -717,6 +717,7 @@ impl Inst {
    ) -> Inst {
        debug_assert!(src.get_class() == RegClass::V128);
        debug_assert!(dst.to_reg().get_class() == RegClass::I64);
+        debug_assert!(dst_size.is_size(&[OperandSize::Size32, OperandSize::Size64]));
        Inst::XmmToGpr {
            op,
            src,
@@ -732,6 +733,7 @@ impl Inst {
        dst: Writable<Reg>,
    ) -> Inst {
        src.assert_regclass_is(RegClass::I64);
+        debug_assert!(src_size.is_size(&[OperandSize::Size32, OperandSize::Size64]));
        debug_assert!(dst.to_reg().get_class() == RegClass::V128);
        Inst::GprToXmm {
            op,
@@ -776,6 +778,8 @@ impl Inst {
        tmp_gpr: Writable<Reg>,
        tmp_xmm: Writable<Reg>,
    ) -> Inst {
+        debug_assert!(src_size.is_size(&[OperandSize::Size32, OperandSize::Size64]));
+        debug_assert!(dst_size.is_size(&[OperandSize::Size32, OperandSize::Size64]));
        debug_assert!(src.to_reg().get_class() == RegClass::V128);
        debug_assert!(tmp_xmm.to_reg().get_class() == RegClass::V128);
        debug_assert!(tmp_gpr.to_reg().get_class() == RegClass::I64);
@@ -800,6 +804,8 @@ impl Inst {
        tmp_gpr: Writable<Reg>,
        tmp_xmm: Writable<Reg>,
    ) -> Inst {
+        debug_assert!(src_size.is_size(&[OperandSize::Size32, OperandSize::Size64]));
+        debug_assert!(dst_size.is_size(&[OperandSize::Size32, OperandSize::Size64]));
        debug_assert!(src.to_reg().get_class() == RegClass::V128);
        debug_assert!(tmp_xmm.to_reg().get_class() == RegClass::V128);
        debug_assert!(tmp_gpr.to_reg().get_class() == RegClass::I64);
@@ -821,6 +827,7 @@ impl Inst {
        lhs: Reg,
        rhs_dst: Writable<Reg>,
    ) -> Inst {
+        debug_assert!(size.is_size(&[OperandSize::Size32, OperandSize::Size64]));
        debug_assert_eq!(lhs.get_class(), RegClass::V128);
        debug_assert_eq!(rhs_dst.to_reg().get_class(), RegClass::V128);
        Inst::XmmMinMaxSeq {
@@ -1416,11 +1423,7 @@ impl PrettyPrint for Inst {
                    } else {
                        "xmm max seq ".to_string()
                    },
-                    match size {
-                        OperandSize::Size32 => "f32",
-                        OperandSize::Size64 => "f64",
-                    }
-                    .into()
+                    format!("f{}", size.to_bits())
                ),
                show_ireg_sized(*lhs, mb_rru, 8),
                show_ireg_sized(rhs_dst.to_reg(), mb_rru, 8),
@@ -1459,10 +1462,7 @@ impl PrettyPrint for Inst {
                dst,
                dst_size,
            } => {
-                let dst_size = match dst_size {
-                    OperandSize::Size32 => 4,
-                    OperandSize::Size64 => 8,
-                };
+                let dst_size = dst_size.to_bytes();
                format!(
                    "{} {}, {}",
                    ljustify(op.to_string()),
@@ -1512,16 +1512,8 @@ impl PrettyPrint for Inst {
                "{} {}, {}",
                ljustify(format!(
                    "cvt_float{}_to_sint{}_seq",
-                    if *src_size == OperandSize::Size64 {
-                        "64"
-                    } else {
-                        "32"
-                    },
-                    if *dst_size == OperandSize::Size64 {
-                        "64"
-                    } else {
-                        "32"
-                    }
+                    src_size.to_bits(),
+                    dst_size.to_bits()
                )),
                show_ireg_sized(src.to_reg(), mb_rru, 8),
                show_ireg_sized(dst.to_reg(), mb_rru, dst_size.to_bytes()),
@@ -1537,16 +1529,8 @@ impl PrettyPrint for Inst {
                "{} {}, {}",
                ljustify(format!(
                    "cvt_float{}_to_uint{}_seq",
-                    if *src_size == OperandSize::Size64 {
-                        "64"
-                    } else {
-                        "32"
-                    },
-                    if *dst_size == OperandSize::Size64 {
-                        "64"
-                    } else {
-                        "32"
-                    }
+                    src_size.to_bits(),
+                    dst_size.to_bits()
                )),
                show_ireg_sized(src.to_reg(), mb_rru, 8),
                show_ireg_sized(dst.to_reg(), mb_rru, dst_size.to_bytes()),