From 4c3e590bb92870a53f74103ec57b656ac10d7c1a Mon Sep 17 00:00:00 2001
From: Jakob Stoklund Olesen <jolesen@mozilla.com>
Date: Wed, 29 Mar 2017 15:16:59 -0700
Subject: [PATCH] Run verifier after legalizer and regalloc file tests.

Run the verify_contexti() function after invoking the legalize() and
regalloc() context functions. This will help catch bad code produced by
these passes.
---
 cranelift/src/filetest/legalizer.rs | 15 +++++++++------
 cranelift/src/filetest/regalloc.rs  |  6 ++++--
 cranelift/src/utils.rs              | 18 +++++++++++++++++-
 lib/cretonne/src/context.rs         | 11 +++++++++++
 4 files changed, 41 insertions(+), 9 deletions(-)

diff --git a/cranelift/src/filetest/legalizer.rs b/cranelift/src/filetest/legalizer.rs
index 25247a5aba..39854769ac 100644
--- a/cranelift/src/filetest/legalizer.rs
+++ b/cranelift/src/filetest/legalizer.rs
@@ -4,11 +4,11 @@
 //! the result to filecheck.
 
 use std::borrow::Cow;
-use cretonne::{legalize_function, write_function};
-use cretonne::flowgraph::ControlFlowGraph;
+use cretonne::{self, write_function};
 use cretonne::ir::Function;
 use cton_reader::TestCommand;
 use filetest::subtest::{SubTest, Context, Result, run_filecheck};
+use utils::pretty_verifier_error;
 
 struct TestLegalizer;
 
@@ -35,13 +35,16 @@ impl SubTest for TestLegalizer {
     }
 
     fn run(&self, func: Cow<Function>, context: &Context) -> Result<()> {
-        let mut func = func.into_owned();
+        let mut comp_ctx = cretonne::Context::new();
+        comp_ctx.func = func.into_owned();
         let isa = context.isa.expect("legalizer needs an ISA");
-        let mut cfg = ControlFlowGraph::with_function(&func);
-        legalize_function(&mut func, &mut cfg, isa);
+
+        comp_ctx.flowgraph();
+        comp_ctx.legalize(isa);
+        comp_ctx.verify(isa).map_err(|e| pretty_verifier_error(&comp_ctx.func, e))?;
 
         let mut text = String::new();
-        write_function(&mut text, &func, Some(isa)).map_err(|e| e.to_string())?;
+        write_function(&mut text, &comp_ctx.func, Some(isa)).map_err(|e| e.to_string())?;
         run_filecheck(&text, context)
     }
 }
diff --git a/cranelift/src/filetest/regalloc.rs b/cranelift/src/filetest/regalloc.rs
index f64d7bb830..5f70f51100 100644
--- a/cranelift/src/filetest/regalloc.rs
+++ b/cranelift/src/filetest/regalloc.rs
@@ -5,11 +5,12 @@
 //!
 //! The resulting function is sent to `filecheck`.
 
-use std::borrow::Cow;
-use cretonne::{self, write_function};
 use cretonne::ir::Function;
+use cretonne::{self, write_function};
 use cton_reader::TestCommand;
 use filetest::subtest::{SubTest, Context, Result, run_filecheck};
+use std::borrow::Cow;
+use utils::pretty_verifier_error;
 
 struct TestRegalloc;
 
@@ -46,6 +47,7 @@ impl SubTest for TestRegalloc {
         // TODO: Should we have an option to skip legalization?
         comp_ctx.legalize(isa);
         comp_ctx.regalloc(isa);
+        comp_ctx.verify(isa).map_err(|e| pretty_verifier_error(&comp_ctx.func, e))?;
 
         let mut text = String::new();
         write_function(&mut text, &comp_ctx.func, Some(isa)).map_err(|e| e.to_string())?;
diff --git a/cranelift/src/utils.rs b/cranelift/src/utils.rs
index 9cbe20b4bd..d2a6fae3c7 100644
--- a/cranelift/src/utils.rs
+++ b/cranelift/src/utils.rs
@@ -1,8 +1,11 @@
 //! Utility functions.
 
-use std::path::Path;
+use cretonne::ir::entities::AnyEntity;
+use cretonne::{ir, verifier, write_function};
+use std::fmt::Write;
 use std::fs::File;
 use std::io::{Result, Read};
+use std::path::Path;
 
 /// Read an entire file into a string.
 pub fn read_to_string<P: AsRef<Path>>(path: P) -> Result<String> {
@@ -29,6 +32,19 @@ pub fn match_directive<'a>(comment: &'a str, directive: &str) -> Option<&'a str>
     }
 }
 
+/// Pretty-print a verifier error.
+pub fn pretty_verifier_error(func: &ir::Function, err: verifier::Error) -> String {
+    let mut msg = err.to_string();
+    match err.location {
+        AnyEntity::Inst(inst) => {
+            write!(msg, "\n{}: {}\n\n", inst, func.dfg.display_inst(inst)).unwrap()
+        }
+        _ => {}
+    }
+    write_function(&mut msg, func, None).unwrap();
+    msg
+}
+
 #[test]
 fn test_match_directive() {
     assert_eq!(match_directive("; foo: bar  ", "foo:"), Some("bar"));
diff --git a/lib/cretonne/src/context.rs b/lib/cretonne/src/context.rs
index 9ce9d4912e..2d5631f62c 100644
--- a/lib/cretonne/src/context.rs
+++ b/lib/cretonne/src/context.rs
@@ -15,6 +15,7 @@ use ir::Function;
 use isa::TargetIsa;
 use legalize_function;
 use regalloc;
+use verifier;
 
 /// Persistent data structures and compilation pipeline.
 pub struct Context {
@@ -45,6 +46,16 @@ impl Context {
         }
     }
 
+    /// Run the verifier on the function.
+    ///
+    /// Also check that the dominator tree and control flow graph are consistent with the function.
+    ///
+    /// The `TargetIsa` argument is currently unused, but the verifier will soon be able to also
+    /// check ISA-dependent constraints.
+    pub fn verify<'a, ISA: Into<Option<&'a TargetIsa>>>(&self, _isa: ISA) -> verifier::Result<()> {
+        verifier::verify_context(self)
+    }
+
     /// Run the legalizer for `isa` on the function.
     pub fn legalize(&mut self, isa: &TargetIsa) {
         legalize_function(&mut self.func, &mut self.cfg, isa);