cranelift: Add ability to auto-update test expectations (#3612)

* cranelift: Add ability to auto-update test expectations

One of the problems of the current `*.clif` testing is that the files
are difficult to update when widespread changes are made (such as
removing modification of the frame pointer). Additionally when changing
register allocation or similar it can cause a large number of changes in
tests but the tests themselves didn't actually break. For this reason
this commit adds the ability to automatically update test expectations.

The idea behind this commit is that tests of the form `test compile` can
also optionally be flagged with the `precise-output` flag:

    test compile precise-output

and when doing so the compiled form of each function is asserted to 100%
match the following comments and their test expectations. If a match is
not found then a `BLESS=1` environment variable can be used to
automatically rewrite the test file itself with the correct assertion.
If the environment variable isn't present and the expectation doesn't
match then the test fails.

It's hoped that, if approved, a follow-up commit can add
`precise-output` to all current `test compile` tests (or make it the
default) and all tests can be mass-updated. When developing locally test
expectations need not be written and instead tests can be run with
`BLESS=1` and the output can be manually verified. The environment
variable will not be present on CI which means that changes to the
output which don't also change the test expectation will cause CI to
fail. Furthermore this should still make updates to the test output
easily readable in review on CI because the test expectations are
intended to look the same as before.

Closes #1539

* Use raw vcode output in tests

* Fix a merge conflict

* Review comments

cranelift/filetests/src/test_compile.rs

@@ -3,21 +3,34 @@
 //! The `compile` test command runs each function through the full code generator pipeline
 
 use crate::subtest::{run_filecheck, Context, SubTest};
-use cranelift_codegen;
+use anyhow::{bail, Result};
 use cranelift_codegen::binemit::{self, CodeInfo};
 use cranelift_codegen::ir;
-use cranelift_reader::TestCommand;
+use cranelift_reader::{TestCommand, TestOption};
 use log::info;
 use std::borrow::Cow;
+use std::env;
 
-struct TestCompile;
+struct TestCompile {
+    /// Flag indicating that the text expectation, comments after the function,
+    /// must be a precise 100% match on the compiled output of the function.
+    /// This test assertion is also automatically-update-able to allow tweaking
+    /// the code generator and easily updating all affected tests.
+    precise_output: bool,
+}
 
-pub fn subtest(parsed: &TestCommand) -> anyhow::Result<Box<dyn SubTest>> {
+pub fn subtest(parsed: &TestCommand) -> Result<Box<dyn SubTest>> {
     assert_eq!(parsed.command, "compile");
-    if !parsed.options.is_empty() {
-        anyhow::bail!("No options allowed on {}", parsed);
+    let mut test = TestCompile {
+        precise_output: false,
+    };
+    for option in parsed.options.iter() {
+        match option {
+            TestOption::Flag("precise-output") => test.precise_output = true,
+            _ => anyhow::bail!("unknown option on {}", parsed),
+        }
     }
-    Ok(Box::new(TestCompile))
+    Ok(Box::new(test))
 }
 
 impl SubTest for TestCompile {
@@ -33,7 +46,7 @@ impl SubTest for TestCompile {
         true
     }
 
-    fn run(&self, func: Cow<ir::Function>, context: &Context) -> anyhow::Result<()> {
+    fn run(&self, func: Cow<ir::Function>, context: &Context) -> Result<()> {
         let isa = context.isa.expect("compile needs an ISA");
         let mut comp_ctx = cranelift_codegen::Context::for_function(func.into_owned());
 
@@ -54,7 +67,11 @@ impl SubTest for TestCompile {
 
         info!("Generated {} bytes of code:\n{}", total_size, disasm);
 
-        run_filecheck(&disasm, context)
+        if self.precise_output {
+            check_precise_output(&disasm, context)
+        } else {
+            run_filecheck(&disasm, context)
+        }
     }
 }
 
@@ -95,3 +112,74 @@ impl binemit::CodeSink for SizeSink {
     fn trap(&mut self, _code: ir::TrapCode, _srcloc: ir::SourceLoc) {}
     fn end_codegen(&mut self) {}
 }
+
+fn check_precise_output(text: &str, context: &Context) -> Result<()> {
+    let actual = text.lines().collect::<Vec<_>>();
+
+    // Use the comments after the function to build the test expectation.
+    let expected = context
+        .details
+        .comments
+        .iter()
+        .map(|c| c.text.strip_prefix("; ").unwrap_or(c.text))
+        .collect::<Vec<_>>();
+
+    // If the expectation matches what we got, then there's nothing to do.
+    if actual == expected {
+        return Ok(());
+    }
+
+    // If we're supposed to automatically update the test, then do so here.
+    if env::var("CRANELIFT_TEST_BLESS").unwrap_or(String::new()) == "1" {
+        return update_test(&actual, context);
+    }
+
+    // Otherwise this test has failed, and we can print out as such.
+    bail!(
+        "compilation of function on line {} does not match\n\
+         the text expectation\n\
+         \n\
+         expected:\n\
+         {:#?}\n\
+         actual:\n\
+         {:#?}\n\
+         \n\
+         This test assertion can be automatically updated by setting the\n\
+         CRANELIFT_TEST_BLESS=1 environment variable when running this test.
+         ",
+        context.details.location.line_number,
+        expected,
+        actual,
+    )
+}
+
+fn update_test(output: &[&str], context: &Context) -> Result<()> {
+    context
+        .file_update
+        .update_at(&context.details.location, |new_test, old_test| {
+            // blank newline after the function
+            new_test.push_str("\n");
+
+            // Splice in the test output
+            for output in output {
+                new_test.push_str("; ");
+                new_test.push_str(output);
+                new_test.push_str("\n");
+            }
+
+            // blank newline after test assertion
+            new_test.push_str("\n");
+
+            // Drop all remaining commented lines (presumably the old test expectation),
+            // but after we hit a real line then we push all remaining lines.
+            let mut in_next_function = false;
+            for line in old_test {
+                if !in_next_function && (line.trim().is_empty() || line.starts_with(";")) {
+                    continue;
+                }
+                in_next_function = true;
+                new_test.push_str(line);
+                new_test.push_str("\n");
+            }
+        })
+}