x64: match multiple ISA requirements before emitting

Because there are instructions that are present in more than one ISA feature set, we need to see if any of the ISA requirements match before emitting. This change includes the `VPABSQ` instruction as an example, which is present in both `AVX512F` and `AVX512VL`.

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

@@ -10,6 +10,7 @@ use regalloc::{
     PrettyPrint, PrettyPrintSized, RealRegUniverse, Reg, RegClass, RegUsageCollector,
     RegUsageMapper, Writable,
 };
+use smallvec::{smallvec, SmallVec};
 use std::fmt;
 use std::string::String;
 
@@ -411,12 +412,12 @@ pub enum UnaryRmROpcode {
 }
 
 impl UnaryRmROpcode {
-    pub(crate) fn available_from(&self) -> Option<InstructionSet> {
+    pub(crate) fn available_from(&self) -> SmallVec<[InstructionSet; 2]> {
         match self {
-            UnaryRmROpcode::Bsr | UnaryRmROpcode::Bsf => None,
+            UnaryRmROpcode::Bsr | UnaryRmROpcode::Bsf => smallvec![],
-            UnaryRmROpcode::Lzcnt => Some(InstructionSet::Lzcnt),
+            UnaryRmROpcode::Lzcnt => smallvec![InstructionSet::Lzcnt],
-            UnaryRmROpcode::Tzcnt => Some(InstructionSet::BMI1),
+            UnaryRmROpcode::Tzcnt => smallvec![InstructionSet::BMI1],
-            UnaryRmROpcode::Popcnt => Some(InstructionSet::Popcnt),
+            UnaryRmROpcode::Popcnt => smallvec![InstructionSet::Popcnt],
         }
     }
 }
@@ -447,6 +448,7 @@ pub enum CmpOpcode {
     Test,
 }
 
+#[derive(Debug)]
 pub(crate) enum InstructionSet {
     SSE,
     SSE2,
@@ -458,6 +460,10 @@ pub(crate) enum InstructionSet {
     BMI1,
     #[allow(dead_code)] // never constructed (yet).
     BMI2,
+    #[allow(dead_code)]
+    AVX512F,
+    #[allow(dead_code)]
+    AVX512VL,
 }
 
 /// Some SSE operations requiring 2 operands r/m and r.
@@ -987,6 +993,37 @@ impl fmt::Display for SseOpcode {
     }
 }
 
+#[derive(Clone)]
+pub enum Avx512Opcode {
+    #[allow(dead_code)]
+    Vpabsq,
+}
+
+impl Avx512Opcode {
+    /// Which `InstructionSet`s support the opcode?
+    #[allow(dead_code)]
+    pub(crate) fn available_from(&self) -> SmallVec<[InstructionSet; 2]> {
+        match self {
+            Avx512Opcode::Vpabsq => smallvec![InstructionSet::AVX512F, InstructionSet::AVX512VL],
+        }
+    }
+}
+
+impl fmt::Debug for Avx512Opcode {
+    fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result {
+        let name = match self {
+            Avx512Opcode::Vpabsq => "vpabsq",
+        };
+        write!(fmt, "{}", name)
+    }
+}
+
+impl fmt::Display for Avx512Opcode {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        fmt::Debug::fmt(self, f)
+    }
+}
+
 /// This defines the ways a value can be extended: either signed- or zero-extension, or none for
 /// types that are not extended. Contrast with [ExtMode], which defines the widths from and to which
 /// values can be extended.

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

@@ -113,18 +113,30 @@ pub(crate) fn emit(
     info: &EmitInfo,
     state: &mut EmitState,
 ) {
-    if let Some(iset_requirement) = inst.isa_requirement() {
+    let matches_isa_flags = |iset_requirement: &InstructionSet| -> bool {
         match iset_requirement {
             // Cranelift assumes SSE2 at least.
-            InstructionSet::SSE | InstructionSet::SSE2 => {}
+            InstructionSet::SSE | InstructionSet::SSE2 => true,
-            InstructionSet::SSSE3 => assert!(info.isa_flags.use_ssse3()),
+            InstructionSet::SSSE3 => info.isa_flags.use_ssse3(),
-            InstructionSet::SSE41 => assert!(info.isa_flags.use_sse41()),
+            InstructionSet::SSE41 => info.isa_flags.use_sse41(),
-            InstructionSet::SSE42 => assert!(info.isa_flags.use_sse42()),
+            InstructionSet::SSE42 => info.isa_flags.use_sse42(),
-            InstructionSet::Popcnt => assert!(info.isa_flags.use_popcnt()),
+            InstructionSet::Popcnt => info.isa_flags.use_popcnt(),
-            InstructionSet::Lzcnt => assert!(info.isa_flags.use_lzcnt()),
+            InstructionSet::Lzcnt => info.isa_flags.use_lzcnt(),
-            InstructionSet::BMI1 => assert!(info.isa_flags.use_bmi1()),
+            InstructionSet::BMI1 => info.isa_flags.use_bmi1(),
-            InstructionSet::BMI2 => assert!(info.isa_flags.has_bmi2()),
+            InstructionSet::BMI2 => info.isa_flags.has_bmi2(),
+            InstructionSet::AVX512F => info.isa_flags.has_avx512f(),
+            InstructionSet::AVX512VL => info.isa_flags.has_avx512vl(),
         }
+    };
+
+    // Certain instructions may be present in more than one ISA feature set; we must at least match
+    // one of them in the target CPU.
+    let isa_requirements = inst.available_in_any_isa();
+    if !isa_requirements.is_empty() && !isa_requirements.iter().any(matches_isa_flags) {
+        panic!(
+            "Cannot emit inst '{:?}' for target; failed to match ISA requirements: {:?}",
+            inst, isa_requirements
+        )
     }
 
     match inst {

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

@@ -14,7 +14,7 @@ use regalloc::{
     PrettyPrint, PrettyPrintSized, RealRegUniverse, Reg, RegClass, RegUsageCollector,
     RegUsageMapper, SpillSlot, VirtualReg, Writable,
 };
-use smallvec::SmallVec;
+use smallvec::{smallvec, SmallVec};
 use std::fmt;
 use std::string::{String, ToString};
 
@@ -502,7 +502,11 @@ pub(crate) fn low32_will_sign_extend_to_64(x: u64) -> bool {
 }
 
 impl Inst {
-    fn isa_requirement(&self) -> Option<InstructionSet> {
+    /// Retrieve a list of ISA feature sets in which the instruction is available. An empty list
+    /// indicates that the instruction is available in the baseline feature set (i.e. SSE2 and
+    /// below); more than one `InstructionSet` in the list indicates that the instruction is present
+    /// *any* of the included ISA feature sets.
+    fn available_in_any_isa(&self) -> SmallVec<[InstructionSet; 2]> {
         match self {
             // These instructions are part of SSE2, which is a basic requirement in Cranelift, and
             // don't have to be checked.
@@ -555,7 +559,7 @@ impl Inst {
             | Inst::ElfTlsGetAddr { .. }
             | Inst::MachOTlsGetAddr { .. }
             | Inst::ValueLabelMarker { .. }
-            | Inst::Unwind { .. } => None,
+            | Inst::Unwind { .. } => smallvec![],
 
             Inst::UnaryRmR { op, .. } => op.available_from(),
 
@@ -566,7 +570,7 @@ impl Inst {
             | Inst::XmmRmR { op, .. }
             | Inst::XmmRmRImm { op, .. }
             | Inst::XmmToGpr { op, .. }
-            | Inst::XmmUnaryRmR { op, .. } => Some(op.available_from()),
+            | Inst::XmmUnaryRmR { op, .. } => smallvec![op.available_from()],
         }
     }
 }