From 0d703c12edec9940662ce73d47ba6e6306c5d832 Mon Sep 17 00:00:00 2001 From: Chris Fallin Date: Thu, 12 Nov 2020 20:40:54 -0800 Subject: [PATCH] Don't run old x86 backend-specific tests with new x64 backend. Some of the test failures tracked by #2079 are in unwind tests that are specific to the old x86 backend: namely, these tests invoke the unwind implementation that is paired with the old backend, rather than generic over all backends. It thus doesn't make sense to try to run these tests with the new backend. (The new backend's unwind code should have analogous tests written/ported over eventually.) It seems that we were actually building *both* x86 backends when the `x64` feature was enabled, except that the old x86 backend would never be instantiated by the usual ISA-lookup logic because a `x86-64` target triple unconditionally resolves to the new one. This PR resolves both of the issues by tweaking the feature-config directives to exclude the `x86` backend when `x64` is enabled. --- cranelift/codegen/src/isa/mod.rs | 21 +++++++++++-------- cranelift/codegen/src/isa/x86/unwind.rs | 4 ---- .../codegen/src/isa/x86/unwind/systemv.rs | 2 -- .../codegen/src/isa/x86/unwind/winx64.rs | 3 --- 4 files changed, 12 insertions(+), 18 deletions(-) diff --git a/cranelift/codegen/src/isa/mod.rs b/cranelift/codegen/src/isa/mod.rs index 2e56c025d0..a1a4c3c397 100644 --- a/cranelift/codegen/src/isa/mod.rs +++ b/cranelift/codegen/src/isa/mod.rs @@ -75,7 +75,10 @@ use thiserror::Error; #[cfg(feature = "riscv")] mod riscv; -#[cfg(feature = "x86")] +// 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")))] mod x86; #[cfg(feature = "x64")] @@ -102,12 +105,12 @@ mod test_utils; /// Returns a builder that can create a corresponding `TargetIsa` /// or `Err(LookupError::SupportDisabled)` if not enabled. macro_rules! isa_builder { - ($name: ident, $feature: tt, $triple: ident) => {{ - #[cfg(feature = $feature)] + ($name: ident, $cfg_terms: tt, $triple: ident) => {{ + #[cfg $cfg_terms] { Ok($name::isa_builder($triple)) } - #[cfg(not(feature = $feature))] + #[cfg(not $cfg_terms)] { Err(LookupError::SupportDisabled) } @@ -119,17 +122,17 @@ macro_rules! isa_builder { pub fn lookup(triple: Triple) -> Result { match triple.architecture { Architecture::Riscv32 { .. } | Architecture::Riscv64 { .. } => { - isa_builder!(riscv, "riscv", triple) + isa_builder!(riscv, (feature = "riscv"), triple) } Architecture::X86_32 { .. } | Architecture::X86_64 => { if cfg!(feature = "x64") { - isa_builder!(x64, "x64", triple) + isa_builder!(x64, (feature = "x64"), triple) } else { - isa_builder!(x86, "x86", triple) + isa_builder!(x86, (all(feature = "x86", not(feature = "x64"))), triple) } } - Architecture::Arm { .. } => isa_builder!(arm32, "arm32", triple), - Architecture::Aarch64 { .. } => isa_builder!(aarch64, "arm64", triple), + Architecture::Arm { .. } => isa_builder!(arm32, (feature = "arm32"), triple), + Architecture::Aarch64 { .. } => isa_builder!(aarch64, (feature = "arm64"), triple), _ => Err(LookupError::Unsupported), } } diff --git a/cranelift/codegen/src/isa/x86/unwind.rs b/cranelift/codegen/src/isa/x86/unwind.rs index 2d6b29f04d..717964a287 100644 --- a/cranelift/codegen/src/isa/x86/unwind.rs +++ b/cranelift/codegen/src/isa/x86/unwind.rs @@ -257,7 +257,6 @@ mod tests { use target_lexicon::triple; #[test] - #[cfg_attr(feature = "x64", should_panic)] // TODO #2079 fn test_small_alloc() { let isa = lookup(triple!("x86_64")) .expect("expect x86 ISA") @@ -314,7 +313,6 @@ mod tests { } #[test] - #[cfg_attr(feature = "x64", should_panic)] // TODO #2079 fn test_medium_alloc() { let isa = lookup(triple!("x86_64")) .expect("expect x86 ISA") @@ -371,7 +369,6 @@ mod tests { } #[test] - #[cfg_attr(feature = "x64", should_panic)] // TODO #2079 fn test_large_alloc() { let isa = lookup(triple!("x86_64")) .expect("expect x86 ISA") @@ -444,7 +441,6 @@ mod tests { } #[test] - #[cfg_attr(feature = "x64", should_panic)] // TODO #2079 fn test_multi_return_func() { let isa = lookup(triple!("x86_64")) .expect("expect x86 ISA") diff --git a/cranelift/codegen/src/isa/x86/unwind/systemv.rs b/cranelift/codegen/src/isa/x86/unwind/systemv.rs index f6333f5afb..8f112d943a 100644 --- a/cranelift/codegen/src/isa/x86/unwind/systemv.rs +++ b/cranelift/codegen/src/isa/x86/unwind/systemv.rs @@ -142,7 +142,6 @@ mod tests { use target_lexicon::triple; #[test] - #[cfg_attr(feature = "x64", should_panic)] // TODO #2079 fn test_simple_func() { let isa = lookup(triple!("x86_64")) .expect("expect x86 ISA") @@ -185,7 +184,6 @@ mod tests { } #[test] - #[cfg_attr(feature = "x64", should_panic)] // TODO #2079 fn test_multi_return_func() { let isa = lookup(triple!("x86_64")) .expect("expect x86 ISA") diff --git a/cranelift/codegen/src/isa/x86/unwind/winx64.rs b/cranelift/codegen/src/isa/x86/unwind/winx64.rs index ed046f9a87..e49b2c1ef1 100644 --- a/cranelift/codegen/src/isa/x86/unwind/winx64.rs +++ b/cranelift/codegen/src/isa/x86/unwind/winx64.rs @@ -69,7 +69,6 @@ mod tests { } #[test] - #[cfg_attr(feature = "x64", should_panic)] // TODO #2079 fn test_small_alloc() { let isa = lookup(triple!("x86_64")) .expect("expect x86 ISA") @@ -127,7 +126,6 @@ mod tests { } #[test] - #[cfg_attr(feature = "x64", should_panic)] // TODO #2079 fn test_medium_alloc() { let isa = lookup(triple!("x86_64")) .expect("expect x86 ISA") @@ -189,7 +187,6 @@ mod tests { } #[test] - #[cfg_attr(feature = "x64", should_panic)] // TODO #2079 fn test_large_alloc() { let isa = lookup(triple!("x86_64")) .expect("expect x86 ISA")