From 8e495ac79d68ca43cdd07395076dbdaa84fd8f98 Mon Sep 17 00:00:00 2001 From: Andrew Brown Date: Wed, 7 Apr 2021 16:00:54 -0700 Subject: [PATCH] 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 | 47 +++++++++++++++++++--- cranelift/codegen/src/isa/x64/inst/emit.rs | 30 +++++++++----- cranelift/codegen/src/isa/x64/inst/mod.rs | 12 ++++-- 3 files changed, 71 insertions(+), 18 deletions(-) diff --git a/cranelift/codegen/src/isa/x64/inst/args.rs b/cranelift/codegen/src/isa/x64/inst/args.rs index 19fddae61e..2eebf4140e 100644 --- a/cranelift/codegen/src/isa/x64/inst/args.rs +++ b/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 { + pub(crate) fn available_from(&self) -> SmallVec<[InstructionSet; 2]> { match self { - UnaryRmROpcode::Bsr | UnaryRmROpcode::Bsf => None, - UnaryRmROpcode::Lzcnt => Some(InstructionSet::Lzcnt), - UnaryRmROpcode::Tzcnt => Some(InstructionSet::BMI1), - UnaryRmROpcode::Popcnt => Some(InstructionSet::Popcnt), + UnaryRmROpcode::Bsr | UnaryRmROpcode::Bsf => smallvec![], + UnaryRmROpcode::Lzcnt => smallvec![InstructionSet::Lzcnt], + UnaryRmROpcode::Tzcnt => smallvec![InstructionSet::BMI1], + 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. diff --git a/cranelift/codegen/src/isa/x64/inst/emit.rs b/cranelift/codegen/src/isa/x64/inst/emit.rs index 1674f2ab26..f16f7d66c1 100644 --- a/cranelift/codegen/src/isa/x64/inst/emit.rs +++ b/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::SSSE3 => assert!(info.isa_flags.use_ssse3()), - InstructionSet::SSE41 => assert!(info.isa_flags.use_sse41()), - InstructionSet::SSE42 => assert!(info.isa_flags.use_sse42()), - InstructionSet::Popcnt => assert!(info.isa_flags.use_popcnt()), - InstructionSet::Lzcnt => assert!(info.isa_flags.use_lzcnt()), - InstructionSet::BMI1 => assert!(info.isa_flags.use_bmi1()), - InstructionSet::BMI2 => assert!(info.isa_flags.has_bmi2()), + InstructionSet::SSE | InstructionSet::SSE2 => true, + InstructionSet::SSSE3 => info.isa_flags.use_ssse3(), + InstructionSet::SSE41 => info.isa_flags.use_sse41(), + InstructionSet::SSE42 => info.isa_flags.use_sse42(), + InstructionSet::Popcnt => info.isa_flags.use_popcnt(), + InstructionSet::Lzcnt => info.isa_flags.use_lzcnt(), + InstructionSet::BMI1 => info.isa_flags.use_bmi1(), + 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 { diff --git a/cranelift/codegen/src/isa/x64/inst/mod.rs b/cranelift/codegen/src/isa/x64/inst/mod.rs index a1556c8b07..099c975531 100644 --- a/cranelift/codegen/src/isa/x64/inst/mod.rs +++ b/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 { + /// 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()], } } }