Add options for parsing test files (#942)

* Add options for parsing test files

This change allows adding parsing parameters more easily; e.g. a parameter is needed for setting the default calling convention for functions parsed as a part of the `run` test feature.

* Set default calling convention that of the host for `test run` file tests

Previously `test run` used the parser's hard-coded CallConv::Fast as the default calling convention but with this change any test being `run` will use the default calling convention of the machine running the test. `test run` will now throw an error if the calling convention of the function does not match the host's.

cranelift/filetests/src/function_runner.rs

@@ -1,7 +1,7 @@
 use core::mem;
 use cranelift_codegen::binemit::{NullRelocSink, NullStackmapSink, NullTrapSink};
 use cranelift_codegen::ir::Function;
-use cranelift_codegen::isa::{CallConv, TargetIsa};
+use cranelift_codegen::isa::TargetIsa;
 use cranelift_codegen::{settings, Context};
 use cranelift_native::builder as host_isa_builder;
 use memmap::MmapMut;
@@ -47,10 +47,7 @@ impl FunctionRunner {
             ));
         }
 
-        if func.signature.call_conv != self.isa.default_call_conv()
+        if func.signature.call_conv != self.isa.default_call_conv() {
-            && func.signature.call_conv != CallConv::Fast
-        {
-            // ideally we wouldn't have to also check for Fast here but currently there is no way to inform the filetest parser that we would like to use a default other than Fast
             return Err(String::from(
                 "Functions only run on the host's default calling convention; remove the specified calling convention in the function signature to use the host's default.",
             ));
@@ -97,7 +94,7 @@ impl FunctionRunner {
 #[cfg(test)]
 mod test {
     use super::*;
-    use cranelift_reader::parse_test;
+    use cranelift_reader::{parse_test, ParseOptions};
 
     #[test]
     fn nop() {
@@ -111,7 +108,7 @@ mod test {
         );
 
         // extract function
-        let test_file = parse_test(code.as_str(), None, None).unwrap();
+        let test_file = parse_test(code.as_str(), ParseOptions::default()).unwrap();
         assert_eq!(1, test_file.functions.len());
         let function = test_file.functions[0].0.clone();
 

cranelift/filetests/src/runone.rs

@@ -8,8 +8,8 @@ use cranelift_codegen::print_errors::pretty_verifier_error;
 use cranelift_codegen::settings::Flags;
 use cranelift_codegen::timing;
 use cranelift_codegen::verify_function;
-use cranelift_reader::parse_test;
 use cranelift_reader::IsaSpec;
+use cranelift_reader::{parse_test, ParseOptions};
 use log::info;
 use std::borrow::Cow;
 use std::fs;
@@ -33,8 +33,13 @@ 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 options = ParseOptions {
+        target,
+        passes,
+        ..ParseOptions::default()
+    };
 
-    let testfile = match parse_test(&buffer, passes, target) {
+    let testfile = match parse_test(&buffer, options) {
         Ok(testfile) => testfile,
         Err(e) => {
             if e.is_warning {

cranelift/reader/src/lib.rs

@@ -28,7 +28,7 @@
 
 pub use crate::error::{Location, ParseError, ParseResult};
 pub use crate::isaspec::{parse_options, IsaSpec};
-pub use crate::parser::{parse_functions, parse_test};
+pub use crate::parser::{parse_functions, parse_test, ParseOptions};
 pub use crate::sourcemap::SourceMap;
 pub use crate::testcommand::{TestCommand, TestOption};
 pub use crate::testfile::{Comment, Details, TestFile};

cranelift/reader/src/parser.rs

@@ -31,20 +31,37 @@ use target_lexicon::Triple;
 /// Any test commands or target declarations are ignored.
 pub fn parse_functions(text: &str) -> ParseResult<Vec<Function>> {
     let _tt = timing::parse_text();
-    parse_test(text, None, None)
+    parse_test(text, ParseOptions::default())
         .map(|file| file.functions.into_iter().map(|(func, _)| func).collect())
 }
 
+/// Options for configuring the parsing of filetests.
+pub struct ParseOptions<'a> {
+    /// Compiler passes to run on the parsed functions.
+    pub passes: Option<&'a [String]>,
+    /// Target ISA for compiling the parsed functions, e.g. "x86_64 skylake".
+    pub target: Option<&'a str>,
+    /// Default calling convention used when none is specified for a parsed function.
+    pub default_calling_convention: CallConv,
+}
+
+impl Default for ParseOptions<'_> {
+    fn default() -> Self {
+        Self {
+            passes: None,
+            target: None,
+            default_calling_convention: CallConv::Fast,
+        }
+    }
+}
+
 /// Parse the entire `text` as a test case file.
 ///
 /// The returned `TestFile` contains direct references to substrings of `text`.
-pub fn parse_test<'a>(
+pub fn parse_test<'a>(text: &'a str, options: ParseOptions<'a>) -> ParseResult<TestFile<'a>> {
-    text: &'a str,
-    passes: Option<&'a [String]>,
-    target: Option<&str>,
-) -> ParseResult<TestFile<'a>> {
     let _tt = timing::parse_text();
     let mut parser = Parser::new(text);
+
     // Gather the preamble comments.
     parser.start_gathering_comments();
 
@@ -53,12 +70,12 @@ pub fn parse_test<'a>(
 
     // Check for specified passes and target, if present throw out test commands/targets specified
     // in file.
-    match passes {
+    match options.passes {
         Some(pass_vec) => {
             parser.parse_test_commands();
             commands = parser.parse_cmdline_passes(pass_vec);
             parser.parse_target_specs()?;
-            isa_spec = parser.parse_cmdline_target(target)?;
+            isa_spec = parser.parse_cmdline_target(options.target)?;
         }
         None => {
             commands = parser.parse_test_commands();
@@ -66,6 +83,16 @@ pub fn parse_test<'a>(
         }
     };
 
+    // Decide between using the calling convention passed in the options or using the
+    // host's calling convention--if any tests are to be run on the host we should default to the
+    // host's calling convention.
+    parser = if commands.iter().any(|tc| tc.command == "run") {
+        let host_default_calling_convention = CallConv::triple_default(&Triple::host());
+        parser.with_default_calling_convention(host_default_calling_convention)
+    } else {
+        parser.with_default_calling_convention(options.default_calling_convention)
+    };
+
     parser.token();
     parser.claim_gathered_comments(AnyEntity::Function);
 
@@ -99,6 +126,9 @@ pub struct Parser<'a> {
 
     /// Comments collected so far.
     comments: Vec<Comment<'a>>,
+
+    /// Default calling conventions; used when none is specified.
+    default_calling_convention: CallConv,
 }
 
 /// Context for resolving references when parsing a single function.
@@ -235,11 +265,16 @@ impl<'a> Context<'a> {
     }
 
     // Allocate a new signature.
-    fn add_sig(&mut self, sig: SigRef, data: Signature, loc: Location) -> ParseResult<()> {
+    fn add_sig(
+        &mut self,
+        sig: SigRef,
+        data: Signature,
+        loc: Location,
+        defaultcc: CallConv,
+    ) -> ParseResult<()> {
         self.map.def_sig(sig, loc)?;
         while self.function.dfg.signatures.next_key().index() <= sig.index() {
-            self.function
+            self.function.import_signature(Signature::new(defaultcc));
-                .import_signature(Signature::new(CallConv::Fast));
         }
         self.function.dfg.signatures[sig] = data;
         Ok(())
@@ -318,6 +353,16 @@ impl<'a> Parser<'a> {
             gathering_comments: false,
             gathered_comments: Vec::new(),
             comments: Vec::new(),
+            default_calling_convention: CallConv::Fast,
+        }
+    }
+
+    /// Modify the default calling convention; returns a new parser with the changed calling
+    /// convention.
+    pub fn with_default_calling_convention(self, default_calling_convention: CallConv) -> Self {
+        Self {
+            default_calling_convention,
+            ..self
         }
     }
 
@@ -1018,7 +1063,7 @@ impl<'a> Parser<'a> {
     //
     fn parse_signature(&mut self, unique_isa: Option<&dyn TargetIsa>) -> ParseResult<Signature> {
         // Calling convention defaults to `fast`, but can be changed.
-        let mut sig = Signature::new(CallConv::Fast);
+        let mut sig = Signature::new(self.default_calling_convention);
 
         self.match_token(Token::LPar, "expected function signature: ( args... )")?;
         // signature ::=  "(" * [abi-param-list] ")" ["->" retlist] [callconv]
@@ -1170,7 +1215,9 @@ impl<'a> Parser<'a> {
                 Some(Token::SigRef(..)) => {
                     self.start_gathering_comments();
                     self.parse_signature_decl(ctx.unique_isa)
-                        .and_then(|(sig, dat)| ctx.add_sig(sig, dat, self.loc))
+                        .and_then(|(sig, dat)| {
+                            ctx.add_sig(sig, dat, self.loc, self.default_calling_convention)
+                        })
                 }
                 Some(Token::FuncRef(..)) => {
                     self.start_gathering_comments();
@@ -2951,8 +2998,7 @@ mod tests {
                              set enable_float=false
                              ; still preamble
                              function %comment() system_v {}",
-            None,
+            ParseOptions::default(),
-            None,
         )
         .unwrap();
         assert_eq!(tf.commands.len(), 2);
@@ -2978,6 +3024,7 @@ mod tests {
         assert!(parse_test(
             "target
                             function %foo() system_v {}",
+            ParseOptions::default()
         )
         .is_err());
 
@@ -2985,6 +3032,7 @@ mod tests {
             "target riscv32
                             set enable_float=false
                             function %foo() system_v {}",
+            ParseOptions::default()
         )
         .is_err());
 
@@ -2992,6 +3040,7 @@ mod tests {
             "set enable_float=false
                           isa riscv
                           function %foo() system_v {}",
+            ParseOptions::default(),
         )
         .unwrap()
         .isa_spec
@@ -3052,4 +3101,26 @@ mod tests {
         );
         assert!(parser.parse_function(None).is_err());
     }
+
+    #[test]
+    fn change_default_calling_convention() {
+        let code = "function %test() {
+        ebb0:
+            return
+        }";
+
+        // By default the parser will use the fast calling convention if none is specified.
+        let mut parser = Parser::new(code);
+        assert_eq!(
+            parser.parse_function(None).unwrap().0.signature.call_conv,
+            CallConv::Fast
+        );
+
+        // However, we can specify a different calling convention to be the default.
+        let mut parser = Parser::new(code).with_default_calling_convention(CallConv::Cold);
+        assert_eq!(
+            parser.parse_function(None).unwrap().0.signature.call_conv,
+            CallConv::Cold
+        );
+    }
 }

cranelift/reader/src/sourcemap.rs

@@ -211,7 +211,7 @@ impl SourceMap {
 
 #[cfg(test)]
 mod tests {
-    use crate::parse_test;
+    use crate::{parse_test, ParseOptions};
 
     #[test]
     fn details() {
@@ -222,8 +222,7 @@ mod tests {
                              ebb0(v4: i32, v7: i32):
                                v10 = iadd v4, v7
                              }",
-            None,
+            ParseOptions::default(),
-            None,
         )
         .unwrap();
         let map = &tf.functions[0].1.map;

cranelift/src/bugpoint.rs

@@ -9,7 +9,7 @@ use cranelift_codegen::ir::{
 use cranelift_codegen::isa::TargetIsa;
 use cranelift_codegen::Context;
 use cranelift_entity::PrimaryMap;
-use cranelift_reader::parse_test;
+use cranelift_reader::{parse_test, ParseOptions};
 use std::collections::HashMap;
 use std::path::Path;
 
@@ -27,7 +27,8 @@ pub fn run(
     let path = Path::new(&filename).to_path_buf();
 
     let buffer = read_to_string(&path).map_err(|e| format!("{}: {}", filename, e))?;
-    let test_file = parse_test(&buffer, None, None).map_err(|e| format!("{}: {}", filename, e))?;
+    let test_file =
+        parse_test(&buffer, ParseOptions::default()).map_err(|e| format!("{}: {}", filename, e))?;
 
     // If we have an isa from the command-line, use that. Otherwise if the
     // file contains a unique isa, use that.
@@ -754,12 +755,13 @@ impl<'a> CrashCheckContext<'a> {
 #[cfg(test)]
 mod tests {
     use super::*;
+    use cranelift_reader::ParseOptions;
 
     #[test]
     fn test_reduce() {
         const TEST: &'static str = include_str!("./bugpoint_test.clif");
 
-        let test_file = parse_test(TEST, None, None).unwrap();
+        let test_file = parse_test(TEST, ParseOptions::default()).unwrap();
 
         // If we have an isa from the command-line, use that. Otherwise if the
         // file contains a unique isa, use that.

cranelift/src/compile.rs

@@ -6,7 +6,7 @@ use cranelift_codegen::print_errors::pretty_error;
 use cranelift_codegen::settings::FlagsOrIsa;
 use cranelift_codegen::timing;
 use cranelift_codegen::Context;
-use cranelift_reader::parse_test;
+use cranelift_reader::{parse_test, ParseOptions};
 use std::path::Path;
 use std::path::PathBuf;
 
@@ -44,7 +44,8 @@ fn handle_module(
     fisa: FlagsOrIsa,
 ) -> Result<(), String> {
     let buffer = read_to_string(&path).map_err(|e| format!("{}: {}", name, e))?;
-    let test_file = parse_test(&buffer, None, None).map_err(|e| format!("{}: {}", name, e))?;
+    let test_file =
+        parse_test(&buffer, ParseOptions::default()).map_err(|e| format!("{}: {}", name, e))?;
 
     // If we have an isa from the command-line, use that. Otherwise if the
     // file contains a unique isa, use that.

cranelift/src/run.rs

@@ -1,11 +1,12 @@
 //! CLI tool to compile Cranelift IR files to native code in memory and execute them.
 
 use crate::utils::read_to_string;
-use cranelift_codegen::isa::TargetIsa;
+use cranelift_codegen::isa::{CallConv, TargetIsa};
 use cranelift_filetests::FunctionRunner;
 use cranelift_native::builder as host_isa_builder;
-use cranelift_reader::{parse_test, Details, IsaSpec};
+use cranelift_reader::{parse_test, Details, IsaSpec, ParseOptions};
 use std::path::PathBuf;
+use target_lexicon::Triple;
 use walkdir::WalkDir;
 
 pub fn run(files: Vec<String>, flag_print: bool) -> Result<(), String> {
@@ -74,7 +75,11 @@ fn run_single_file(path: &PathBuf) -> Result<(), String> {
 
 /// Main body of `run_single_file` separated for testing
 fn run_file_contents(file_contents: String) -> Result<(), String> {
-    let test_file = parse_test(&file_contents, None, None).map_err(|e| e.to_string())?;
+    let options = ParseOptions {
+        default_calling_convention: CallConv::triple_default(&Triple::host()), // use the host's default calling convention
+        ..ParseOptions::default()
+    };
+    let test_file = parse_test(&file_contents, options).map_err(|e| e.to_string())?;
     for (func, Details { comments, .. }) in test_file.functions {
         if comments.iter().any(|c| c.text.contains("run")) {
             let isa = create_target_isa(&test_file.isa_spec)?;