diff --git a/cranelift/filetests/verifier/return_at_end.cton b/cranelift/filetests/verifier/return_at_end.cton index b0fe05889a..7acc8b8fad 100644 --- a/cranelift/filetests/verifier/return_at_end.cton +++ b/cranelift/filetests/verifier/return_at_end.cton @@ -1,10 +1,6 @@ test verifier set return_at_end -; The verifier doesn't have an API for verifying with flags without also -; setting an ISA. -isa riscv - function %ok(i32) { ebb0(v0: i32): brnz v0, ebb1 diff --git a/cranelift/src/filetest/licm.rs b/cranelift/src/filetest/licm.rs index 5806776e84..3f3bc6cecc 100644 --- a/cranelift/src/filetest/licm.rs +++ b/cranelift/src/filetest/licm.rs @@ -41,7 +41,7 @@ impl SubTest for TestLICM { comp_ctx.flowgraph(); comp_ctx.compute_loop_analysis(); comp_ctx.licm(); - comp_ctx.verify(context.isa).map_err(|e| { + comp_ctx.verify(context.flags_or_isa()).map_err(|e| { pretty_error(&comp_ctx.func, context.isa, Into::into(e)) })?; diff --git a/cranelift/src/filetest/runone.rs b/cranelift/src/filetest/runone.rs index dad7b89e16..67ea25b08d 100644 --- a/cranelift/src/filetest/runone.rs +++ b/cranelift/src/filetest/runone.rs @@ -119,9 +119,11 @@ fn run_one_test<'a>( // Should we run the verifier before this test? if !context.verified && test.needs_verifier() { - verify_function(&func, isa).map_err(|e| { - pretty_verifier_error(&func, isa, e) - })?; + verify_function(&func, context.flags_or_isa()).map_err( + |e| { + pretty_verifier_error(&func, isa, e) + }, + )?; context.verified = true; } diff --git a/cranelift/src/filetest/simple_gvn.rs b/cranelift/src/filetest/simple_gvn.rs index d25618eb1d..12a5554f74 100644 --- a/cranelift/src/filetest/simple_gvn.rs +++ b/cranelift/src/filetest/simple_gvn.rs @@ -40,7 +40,7 @@ impl SubTest for TestSimpleGVN { comp_ctx.flowgraph(); comp_ctx.simple_gvn(); - comp_ctx.verify(context.isa).map_err(|e| { + comp_ctx.verify(context.flags_or_isa()).map_err(|e| { pretty_error(&comp_ctx.func, context.isa, Into::into(e)) })?; diff --git a/cranelift/src/filetest/subtest.rs b/cranelift/src/filetest/subtest.rs index 1129617db2..dc7ac6eeee 100644 --- a/cranelift/src/filetest/subtest.rs +++ b/cranelift/src/filetest/subtest.rs @@ -4,7 +4,7 @@ use std::result; use std::borrow::Cow; use cretonne::ir::Function; use cretonne::isa::TargetIsa; -use cretonne::settings::Flags; +use cretonne::settings::{Flags, FlagsOrIsa}; use cton_reader::{Details, Comment}; use filecheck::{self, CheckerBuilder, Checker, Value as FCValue}; @@ -29,6 +29,16 @@ pub struct Context<'a> { pub isa: Option<&'a TargetIsa>, } +impl<'a> Context<'a> { + /// Get a `FlagsOrIsa` object for passing to the verifier. + pub fn flags_or_isa(&self) -> FlagsOrIsa<'a> { + FlagsOrIsa { + flags: self.flags, + isa: self.isa, + } + } +} + /// Common interface for implementations of test commands. /// /// Each `.cton` test file may contain multiple test commands, each represented by a `SubTest` diff --git a/cranelift/src/filetest/verifier.rs b/cranelift/src/filetest/verifier.rs index 7fa94f7b5e..7834fa778d 100644 --- a/cranelift/src/filetest/verifier.rs +++ b/cranelift/src/filetest/verifier.rs @@ -51,7 +51,7 @@ impl SubTest for TestVerifier { } } - match verify_function(func, context.isa) { + match verify_function(func, context.flags_or_isa()) { Ok(_) => { match expected { None => Ok(()), diff --git a/cranelift/src/wasm.rs b/cranelift/src/wasm.rs index 7ed47a2a9d..e9f9f39c30 100644 --- a/cranelift/src/wasm.rs +++ b/cranelift/src/wasm.rs @@ -61,7 +61,6 @@ pub fn run( &mut flag_builder, &Location { line_number: 0 }, ).map_err(|err| err.to_string())?; - let flags = settings::Flags::new(&flag_builder); let mut words = flag_isa.trim().split_whitespace(); // Look for `isa foo`. @@ -84,7 +83,6 @@ pub fn run( flag_check, path.to_path_buf(), name, - &flags, &*isa, )?; } @@ -97,7 +95,6 @@ fn handle_module( flag_check: bool, path: PathBuf, name: String, - flags: &settings::Flags, isa: &TargetIsa, ) -> Result<(), String> { let mut terminal = term::stdout().unwrap(); @@ -144,7 +141,7 @@ fn handle_module( } } }; - let mut dummy_runtime = DummyRuntime::with_flags(flags.clone()); + let mut dummy_runtime = DummyRuntime::with_flags(isa.flags().clone()); let translation = { let runtime: &mut WasmRuntime = &mut dummy_runtime; translate_module(&data, runtime)? @@ -157,8 +154,8 @@ fn handle_module( vprint!(flag_verbose, "Checking... "); terminal.reset().unwrap(); for func in &translation.functions { - verifier::verify_function(func, Some(isa)).map_err(|err| { - pretty_verifier_error(func, Some(isa), err) + verifier::verify_function(func, isa).map_err(|err| { + pretty_verifier_error(func, None, err) })?; } terminal.fg(term::color::GREEN).unwrap(); @@ -172,18 +169,18 @@ fn handle_module( for func in &translation.functions { let mut context = Context::new(); context.func = func.clone(); - context.verify(Some(isa)).map_err(|err| { - pretty_verifier_error(&context.func, Some(isa), err) + context.verify(isa).map_err(|err| { + pretty_verifier_error(&context.func, None, err) })?; context.flowgraph(); context.compute_loop_analysis(); context.licm(); - context.verify(Some(isa)).map_err(|err| { - pretty_verifier_error(&context.func, Some(isa), err) + context.verify(isa).map_err(|err| { + pretty_verifier_error(&context.func, None, err) })?; context.simple_gvn(); - context.verify(Some(isa)).map_err(|err| { - pretty_verifier_error(&context.func, Some(isa), err) + context.verify(isa).map_err(|err| { + pretty_verifier_error(&context.func, None, err) })?; } terminal.fg(term::color::GREEN).unwrap(); diff --git a/lib/cretonne/src/context.rs b/lib/cretonne/src/context.rs index 398fc33c71..30f67a811f 100644 --- a/lib/cretonne/src/context.rs +++ b/lib/cretonne/src/context.rs @@ -18,6 +18,7 @@ use isa::TargetIsa; use legalize_function; use regalloc; use result::{CtonError, CtonResult}; +use settings::FlagsOrIsa; use verifier; use simple_gvn::do_simple_gvn; use licm::do_licm; @@ -86,17 +87,14 @@ impl Context { /// Run the verifier on the function. /// /// Also check that the dominator tree and control flow graph are consistent with the function. - /// - /// The `isa` argument is currently unused, but the verifier will soon be able to also - /// check ISA-dependent constraints. - pub fn verify(&self, isa: Option<&TargetIsa>) -> verifier::Result { - verifier::verify_context(&self.func, &self.cfg, &self.domtree, isa) + pub fn verify<'a, FOI: Into>>(&self, fisa: FOI) -> verifier::Result { + verifier::verify_context(&self.func, &self.cfg, &self.domtree, fisa) } /// Run the verifier only if the `enable_verifier` setting is true. pub fn verify_if(&self, isa: &TargetIsa) -> CtonResult { if isa.flags().enable_verifier() { - self.verify(Some(isa)).map_err(Into::into) + self.verify(isa).map_err(Into::into) } else { Ok(()) } diff --git a/lib/cretonne/src/dominator_tree.rs b/lib/cretonne/src/dominator_tree.rs index faf369fcf6..cf8ecc8cb9 100644 --- a/lib/cretonne/src/dominator_tree.rs +++ b/lib/cretonne/src/dominator_tree.rs @@ -427,9 +427,10 @@ impl DominatorTree { mod test { use cursor::{Cursor, FuncCursor}; use flowgraph::ControlFlowGraph; - use ir::{Function, InstBuilder, types}; - use super::*; use ir::types::*; + use ir::{Function, InstBuilder, types}; + use settings; + use super::*; use verifier::verify_context; #[test] @@ -608,6 +609,8 @@ mod test { dt.recompute_split_ebb(ebb3, ebb4, middle_jump_inst); cfg.compute(cur.func); - verify_context(cur.func, &cfg, &dt, None).unwrap(); + + let flags = settings::Flags::new(&settings::builder()); + verify_context(cur.func, &cfg, &dt, &flags).unwrap(); } } diff --git a/lib/cretonne/src/regalloc/context.rs b/lib/cretonne/src/regalloc/context.rs index 1cb7d3f129..5e5c17c113 100644 --- a/lib/cretonne/src/regalloc/context.rs +++ b/lib/cretonne/src/regalloc/context.rs @@ -87,7 +87,7 @@ impl Context { ); if isa.flags().enable_verifier() { - verify_context(func, cfg, domtree, Some(isa))?; + verify_context(func, cfg, domtree, isa)?; verify_liveness(isa, func, cfg, &self.liveness)?; verify_cssa(func, cfg, domtree, &self.liveness, &self.virtregs)?; } @@ -105,7 +105,7 @@ impl Context { ); if isa.flags().enable_verifier() { - verify_context(func, cfg, domtree, Some(isa))?; + verify_context(func, cfg, domtree, isa)?; verify_liveness(isa, func, cfg, &self.liveness)?; verify_cssa(func, cfg, domtree, &self.liveness, &self.virtregs)?; } @@ -121,7 +121,7 @@ impl Context { ); if isa.flags().enable_verifier() { - verify_context(func, cfg, domtree, Some(isa))?; + verify_context(func, cfg, domtree, isa)?; verify_liveness(isa, func, cfg, &self.liveness)?; verify_cssa(func, cfg, domtree, &self.liveness, &self.virtregs)?; } @@ -136,7 +136,7 @@ impl Context { ); if isa.flags().enable_verifier() { - verify_context(func, cfg, domtree, Some(isa))?; + verify_context(func, cfg, domtree, isa)?; verify_liveness(isa, func, cfg, &self.liveness)?; verify_cssa(func, cfg, domtree, &self.liveness, &self.virtregs)?; } diff --git a/lib/cretonne/src/settings.rs b/lib/cretonne/src/settings.rs index 43d1639028..67ee68a214 100644 --- a/lib/cretonne/src/settings.rs +++ b/lib/cretonne/src/settings.rs @@ -20,11 +20,11 @@ //! assert_eq!(f.opt_level(), settings::OptLevel::Fastest); //! ``` +use constant_hash::{probe, simple_hash}; +use isa::TargetIsa; use std::fmt; use std::result; -use constant_hash::{probe, simple_hash}; - /// A string-based configurator for settings groups. /// /// The `Configurable` protocol allows settings to be modified by name before a finished `Flags` @@ -303,6 +303,34 @@ pub mod detail { // with an impl for all of the settings defined in `meta/cretonne/settings.py`. include!(concat!(env!("OUT_DIR"), "/settings.rs")); +/// Wrapper containing flags and optionally a `TargetIsa` trait object. +/// +/// A few passes need to access the flags but only optionally a target ISA. The `FlagsOrIsa` +/// wrapper can be used to pass either, and extract the flags so they are always accessible. +#[derive(Clone, Copy)] +pub struct FlagsOrIsa<'a> { + /// Flags are always present. + pub flags: &'a Flags, + + /// The ISA may not be present. + pub isa: Option<&'a TargetIsa>, +} + +impl<'a> From<&'a Flags> for FlagsOrIsa<'a> { + fn from(flags: &'a Flags) -> FlagsOrIsa { + FlagsOrIsa { flags, isa: None } + } +} + +impl<'a> From<&'a TargetIsa> for FlagsOrIsa<'a> { + fn from(isa: &'a TargetIsa) -> FlagsOrIsa { + FlagsOrIsa { + flags: isa.flags(), + isa: Some(isa), + } + } +} + #[cfg(test)] mod tests { use super::{builder, Flags}; diff --git a/lib/cretonne/src/verifier/mod.rs b/lib/cretonne/src/verifier/mod.rs index 118192786b..f7170bbd24 100644 --- a/lib/cretonne/src/verifier/mod.rs +++ b/lib/cretonne/src/verifier/mod.rs @@ -66,6 +66,7 @@ use ir::instructions::{InstructionFormat, BranchInfo, ResolvedConstraint, CallIn use ir::{types, Function, ValueDef, Ebb, Inst, SigRef, FuncRef, ValueList, JumpTable, StackSlot, StackSlotKind, GlobalVar, Value, Type, Opcode, ValueLoc, ArgumentLoc}; use isa::TargetIsa; +use settings::{Flags, FlagsOrIsa}; use std::error as std_error; use std::fmt::{self, Display, Formatter}; use std::result; @@ -121,19 +122,19 @@ impl std_error::Error for Error { pub type Result = result::Result<(), Error>; /// Verify `func`. -pub fn verify_function(func: &Function, isa: Option<&TargetIsa>) -> Result { - Verifier::new(func, isa).run() +pub fn verify_function<'a, FOI: Into>>(func: &Function, fisa: FOI) -> Result { + Verifier::new(func, fisa.into()).run() } /// Verify `func` after checking the integrity of associated context data structures `cfg` and /// `domtree`. -pub fn verify_context( +pub fn verify_context<'a, FOI: Into>>( func: &Function, cfg: &ControlFlowGraph, domtree: &DominatorTree, - isa: Option<&TargetIsa>, + fisa: FOI, ) -> Result { - let verifier = Verifier::new(func, isa); + let verifier = Verifier::new(func, fisa.into()); if cfg.is_valid() { verifier.cfg_integrity(cfg)?; } @@ -147,18 +148,20 @@ struct Verifier<'a> { func: &'a Function, cfg: ControlFlowGraph, domtree: DominatorTree, + flags: &'a Flags, isa: Option<&'a TargetIsa>, } impl<'a> Verifier<'a> { - pub fn new(func: &'a Function, isa: Option<&'a TargetIsa>) -> Verifier<'a> { + pub fn new(func: &'a Function, fisa: FlagsOrIsa<'a>) -> Verifier<'a> { let cfg = ControlFlowGraph::with_function(func); let domtree = DominatorTree::with_function(func, &cfg); Verifier { func, cfg, domtree, - isa, + flags: fisa.flags, + isa: fisa.isa, } } @@ -971,7 +974,7 @@ impl<'a> Verifier<'a> { } } - if self.isa.map(|isa| isa.flags().return_at_end()) == Some(true) { + if self.flags.return_at_end() { self.verify_return_at_end()?; } @@ -984,6 +987,7 @@ mod tests { use super::{Verifier, Error}; use ir::Function; use ir::instructions::{InstructionData, Opcode}; + use settings; macro_rules! assert_err_with_msg { ($e:expr, $msg:expr) => ( @@ -1001,7 +1005,8 @@ mod tests { #[test] fn empty() { let func = Function::new(); - let verifier = Verifier::new(&func, None); + let flags = &settings::Flags::new(&settings::builder()); + let verifier = Verifier::new(&func, flags.into()); assert_eq!(verifier.run(), Ok(())); } @@ -1014,7 +1019,8 @@ mod tests { InstructionData::Nullary { opcode: Opcode::Jump }, ); func.layout.append_inst(nullary_with_bad_opcode, ebb0); - let verifier = Verifier::new(&func, None); + let flags = &settings::Flags::new(&settings::builder()); + let verifier = Verifier::new(&func, flags.into()); assert_err_with_msg!(verifier.run(), "instruction format"); } } diff --git a/lib/frontend/src/frontend.rs b/lib/frontend/src/frontend.rs index 696dad6556..29a4f3f644 100644 --- a/lib/frontend/src/frontend.rs +++ b/lib/frontend/src/frontend.rs @@ -631,6 +631,7 @@ mod tests { use cretonne::ir::types::*; use frontend::{ILBuilder, FunctionBuilder}; use cretonne::verifier::verify_function; + use cretonne::settings; use std::u32; @@ -727,7 +728,8 @@ mod tests { builder.seal_block(block1); } - let res = verify_function(&func, None); + let flags = settings::Flags::new(&settings::builder()); + let res = verify_function(&func, &flags); // println!("{}", func.display(None)); match res { Ok(_) => {} diff --git a/lib/frontend/src/lib.rs b/lib/frontend/src/lib.rs index af31a2c723..cdf2361489 100644 --- a/lib/frontend/src/lib.rs +++ b/lib/frontend/src/lib.rs @@ -38,6 +38,7 @@ //! use cretonne::entity::EntityRef; //! use cretonne::ir::{FunctionName, CallConv, Function, Signature, ArgumentType, InstBuilder}; //! use cretonne::ir::types::*; +//! use cretonne::settings; //! use cton_frontend::{ILBuilder, FunctionBuilder}; //! use cretonne::verifier::verify_function; //! use std::u32; @@ -133,7 +134,8 @@ //! builder.seal_block(block1); //! } //! -//! let res = verify_function(&func, None); +//! let flags = settings::Flags::new(&settings::builder()); +//! let res = verify_function(&func, &flags); //! println!("{}", func.display(None)); //! match res { //! Ok(_) => {} diff --git a/lib/frontend/src/ssa.rs b/lib/frontend/src/ssa.rs index 3dd0335009..ed2847c93e 100644 --- a/lib/frontend/src/ssa.rs +++ b/lib/frontend/src/ssa.rs @@ -589,6 +589,7 @@ mod tests { use cretonne::ir::types::*; use cretonne::verify_function; use cretonne::ir::instructions::BranchInfo; + use cretonne::settings; use ssa::SSABuilder; use std::u32; @@ -1194,7 +1195,8 @@ mod tests { cur.goto_bottom(ebb1); func.dfg.ins(cur).return_(&[]) }; - match verify_function(&func, None) { + let flags = settings::Flags::new(&settings::builder()); + match verify_function(&func, &flags) { Ok(()) => {} Err(err) => panic!(err.message), } diff --git a/lib/wasm/src/func_translator.rs b/lib/wasm/src/func_translator.rs index acbb43b36b..c38c420bbb 100644 --- a/lib/wasm/src/func_translator.rs +++ b/lib/wasm/src/func_translator.rs @@ -218,7 +218,7 @@ fn parse_function_body( mod tests { use cretonne::{ir, Context}; use cretonne::ir::types::I32; - use runtime::DummyRuntime; + use runtime::{DummyRuntime, FuncEnvironment}; use super::FuncTranslator; #[test] @@ -250,7 +250,7 @@ mod tests { trans.translate(&BODY, &mut ctx.func, &mut runtime).unwrap(); dbg!("{}", ctx.func.display(None)); - ctx.verify(None).unwrap(); + ctx.verify(runtime.flags()).unwrap(); } #[test] @@ -283,7 +283,7 @@ mod tests { trans.translate(&BODY, &mut ctx.func, &mut runtime).unwrap(); dbg!("{}", ctx.func.display(None)); - ctx.verify(None).unwrap(); + ctx.verify(runtime.flags()).unwrap(); } #[test] @@ -322,6 +322,6 @@ mod tests { trans.translate(&BODY, &mut ctx.func, &mut runtime).unwrap(); dbg!("{}", ctx.func.display(None)); - ctx.verify(None).unwrap(); + ctx.verify(runtime.flags()).unwrap(); } } diff --git a/lib/wasm/tests/testsuite.rs b/lib/wasm/tests/testsuite.rs index 96d1a645ef..b5fe5997e8 100644 --- a/lib/wasm/tests/testsuite.rs +++ b/lib/wasm/tests/testsuite.rs @@ -4,7 +4,6 @@ extern crate tempdir; use cton_wasm::{translate_module, DummyRuntime, WasmRuntime}; use std::path::PathBuf; -use std::borrow::Borrow; use std::fs::File; use std::error::Error; use std::io; @@ -15,8 +14,8 @@ use std::process::Command; use std::fs; use cretonne::ir; use cretonne::ir::entities::AnyEntity; -use cretonne::isa::{self, TargetIsa}; -use cretonne::settings::{self, Configurable}; +use cretonne::isa::TargetIsa; +use cretonne::settings::{self, Configurable, Flags}; use cretonne::verifier; use tempdir::TempDir; @@ -27,9 +26,10 @@ fn testsuite() { .map(|r| r.unwrap()) .collect(); paths.sort_by_key(|dir| dir.path()); + let flags = Flags::new(&settings::builder()); for path in paths { let path = path.path(); - handle_module(path, None); + handle_module(path, &flags); } } @@ -37,19 +37,8 @@ fn testsuite() { fn return_at_end() { let mut flag_builder = settings::builder(); flag_builder.enable("return_at_end").unwrap(); - let flags = settings::Flags::new(&flag_builder); - // We don't care about the target itself here, so just pick one arbitrarily. - let isa = match isa::lookup("riscv") { - Err(_) => { - println!("riscv target not found; disabled test return_at_end.wat"); - return; - } - Ok(isa_builder) => isa_builder.finish(flags), - }; - handle_module( - PathBuf::from("../../wasmtests/return_at_end.wat"), - Some(isa.borrow()), - ); + let flags = Flags::new(&flag_builder); + handle_module(PathBuf::from("../../wasmtests/return_at_end.wat"), &flags); } fn read_wasm_file(path: PathBuf) -> Result, io::Error> { @@ -60,7 +49,7 @@ fn read_wasm_file(path: PathBuf) -> Result, io::Error> { Ok(buf) } -fn handle_module(path: PathBuf, isa: Option<&TargetIsa>) { +fn handle_module(path: PathBuf, flags: &Flags) { let data = match path.extension() { None => { panic!("the file extension is not wasm or wat"); @@ -105,17 +94,14 @@ fn handle_module(path: PathBuf, isa: Option<&TargetIsa>) { } } }; - let mut dummy_runtime = match isa { - Some(isa) => DummyRuntime::with_flags(isa.flags().clone()), - None => DummyRuntime::default(), - }; + let mut dummy_runtime = DummyRuntime::with_flags(flags.clone()); let translation = { let runtime: &mut WasmRuntime = &mut dummy_runtime; translate_module(&data, runtime).unwrap() }; for func in &translation.functions { - verifier::verify_function(func, isa) - .map_err(|err| panic!(pretty_verifier_error(func, isa, err))) + verifier::verify_function(func, flags) + .map_err(|err| panic!(pretty_verifier_error(func, None, err))) .unwrap(); } }