Verify that FunctionBuilder blocks are basic blocks in debug mode (#857)

To use, enable the "basic-blocks" feature on cranelift-frontend.

cranelift/codegen/src/ir/function.rs

@@ -19,6 +19,9 @@ use crate::value_label::ValueLabelsRanges;
 use crate::write::write_function;
 use core::fmt;
 
+#[cfg(feature = "basic-blocks")]
+use crate::ir::{Inst, Opcode};
+
 /// A function.
 ///
 /// Functions can be cloned, but it is not a very fast operation.
@@ -219,6 +222,31 @@ impl Function {
     pub fn collect_debug_info(&mut self) {
         self.dfg.collect_debug_info();
     }
+
+    /// Checks that the specified EBB can be encoded as a basic block.
+    ///
+    /// On error, returns the first invalid instruction and an error message.
+    #[cfg(feature = "basic-blocks")]
+    pub fn is_ebb_basic(&self, ebb: Ebb) -> Result<(), (Inst, &'static str)> {
+        let dfg = &self.dfg;
+        let inst_iter = self.layout.ebb_insts(ebb);
+
+        // Ignore all instructions prior to the first branch.
+        let mut inst_iter = inst_iter.skip_while(|&inst| !dfg[inst].opcode().is_branch());
+
+        // A conditional branch is permitted in a basic block only when followed
+        // by a terminal jump or fallthrough instruction.
+        if let Some(_branch) = inst_iter.next() {
+            if let Some(next) = inst_iter.next() {
+                match dfg[next].opcode() {
+                    Opcode::Fallthrough | Opcode::Jump => (),
+                    _ => return Err((next, "post-branch instruction not fallthrough or jump")),
+                }
+            }
+        }
+
+        Ok(())
+    }
 }
 
 /// Additional annotations for function display.

cranelift/codegen/src/verifier/mod.rs

@@ -472,42 +472,9 @@ impl<'a> Verifier<'a> {
     /// branching instructions are ending the EBB.
     #[cfg(feature = "basic-blocks")]
     fn encodable_as_bb(&self, ebb: Ebb, errors: &mut VerifierErrors) -> VerifierStepResult<()> {
-        let dfg = &self.func.dfg;
+        match self.func.is_ebb_basic(ebb) {
-        let inst_iter = self.func.layout.ebb_insts(ebb);
+            Ok(()) => Ok(()),
-        // Skip non-branching instructions.
+            Err((inst, message)) => fatal!(errors, inst, message),
-        let mut inst_iter = inst_iter.skip_while(|&inst| !dfg[inst].opcode().is_branch());
-
-        let branch = match inst_iter.next() {
-            // There is no branch in the current EBB.
-            None => return Ok(()),
-            Some(br) => br,
-        };
-
-        let after_branch = match inst_iter.next() {
-            // The branch is also the terminator.
-            None => return Ok(()),
-            Some(inst) => inst,
-        };
-
-        let after_branch_opcode = dfg[after_branch].opcode();
-        if !after_branch_opcode.is_terminator() {
-            return fatal!(
-                errors,
-                branch,
-                "branch followed by a non-terminator instruction."
-            );
-        };
-
-        // Allow only one conditional branch and a fallthrough implemented with
-        // a jump or fallthrough instruction. Any other, which returns or check
-        // a different condition would have to be moved to a different EBB.
-        match after_branch_opcode {
-            Opcode::Fallthrough | Opcode::Jump => Ok(()),
-            _ => fatal!(
-                errors,
-                after_branch,
-                "terminator instruction not fallthrough or jump"
-            ),
         }
     }
 

cranelift/frontend/Cargo.toml

@@ -21,6 +21,9 @@ default = ["std"]
 std = ["cranelift-codegen/std"]
 core = ["hashmap_core", "cranelift-codegen/core"]
 
+# Temporary feature that enforces basic block semantics.
+basic-blocks = ["cranelift-codegen/basic-blocks"]
+
 [badges]
 maintenance = { status = "experimental" }
 travis-ci = { repository = "CraneStation/cranelift" }

cranelift/frontend/src/frontend.rs

@@ -470,6 +470,16 @@ impl<'a> FunctionBuilder<'a> {
             "all blocks should be filled before dropping a FunctionBuilder"
         );
 
+        // Check that all blocks are valid basic blocks.
+        #[cfg(feature = "basic-blocks")]
+        debug_assert!(
+            self.func_ctx
+                .ebbs
+                .keys()
+                .all(|ebb| self.func.is_ebb_basic(ebb).is_ok()),
+            "all blocks should be encodable as basic blocks"
+        );
+
         // Clear the state (but preserve the allocated buffers) in preparation
         // for translation another function.
         self.func_ctx.clear();
@@ -840,6 +850,7 @@ impl<'a> FunctionBuilder<'a> {
         );
     }
 
+    /// An Ebb is 'filled' when a terminator instruction is present.
     fn fill_current_block(&mut self) {
         self.func_ctx.ebbs[self.position.ebb.unwrap()].filled = true;
     }