Move verify calls out of Context.

Also, move flowgraph() calls out of filetest and into the passes that
need them so that filetest doesn't have embedded knowledge of these
dependencies.

This resolves a TODO about the way Context was running the verifier, and
it makes the Context functions and the filetest runners more transparent.

This also fixes simple_gvn to use the existing dominator tree rather
than computing its own.

cranelift/src/filetest/licm.rs

@@ -38,9 +38,9 @@ impl SubTest for TestLICM {
         let mut comp_ctx = cretonne::Context::new();
         comp_ctx.func = func.into_owned();
 
-        comp_ctx.flowgraph();
+        comp_ctx.licm();
-        comp_ctx.licm().map_err(|e| {
+        comp_ctx.verify(context.isa).map_err(|e| {
-            pretty_error(&comp_ctx.func, context.isa, e)
+            pretty_error(&comp_ctx.func, context.isa, Into::into(e))
         })?;
 
         let mut text = String::new();

cranelift/src/filetest/simple_gvn.rs

@@ -38,9 +38,9 @@ impl SubTest for TestSimpleGVN {
         let mut comp_ctx = cretonne::Context::new();
         comp_ctx.func = func.into_owned();
 
-        comp_ctx.flowgraph();
+        comp_ctx.simple_gvn();
-        comp_ctx.simple_gvn().map_err(|e| {
+        comp_ctx.verify(context.isa).map_err(|e| {
-            pretty_error(&comp_ctx.func, context.isa, e)
+            pretty_error(&comp_ctx.func, context.isa, Into::into(e))
         })?;
 
         let mut text = String::new();

cranelift/src/wasm.rs

@@ -10,7 +10,6 @@ use cretonne::loop_analysis::LoopAnalysis;
 use cretonne::flowgraph::ControlFlowGraph;
 use cretonne::dominator_tree::DominatorTree;
 use cretonne::Context;
-use cretonne::result::CtonError;
 use cretonne::verifier;
 use cretonne::settings::{self, Configurable};
 use std::fs::File;
@@ -171,18 +170,10 @@ fn handle_module(
             context.loop_analysis = loop_analysis;
             verifier::verify_context(&context.func, &context.cfg, &context.domtree, None)
                 .map_err(|err| pretty_verifier_error(&context.func, None, err))?;
-            context.licm().map_err(|error| match error {
+            context.licm();
-                CtonError::Verifier(err) => pretty_verifier_error(&context.func, None, err),
+            verifier::verify_context(&context.func, &context.cfg, &context.domtree, None)
-                CtonError::InvalidInput |
+                .map_err(|err| pretty_verifier_error(&context.func, None, err))?;
-                CtonError::ImplLimitExceeded |
+            context.simple_gvn();
-                CtonError::CodeTooLarge => String::from(error.description()),
-            })?;
-            context.simple_gvn().map_err(|error| match error {
-                CtonError::Verifier(err) => pretty_verifier_error(&context.func, None, err),
-                CtonError::InvalidInput |
-                CtonError::ImplLimitExceeded |
-                CtonError::CodeTooLarge => String::from(error.description()),
-            })?;
             verifier::verify_context(&context.func, &context.cfg, &context.domtree, None)
                 .map_err(|err| pretty_verifier_error(&context.func, None, err))?;
         }

lib/cretonne/src/context.rs

@@ -116,22 +116,18 @@ impl Context {
     }
 
     /// Perform simple GVN on the function.
-    pub fn simple_gvn(&mut self) -> CtonResult {
+    pub fn simple_gvn(&mut self) {
-        do_simple_gvn(&mut self.func, &mut self.cfg);
+        do_simple_gvn(&mut self.func, &mut self.cfg, &mut self.domtree)
-        // TODO: Factor things such that we can get a Flags and test
-        // enable_verifier().
-        self.verify(None).map_err(Into::into)
     }
 
     /// Perform LICM on the function.
-    pub fn licm(&mut self) -> CtonResult {
+    pub fn licm(&mut self) {
         do_licm(
             &mut self.func,
             &mut self.cfg,
             &mut self.domtree,
             &mut self.loop_analysis,
-        );
+        )
-        self.verify(None).map_err(Into::into)
     }
 
     /// Run the register allocator.

lib/cretonne/src/dominator_tree.rs

@@ -231,8 +231,7 @@ impl DominatorTree {
     ///
     /// Note that this doesn't perform any kind of validity checks. It simply checks if the
     /// `compute()` method has been called since the last `clear()`. It does not check that the
-    /// dominator tree is consistent
+    /// dominator tree is consistent with the CFG.
-    /// with the CFG>
     pub fn is_valid(&self) -> bool {
         !self.nodes.is_empty()
     }

lib/cretonne/src/flowgraph.rs

@@ -131,6 +131,23 @@ impl ControlFlowGraph {
     pub fn get_successors(&self, ebb: Ebb) -> &[Ebb] {
         &self.data[ebb].successors
     }
+
+    /// Check if the CFG is in a valid state.
+    ///
+    /// Note that this doesn't perform any kind of validity checks. It simply checks if the
+    /// `compute()` method has been called since the last `clear()`. It does not check that the
+    /// CFG is consistent with the function.
+    pub fn is_valid(&self) -> bool {
+        self.entry_block.is_some()
+    }
+
+    /// Conveneince function to call `compute` if `compute` hasn't been called
+    /// since the last `clear()`.
+    pub fn ensure(&mut self, func: &Function) {
+        if !self.is_valid() {
+            self.compute(func)
+        }
+    }
 }
 
 #[cfg(test)]

lib/cretonne/src/licm.rs

@@ -16,6 +16,7 @@ pub fn do_licm(
     domtree: &mut DominatorTree,
     loop_analysis: &mut LoopAnalysis,
 ) {
+    cfg.ensure(func);
     domtree.ensure(func, cfg);
     loop_analysis.compute(func, cfg, domtree);
     for lp in loop_analysis.loops() {

lib/cretonne/src/simple_gvn.rs

@@ -13,10 +13,11 @@ fn trivially_unsafe_for_gvn(opcode: Opcode) -> bool {
 
 /// Perform simple GVN on `func`.
 ///
-pub fn do_simple_gvn(func: &mut Function, cfg: &mut ControlFlowGraph) {
+pub fn do_simple_gvn(func: &mut Function, cfg: &mut ControlFlowGraph, domtree: &mut DominatorTree) {
-    let mut visible_values: HashMap<(InstructionData, Type), Inst> = HashMap::new();
+    cfg.ensure(func);
+    domtree.ensure(func, cfg);
 
-    let domtree = DominatorTree::with_function(func, cfg);
+    let mut visible_values: HashMap<(InstructionData, Type), Inst> = HashMap::new();
 
     // Visit EBBs in a reverse post-order.
     let mut pos = Cursor::new(&mut func.layout);