From a45b814de880137453ca78f1e55b103c99d9a149 Mon Sep 17 00:00:00 2001 From: Benjamin Bouvier Date: Mon, 11 Feb 2019 15:01:10 +0100 Subject: [PATCH] Fixes #13: Enable conditional compilation of ISAs through features; --- cranelift/codegen/Cargo.toml | 10 ++- cranelift/codegen/build.rs | 36 ++++++----- cranelift/codegen/meta/src/isa/mod.rs | 1 + cranelift/codegen/meta/src/lib.rs | 39 +----------- cranelift/codegen/src/isa/mod.rs | 24 ++++---- cranelift/codegen/src/regalloc/pressure.rs | 2 +- cranelift/codegen/src/regalloc/solver.rs | 2 +- cranelift/filetests/src/runner.rs | 2 +- cranelift/filetests/src/runone.rs | 16 ++++- cranelift/reader/src/error.rs | 14 +++++ cranelift/reader/src/isaspec.rs | 2 +- cranelift/reader/src/parser.rs | 71 +++++++++++++++++----- 12 files changed, 129 insertions(+), 90 deletions(-) diff --git a/cranelift/codegen/Cargo.toml b/cranelift/codegen/Cargo.toml index 0448d8072e..8ffee131fb 100644 --- a/cranelift/codegen/Cargo.toml +++ b/cranelift/codegen/Cargo.toml @@ -29,16 +29,24 @@ log = { version = "0.4.6", default-features = false } cranelift-codegen-meta = { path = "meta", version = "0.28.0" } [features] +default = ["std", "x86", "arm32", "arm64", "riscv"] + # The "std" feature enables use of libstd. The "core" feature enables use # of some minimal std-like replacement libraries. At least one of these two # features need to be enabled. -default = ["std"] std = ["cranelift-entity/std", "cranelift-bforest/std", "target-lexicon/std"] core = ["hashmap_core"] + # This enables some additional functions useful for writing tests, but which # can significantly increase the size of the library. testing_hooks = [] +# ISA targets for which we should build. +x86 = [] +arm32 = [] +arm64 = [] +riscv = [] + [badges] maintenance = { status = "experimental" } travis-ci = { repository = "CraneStation/cranelift" } diff --git a/cranelift/codegen/build.rs b/cranelift/codegen/build.rs index 6aeaff6834..733a1c782f 100644 --- a/cranelift/codegen/build.rs +++ b/cranelift/codegen/build.rs @@ -11,10 +11,6 @@ // TARGET // Target triple provided by Cargo. // -// CRANELIFT_TARGETS (Optional) -// A setting for conditional compilation of isa targets. Possible values can be "native" or -// known isa targets separated by ','. -// // The build script expects to be run from the directory where this build.rs file lives. The // current directory is used to find the sources. @@ -31,22 +27,24 @@ fn main() { let target_triple = env::var("TARGET").expect("The TARGET environment variable must be set"); // Configure isa targets cfg. - let cranelift_targets = env::var("CRANELIFT_TARGETS").ok(); - let cranelift_targets = cranelift_targets - .as_ref() - .map(|s| s.as_ref()) - .filter(|s: &&str| s.len() > 0); + let isa_targets = meta::isa::Isa::all() + .into_iter() + .cloned() + .filter(|isa| { + let env_key = format!("CARGO_FEATURE_{}", isa.to_string().to_uppercase()); + env::var(env_key).is_ok() + }) + .collect::>(); - let isas = match cranelift_targets { - Some("native") => meta::isa_from_arch(&target_triple.split('-').next().unwrap()), - Some(targets) => meta::isas_from_targets(targets.split(',').collect::>()), - None => meta::all_isas(), - } - .expect("Error when identifying CRANELIFT_TARGETS and TARGET"); - - for isa in &isas { - println!("cargo:rustc-cfg=build_{}", isa.to_string()); - } + let isas = if isa_targets.is_empty() { + // Try to match native target. + let target_name = target_triple.split('-').next().unwrap(); + let isa = meta::isa_from_arch(&target_name).expect("error when identifying target"); + println!("cargo:rustc-cfg=feature=\"{}\"", isa); + vec![isa] + } else { + isa_targets + }; let cur_dir = env::current_dir().expect("Can't access current working directory"); let crate_dir = cur_dir.as_path(); diff --git a/cranelift/codegen/meta/src/isa/mod.rs b/cranelift/codegen/meta/src/isa/mod.rs index c06326689b..9662435867 100644 --- a/cranelift/codegen/meta/src/isa/mod.rs +++ b/cranelift/codegen/meta/src/isa/mod.rs @@ -44,6 +44,7 @@ impl Isa { } impl fmt::Display for Isa { + // These names should be kept in sync with the crate features. fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { match *self { Isa::Riscv => write!(f, "riscv"), diff --git a/cranelift/codegen/meta/src/lib.rs b/cranelift/codegen/meta/src/lib.rs index 610f2cd4a2..b617d31c16 100644 --- a/cranelift/codegen/meta/src/lib.rs +++ b/cranelift/codegen/meta/src/lib.rs @@ -13,43 +13,8 @@ mod shared; mod srcgen; mod unique_table; -pub fn isa_from_arch(arch: &str) -> Result, String> { - isa::Isa::from_arch(arch) - .ok_or_else(|| format!("no supported isa found for arch `{}`", arch)) - .and_then(|isa| Ok(vec![isa])) -} - -pub fn isas_from_targets(targets: Vec<&str>) -> Result, String> { - type R<'a> = Vec<(&'a str, Option)>; - - let (known, unknown): (R, R) = targets - .into_iter() - .map(|target| (target, isa::Isa::from_name(target))) - .partition(|(_, opt_isa)| opt_isa.is_some()); - - if !unknown.is_empty() { - let unknown_targets = unknown - .into_iter() - .map(|(target, _)| target) - .collect::>() - .join(", "); - return Err(format!("unknown isa targets: {}", unknown_targets)); - } - - let isas = if known.is_empty() { - isa::Isa::all().to_vec() - } else { - known - .into_iter() - .map(|(_, opt_isa)| opt_isa.unwrap()) - .collect() - }; - - Ok(isas) -} - -pub fn all_isas() -> Result, String> { - isas_from_targets(vec![]) +pub fn isa_from_arch(arch: &str) -> Result { + isa::Isa::from_arch(arch).ok_or_else(|| format!("no supported isa found for arch `{}`", arch)) } /// Generates all the Rust source files used in Cranelift from the meta-language. diff --git a/cranelift/codegen/src/isa/mod.rs b/cranelift/codegen/src/isa/mod.rs index ab77269ea5..b3e4d81421 100644 --- a/cranelift/codegen/src/isa/mod.rs +++ b/cranelift/codegen/src/isa/mod.rs @@ -68,16 +68,16 @@ use failure_derive::Fail; use std::boxed::Box; use target_lexicon::{Architecture, PointerWidth, Triple}; -#[cfg(build_riscv)] +#[cfg(feature = "riscv")] mod riscv; -#[cfg(build_x86)] +#[cfg(feature = "x86")] mod x86; -#[cfg(build_arm32)] +#[cfg(feature = "arm32")] mod arm32; -#[cfg(build_arm64)] +#[cfg(feature = "arm64")] mod arm64; mod call_conv; @@ -90,12 +90,12 @@ mod stack; /// Returns a builder that can create a corresponding `TargetIsa` /// or `Err(LookupError::Unsupported)` if not enabled. macro_rules! isa_builder { - ($module:ident, $name:ident) => {{ - #[cfg($name)] + ($name:ident, $feature:tt) => {{ + #[cfg(feature = $feature)] fn $name(triple: Triple) -> Result { - Ok($module::isa_builder(triple)) + Ok($name::isa_builder(triple)) }; - #[cfg(not($name))] + #[cfg(not(feature = $feature))] fn $name(_triple: Triple) -> Result { Err(LookupError::Unsupported) } @@ -107,9 +107,9 @@ macro_rules! isa_builder { /// 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, build_riscv)(triple), + Architecture::Riscv32 | Architecture::Riscv64 => isa_builder!(riscv, "riscv")(triple), Architecture::I386 | Architecture::I586 | Architecture::I686 | Architecture::X86_64 => { - isa_builder!(x86, build_x86)(triple) + isa_builder!(x86, "x86")(triple) } Architecture::Thumbv6m | Architecture::Thumbv7em @@ -118,8 +118,8 @@ pub fn lookup(triple: Triple) -> Result { | Architecture::Armv4t | Architecture::Armv5te | Architecture::Armv7 - | Architecture::Armv7s => isa_builder!(arm32, build_arm32)(triple), - Architecture::Aarch64 => isa_builder!(arm64, build_arm64)(triple), + | Architecture::Armv7s => isa_builder!(arm32, "arm32")(triple), + Architecture::Aarch64 => isa_builder!(arm64, "arm64")(triple), _ => Err(LookupError::Unsupported), } } diff --git a/cranelift/codegen/src/regalloc/pressure.rs b/cranelift/codegen/src/regalloc/pressure.rs index cd1ef956a5..2db3ec3d03 100644 --- a/cranelift/codegen/src/regalloc/pressure.rs +++ b/cranelift/codegen/src/regalloc/pressure.rs @@ -270,7 +270,7 @@ impl fmt::Display for Pressure { } #[cfg(test)] -#[cfg(build_arm32)] +#[cfg(feature = "arm32")] mod tests { use super::Pressure; use crate::isa::{RegClass, TargetIsa}; diff --git a/cranelift/codegen/src/regalloc/solver.rs b/cranelift/codegen/src/regalloc/solver.rs index be1c9a2d27..96ccdd2841 100644 --- a/cranelift/codegen/src/regalloc/solver.rs +++ b/cranelift/codegen/src/regalloc/solver.rs @@ -1127,7 +1127,7 @@ impl fmt::Display for Solver { } #[cfg(test)] -#[cfg(build_arm32)] +#[cfg(feature = "arm32")] mod tests { use super::{Move, Solver}; use crate::entity::EntityRef; diff --git a/cranelift/filetests/src/runner.rs b/cranelift/filetests/src/runner.rs index 4c84551dd9..d11ffab79e 100644 --- a/cranelift/filetests/src/runner.rs +++ b/cranelift/filetests/src/runner.rs @@ -83,7 +83,7 @@ pub struct TestRunner { } impl TestRunner { - /// Create a new blank TrstRunner. + /// Create a new blank TestRunner. pub fn new(verbose: bool, report_times: bool) -> Self { Self { verbose, diff --git a/cranelift/filetests/src/runone.rs b/cranelift/filetests/src/runone.rs index a7b95dc7b5..323442d4b5 100644 --- a/cranelift/filetests/src/runone.rs +++ b/cranelift/filetests/src/runone.rs @@ -33,7 +33,21 @@ pub fn run(path: &Path, passes: Option<&[String]>, target: Option<&str>) -> Test info!("---\nFile: {}", path.to_string_lossy()); let started = time::Instant::now(); let buffer = read_to_string(path).map_err(|e| e.to_string())?; - let testfile = parse_test(&buffer, passes, target).map_err(|e| e.to_string())?; + + let testfile = match parse_test(&buffer, passes, target) { + Ok(testfile) => testfile, + Err(e) => { + if e.is_warning { + println!( + "skipping test {:?} (line {}): {}", + path, e.location.line_number, e.message + ); + return Ok(started.elapsed()); + } + return Err(e.to_string()); + } + }; + if testfile.functions.is_empty() { return Err("no functions found".to_string()); } diff --git a/cranelift/reader/src/error.rs b/cranelift/reader/src/error.rs index e006ad5c3b..bed2352e5c 100644 --- a/cranelift/reader/src/error.rs +++ b/cranelift/reader/src/error.rs @@ -19,6 +19,8 @@ pub struct ParseError { pub location: Location, /// Error message. pub message: String, + /// Whether it's a warning or a plain error. + pub is_warning: bool, } impl fmt::Display for ParseError { @@ -40,6 +42,7 @@ macro_rules! err { Err($crate::ParseError { location: $loc.clone(), message: $msg.to_string(), + is_warning: false, }) }; @@ -47,6 +50,17 @@ macro_rules! err { Err($crate::ParseError { location: $loc.clone(), message: format!( $fmt, $( $arg ),+ ), + is_warning: false, + }) + }; +} + +macro_rules! warn { + ( $loc:expr, $fmt:expr, $( $arg:expr ),+ ) => { + Err($crate::ParseError { + location: $loc.clone(), + message: format!($fmt, $( $arg ),+ ), + is_warning: true, }) }; } diff --git a/cranelift/reader/src/isaspec.rs b/cranelift/reader/src/isaspec.rs index 89dac510bb..6aefce354a 100644 --- a/cranelift/reader/src/isaspec.rs +++ b/cranelift/reader/src/isaspec.rs @@ -17,7 +17,7 @@ pub enum IsaSpec { /// which are reflected in the finished `Flags` object. None(Flags), - /// The parsed file does contains `isa` commands. + /// The parsed file does contain `isa` commands. /// Each `isa` command is used to configure a `TargetIsa` trait object. Some(Vec>), } diff --git a/cranelift/reader/src/parser.rs b/cranelift/reader/src/parser.rs index 4f5acc05f4..42dfd55f01 100644 --- a/cranelift/reader/src/parser.rs +++ b/cranelift/reader/src/parser.rs @@ -530,6 +530,7 @@ impl<'a> Parser<'a> { ParseError { location: self.loc, message: message.to_string(), + is_warning: false, } } @@ -778,10 +779,7 @@ impl<'a> Parser<'a> { /// /// Accept the target from the command line for pass command. /// - pub fn parse_cmdline_target( - &mut self, - target_pass: Option<&str>, - ) -> ParseResult { + fn parse_cmdline_target(&mut self, target_pass: Option<&str>) -> ParseResult { // Were there any `target` commands specified? let mut specified_target = false; @@ -799,7 +797,7 @@ impl<'a> Parser<'a> { return err!(loc, "support disabled target '{}'", targ); } Err(isa::LookupError::Unsupported) => { - return err!(loc, "unsupported target '{}'", targ); + return warn!(loc, "unsupported target '{}'", targ); } Ok(b) => b, }; @@ -821,7 +819,7 @@ impl<'a> Parser<'a> { /// /// Accept a mix of `target` and `set` command lines. The `set` commands are cumulative. /// - pub fn parse_target_specs(&mut self) -> ParseResult { + fn parse_target_specs(&mut self) -> ParseResult { // Were there any `target` commands? let mut seen_target = false; // Location of last `set` command since the last `target`. @@ -859,7 +857,7 @@ impl<'a> Parser<'a> { continue; } Err(isa::LookupError::Unsupported) => { - return err!(loc, "unsupported target '{}'", target_name); + return warn!(loc, "unsupported target '{}'", target_name); } Ok(b) => b, }; @@ -874,6 +872,7 @@ impl<'a> Parser<'a> { _ => break, } } + if !seen_target { // No `target` commands, but we allow for `set` commands. Ok(isaspec::IsaSpec::None(settings::Flags::new(flag_builder))) @@ -2588,9 +2587,14 @@ mod tests { assert_eq!(arg.value_type, types::I32); assert_eq!(arg.extension, ArgumentExtension::Sext); assert_eq!(arg.purpose, ArgumentPurpose::Normal); - let ParseError { location, message } = p.parse_abi_param(None).unwrap_err(); + let ParseError { + location, + message, + is_warning, + } = p.parse_abi_param(None).unwrap_err(); assert_eq!(location.line_number, 1); assert_eq!(message, "expected parameter type"); + assert!(!is_warning); } #[test] @@ -2736,7 +2740,11 @@ mod tests { #[test] fn duplicate_ebb() { - let ParseError { location, message } = Parser::new( + let ParseError { + location, + message, + is_warning, + } = Parser::new( "function %ebbs() system_v { ebb0: ebb0: @@ -2747,11 +2755,16 @@ mod tests { assert_eq!(location.line_number, 3); assert_eq!(message, "duplicate entity: ebb0"); + assert!(!is_warning); } #[test] fn duplicate_jt() { - let ParseError { location, message } = Parser::new( + let ParseError { + location, + message, + is_warning, + } = Parser::new( "function %ebbs() system_v { jt0 = jump_table [] jt0 = jump_table []", @@ -2761,11 +2774,16 @@ mod tests { assert_eq!(location.line_number, 3); assert_eq!(message, "duplicate entity: jt0"); + assert!(!is_warning); } #[test] fn duplicate_ss() { - let ParseError { location, message } = Parser::new( + let ParseError { + location, + message, + is_warning, + } = Parser::new( "function %ebbs() system_v { ss0 = explicit_slot 8 ss0 = explicit_slot 8", @@ -2775,11 +2793,16 @@ mod tests { assert_eq!(location.line_number, 3); assert_eq!(message, "duplicate entity: ss0"); + assert!(!is_warning); } #[test] fn duplicate_gv() { - let ParseError { location, message } = Parser::new( + let ParseError { + location, + message, + is_warning, + } = Parser::new( "function %ebbs() system_v { gv0 = vmctx gv0 = vmctx", @@ -2789,11 +2812,16 @@ mod tests { assert_eq!(location.line_number, 3); assert_eq!(message, "duplicate entity: gv0"); + assert!(!is_warning); } #[test] fn duplicate_heap() { - let ParseError { location, message } = Parser::new( + let ParseError { + location, + message, + is_warning, + } = Parser::new( "function %ebbs() system_v { heap0 = static gv0, min 0x1000, bound 0x10_0000, offset_guard 0x1000 heap0 = static gv0, min 0x1000, bound 0x10_0000, offset_guard 0x1000", @@ -2803,11 +2831,16 @@ mod tests { assert_eq!(location.line_number, 3); assert_eq!(message, "duplicate entity: heap0"); + assert!(!is_warning); } #[test] fn duplicate_sig() { - let ParseError { location, message } = Parser::new( + let ParseError { + location, + message, + is_warning, + } = Parser::new( "function %ebbs() system_v { sig0 = () sig0 = ()", @@ -2817,11 +2850,16 @@ mod tests { assert_eq!(location.line_number, 3); assert_eq!(message, "duplicate entity: sig0"); + assert!(!is_warning); } #[test] fn duplicate_fn() { - let ParseError { location, message } = Parser::new( + let ParseError { + location, + message, + is_warning, + } = Parser::new( "function %ebbs() system_v { sig0 = () fn0 = %foo sig0 @@ -2832,6 +2870,7 @@ mod tests { assert_eq!(location.line_number, 4); assert_eq!(message, "duplicate entity: fn0"); + assert!(!is_warning); } #[test] @@ -2905,7 +2944,7 @@ mod tests { } #[test] - #[cfg(build_riscv)] + #[cfg(feature = "riscv")] fn isa_spec() { assert!(parse_test( "target