From e91987c43ccd080596a932a1147b702854f5655b Mon Sep 17 00:00:00 2001 From: Chris Fallin Date: Wed, 16 Dec 2020 07:56:04 -0800 Subject: [PATCH] Allow both x86 backends to be included, selected with a "variant" flag. (#2514) This PR adds a new `isa::lookup_variant()` that takes a `BackendVariant` (`Legacy`, `MachInst` or `Any`), and exposes both x86 backends as separate variants if both are compiled into the build. This will allow some new use-cases that require both backends in the same process: for example, differential fuzzing between old and new backends, or perhaps allowing for dynamic feature-flag selection between the backends. --- cranelift/codegen/src/isa/mod.rs | 73 ++++++++++++++----- cranelift/codegen/src/isa/x86/unwind.rs | 10 +-- .../codegen/src/isa/x86/unwind/systemv.rs | 6 +- .../codegen/src/isa/x86/unwind/winx64.rs | 10 +-- 4 files changed, 67 insertions(+), 32 deletions(-) diff --git a/cranelift/codegen/src/isa/mod.rs b/cranelift/codegen/src/isa/mod.rs index 1ce10a155e..900dfd9ddd 100644 --- a/cranelift/codegen/src/isa/mod.rs +++ b/cranelift/codegen/src/isa/mod.rs @@ -75,10 +75,13 @@ use thiserror::Error; #[cfg(feature = "riscv")] mod riscv; -// Exclude the old x86 backend when the new one is enabled; it is unreachable -// anyway (requesting the x86-64 ISA will return the new backend), and a number -// of old-backend-specific tests fail. -#[cfg(all(feature = "x86", not(feature = "x64")))] +// N.B.: the old x86-64 backend (`x86`) and the new one (`x64`) can both be +// included; if the new backend is included, then it is the default backend +// returned for an x86-64 triple, but a specific option can request the old +// backend. It is important to have the ability to instantiate *both* backends +// in the same build so that we can do things like differential fuzzing between +// backends, or perhaps offer a runtime configuration flag in the future. +#[cfg(feature = "x86")] mod x86; #[cfg(feature = "x64")] @@ -117,24 +120,56 @@ macro_rules! isa_builder { }}; } +/// The "variant" for a given target. On one platform (x86-64), we have two +/// backends, the "old" and "new" one; the new one is the default if included +/// in the build configuration and not otherwise specified. +#[derive(Clone, Copy)] +pub enum BackendVariant { + /// Any backend available. + Any, + /// A "legacy" backend: one that operates using legalizations and encodings. + Legacy, + /// A backend built on `MachInst`s and the `VCode` framework. + MachInst, +} + +impl Default for BackendVariant { + fn default() -> Self { + BackendVariant::Any + } +} + +/// Look for an ISA for the given `triple`, selecting the backend variant given +/// by `variant` if available. +pub fn lookup_variant(triple: Triple, variant: BackendVariant) -> Result { + match (triple.architecture, variant) { + (Architecture::Riscv32 { .. }, _) | (Architecture::Riscv64 { .. }, _) => { + isa_builder!(riscv, (feature = "riscv"), triple) + } + (Architecture::X86_64, BackendVariant::Legacy) => { + isa_builder!(x86, (feature = "x86"), triple) + } + (Architecture::X86_64, BackendVariant::MachInst) => { + isa_builder!(x64, (feature = "x64"), triple) + } + #[cfg(feature = "x64")] + (Architecture::X86_64, BackendVariant::Any) => { + isa_builder!(x64, (feature = "x64"), triple) + } + #[cfg(not(feature = "x64"))] + (Architecture::X86_64, BackendVariant::Any) => { + isa_builder!(x86, (feature = "x86"), triple) + } + (Architecture::Arm { .. }, _) => isa_builder!(arm32, (feature = "arm32"), triple), + (Architecture::Aarch64 { .. }, _) => isa_builder!(aarch64, (feature = "arm64"), triple), + _ => Err(LookupError::Unsupported), + } +} + /// Look for an ISA for the given `triple`. /// Return a builder that can create a corresponding `TargetIsa`. pub fn lookup(triple: Triple) -> Result { - match triple.architecture { - Architecture::Riscv32 { .. } | Architecture::Riscv64 { .. } => { - isa_builder!(riscv, (feature = "riscv"), triple) - } - Architecture::X86_32 { .. } | Architecture::X86_64 => { - if cfg!(feature = "x64") { - isa_builder!(x64, (feature = "x64"), triple) - } else { - isa_builder!(x86, (all(feature = "x86", not(feature = "x64"))), triple) - } - } - Architecture::Arm { .. } => isa_builder!(arm32, (feature = "arm32"), triple), - Architecture::Aarch64 { .. } => isa_builder!(aarch64, (feature = "arm64"), triple), - _ => Err(LookupError::Unsupported), - } + lookup_variant(triple, BackendVariant::Any) } /// Look for a supported ISA with the given `name`. diff --git a/cranelift/codegen/src/isa/x86/unwind.rs b/cranelift/codegen/src/isa/x86/unwind.rs index 717964a287..2eed8b74e4 100644 --- a/cranelift/codegen/src/isa/x86/unwind.rs +++ b/cranelift/codegen/src/isa/x86/unwind.rs @@ -250,7 +250,7 @@ mod tests { use crate::ir::{ types, AbiParam, ExternalName, InstBuilder, Signature, StackSlotData, StackSlotKind, }; - use crate::isa::{lookup, CallConv}; + use crate::isa::{lookup_variant, BackendVariant, CallConv}; use crate::settings::{builder, Flags}; use crate::Context; use std::str::FromStr; @@ -258,7 +258,7 @@ mod tests { #[test] fn test_small_alloc() { - let isa = lookup(triple!("x86_64")) + let isa = lookup_variant(triple!("x86_64"), BackendVariant::Legacy) .expect("expect x86 ISA") .finish(Flags::new(builder())); @@ -314,7 +314,7 @@ mod tests { #[test] fn test_medium_alloc() { - let isa = lookup(triple!("x86_64")) + let isa = lookup_variant(triple!("x86_64"), BackendVariant::Legacy) .expect("expect x86 ISA") .finish(Flags::new(builder())); @@ -370,7 +370,7 @@ mod tests { #[test] fn test_large_alloc() { - let isa = lookup(triple!("x86_64")) + let isa = lookup_variant(triple!("x86_64"), BackendVariant::Legacy) .expect("expect x86 ISA") .finish(Flags::new(builder())); @@ -442,7 +442,7 @@ mod tests { #[test] fn test_multi_return_func() { - let isa = lookup(triple!("x86_64")) + let isa = lookup_variant(triple!("x86_64"), BackendVariant::Legacy) .expect("expect x86 ISA") .finish(Flags::new(builder())); diff --git a/cranelift/codegen/src/isa/x86/unwind/systemv.rs b/cranelift/codegen/src/isa/x86/unwind/systemv.rs index 8f112d943a..09778627bb 100644 --- a/cranelift/codegen/src/isa/x86/unwind/systemv.rs +++ b/cranelift/codegen/src/isa/x86/unwind/systemv.rs @@ -134,7 +134,7 @@ mod tests { use crate::ir::{ types, AbiParam, ExternalName, InstBuilder, Signature, StackSlotData, StackSlotKind, }; - use crate::isa::{lookup, CallConv}; + use crate::isa::{lookup_variant, BackendVariant, CallConv}; use crate::settings::{builder, Flags}; use crate::Context; use gimli::write::Address; @@ -143,7 +143,7 @@ mod tests { #[test] fn test_simple_func() { - let isa = lookup(triple!("x86_64")) + let isa = lookup_variant(triple!("x86_64"), BackendVariant::Legacy) .expect("expect x86 ISA") .finish(Flags::new(builder())); @@ -185,7 +185,7 @@ mod tests { #[test] fn test_multi_return_func() { - let isa = lookup(triple!("x86_64")) + let isa = lookup_variant(triple!("x86_64"), BackendVariant::Legacy) .expect("expect x86 ISA") .finish(Flags::new(builder())); diff --git a/cranelift/codegen/src/isa/x86/unwind/winx64.rs b/cranelift/codegen/src/isa/x86/unwind/winx64.rs index e49b2c1ef1..1fe93f9708 100644 --- a/cranelift/codegen/src/isa/x86/unwind/winx64.rs +++ b/cranelift/codegen/src/isa/x86/unwind/winx64.rs @@ -46,7 +46,7 @@ mod tests { use crate::ir::{ExternalName, InstBuilder, Signature, StackSlotData, StackSlotKind}; use crate::isa::unwind::winx64::UnwindCode; use crate::isa::x86::registers::RU; - use crate::isa::{lookup, CallConv}; + use crate::isa::{lookup_variant, BackendVariant, CallConv}; use crate::settings::{builder, Flags}; use crate::Context; use std::str::FromStr; @@ -54,7 +54,7 @@ mod tests { #[test] fn test_wrong_calling_convention() { - let isa = lookup(triple!("x86_64")) + let isa = lookup_variant(triple!("x86_64"), BackendVariant::Legacy) .expect("expect x86 ISA") .finish(Flags::new(builder())); @@ -70,7 +70,7 @@ mod tests { #[test] fn test_small_alloc() { - let isa = lookup(triple!("x86_64")) + let isa = lookup_variant(triple!("x86_64"), BackendVariant::Legacy) .expect("expect x86 ISA") .finish(Flags::new(builder())); @@ -127,7 +127,7 @@ mod tests { #[test] fn test_medium_alloc() { - let isa = lookup(triple!("x86_64")) + let isa = lookup_variant(triple!("x86_64"), BackendVariant::Legacy) .expect("expect x86 ISA") .finish(Flags::new(builder())); @@ -188,7 +188,7 @@ mod tests { #[test] fn test_large_alloc() { - let isa = lookup(triple!("x86_64")) + let isa = lookup_variant(triple!("x86_64"), BackendVariant::Legacy) .expect("expect x86 ISA") .finish(Flags::new(builder()));