Add comment support (#379)

* Add comment support

* Don't print empty comments

* Add nop instruction

* Add test and note

* Add FuncWriter trait

* Remove comment support

* Add write_preamble to FuncWriter

* Fix test

* Some changes

cranelift/filetests/isa/x86/nop.cton

@@ -0,0 +1,10 @@
+test compile
+
+target x86_64
+
+function %test(i32) -> i32 system_v {
+ebb0(v0: i32):
+    nop
+    v1 = iconst.i32 42
+    return v1
+}

cranelift/src/wasm.rs

@@ -149,7 +149,7 @@ fn handle_module(
         if flag_check_translation {
             context
                 .verify(fisa)
-                .map_err(|err| pretty_verifier_error(&context.func, fisa.isa, &err))?;
+                .map_err(|err| pretty_verifier_error(&context.func, fisa.isa, None, &err))?;
         } else {
             let compiled_size = context
                 .compile(isa)

lib/codegen/meta-python/base/instructions.py

@@ -581,6 +581,13 @@ bconst = Instruction(
 # Generics.
 #
 
+nop = Instruction(
+        'nop', r"""
+        Just a dummy instruction
+
+        Note: this doesn't compile to a machine code nop
+        """)
+
 c = Operand('c', Testable, doc='Controlling value to test')
 x = Operand('x', Any, doc='Value to use when `c` is true')
 y = Operand('y', Any, doc='Value to use when `c` is false')

lib/codegen/src/lib.rs

@@ -74,6 +74,7 @@ pub mod print_errors;
 pub mod settings;
 pub mod timing;
 pub mod verifier;
+pub mod write;
 
 pub use entity::packed_option;
 
@@ -100,7 +101,6 @@ mod simple_gvn;
 mod stack_layout;
 mod topo_order;
 mod unreachable_code;
-mod write;
 
 pub use result::{CodegenError, CodegenResult};
 

lib/codegen/src/print_errors.rs

@@ -3,18 +3,19 @@
 use ir;
 use ir::entities::Inst;
 use ir::function::Function;
-use isa::TargetIsa;
+use isa::{RegInfo, TargetIsa};
 use result::CodegenError;
 use std::fmt;
 use std::fmt::Write;
 use std::string::{String, ToString};
 use verifier::VerifierError;
-use write::decorate_function;
+use write::{decorate_function, FuncWriter, PlainWriter};
 
 /// Pretty-print a verifier error.
-pub fn pretty_verifier_error(
+pub fn pretty_verifier_error<'a>(
     func: &ir::Function,
     isa: Option<&TargetIsa>,
+    func_w: Option<Box<FuncWriter + 'a>>,
     err: &VerifierError,
 ) -> String {
     let mut w = String::new();
@@ -29,7 +30,7 @@ pub fn pretty_verifier_error(
     }
 
     decorate_function(
-        &mut |w, func, isa, inst, indent| pretty_function_error(w, func, isa, inst, indent, err),
+        &mut PrettyVerifierError(func_w.unwrap_or(Box::new(PlainWriter)), err),
         &mut w,
         func,
         isa,
@@ -37,6 +38,30 @@ pub fn pretty_verifier_error(
     w
 }
 
+struct PrettyVerifierError<'a>(Box<FuncWriter + 'a>, &'a VerifierError);
+
+impl<'a> FuncWriter for PrettyVerifierError<'a> {
+    fn write_instruction(
+        &mut self,
+        w: &mut Write,
+        func: &Function,
+        isa: Option<&TargetIsa>,
+        inst: Inst,
+        indent: usize,
+    ) -> fmt::Result {
+        pretty_function_error(w, func, isa, inst, indent, &mut *self.0, self.1)
+    }
+
+    fn write_preamble(
+        &mut self,
+        w: &mut Write,
+        func: &Function,
+        regs: Option<&RegInfo>,
+    ) -> Result<bool, fmt::Error> {
+        self.0.write_preamble(w, func, regs)
+    }
+}
+
 /// Pretty-print a function verifier error.
 fn pretty_function_error(
     w: &mut Write,
@@ -44,17 +69,12 @@ fn pretty_function_error(
     isa: Option<&TargetIsa>,
     cur_inst: Inst,
     indent: usize,
+    func_w: &mut FuncWriter,
     err: &VerifierError,
 ) -> fmt::Result {
     match err.location {
         ir::entities::AnyEntity::Inst(inst) if inst == cur_inst => {
-            writeln!(
+            func_w.write_instruction(w, func, isa, cur_inst, indent)?;
-                w,
-                "{1:0$}{2}",
-                indent,
-                "",
-                func.dfg.display_inst(cur_inst, isa)
-            )?;
             write!(w, "{1:0$}^", indent, "")?;
             for _c in cur_inst.to_string().chars() {
                 write!(w, "~")?;
@@ -74,7 +94,7 @@ fn pretty_function_error(
 /// Pretty-print a Cranelift error.
 pub fn pretty_error(func: &ir::Function, isa: Option<&TargetIsa>, err: CodegenError) -> String {
     if let CodegenError::Verifier(e) = err {
-        pretty_verifier_error(func, isa, &e)
+        pretty_verifier_error(func, isa, None, &e)
     } else {
         err.to_string()
     }

lib/codegen/src/write.rs

@@ -9,35 +9,63 @@ use packed_option::ReservedValue;
 use std::fmt::{self, Write};
 use std::string::String;
 
-fn write_function_plain(
+/// A `FuncWriter` is used to decorate functions during printing
+pub trait FuncWriter {
+    /// Write the given inst to w
+    fn write_instruction(
+        &mut self,
+        w: &mut Write,
+        func: &Function,
+        isa: Option<&TargetIsa>,
+        inst: Inst,
+        ident: usize,
+    ) -> fmt::Result;
+
+    /// Write the preamble to w
+    fn write_preamble(
+        &mut self,
+        w: &mut Write,
+        func: &Function,
+        regs: Option<&RegInfo>,
+    ) -> Result<bool, fmt::Error>;
+}
+
+/// A `PlainWriter` doesn't decorate the function
+pub struct PlainWriter;
+
+impl FuncWriter for PlainWriter {
+    fn write_instruction(
+        &mut self,
         w: &mut Write,
         func: &Function,
         isa: Option<&TargetIsa>,
         inst: Inst,
         indent: usize,
     ) -> fmt::Result {
-    write_instruction(w, func, isa, inst, indent)?;
+        write_instruction(w, func, isa, inst, indent)
-    Ok(())
+    }
+
+    fn write_preamble(
+        &mut self,
+        w: &mut Write,
+        func: &Function,
+        regs: Option<&RegInfo>,
+    ) -> Result<bool, fmt::Error> {
+        write_preamble(w, func, regs)
+    }
 }
 
 /// Write `func` to `w` as equivalent text.
 /// Use `isa` to emit ISA-dependent annotations.
 pub fn write_function(w: &mut Write, func: &Function, isa: Option<&TargetIsa>) -> fmt::Result {
-    decorate_function(
+    decorate_function(&mut PlainWriter, w, func, isa)
-        &mut |w, func, isa, inst, indent| write_function_plain(w, func, isa, inst, indent),
-        w,
-        func,
-        isa,
-    )
 }
 
 /// Writes 'func' to 'w' as text.
 /// write_function_plain is passed as 'closure' to print instructions as text.
 /// pretty_function_error is passed as 'closure' to add error decoration.
-pub fn decorate_function<
+pub fn decorate_function<FW: FuncWriter>(
-    WL: FnMut(&mut Write, &Function, Option<&TargetIsa>, Inst, usize) -> fmt::Result,
+    func_w: &mut FW,
->(
-    closure: &mut WL,
     w: &mut Write,
     func: &Function,
     isa: Option<&TargetIsa>,
@@ -48,12 +76,12 @@ pub fn decorate_function<
     write!(w, "function ")?;
     write_spec(w, func, regs)?;
     writeln!(w, " {{")?;
-    let mut any = write_preamble(w, func, regs)?;
+    let mut any = func_w.write_preamble(w, func, regs)?;
     for ebb in &func.layout {
         if any {
             writeln!(w)?;
         }
-        decorate_ebb(closure, w, func, isa, ebb)?;
+        decorate_ebb(func_w, w, func, isa, ebb)?;
         any = true;
     }
     writeln!(w, "}}")
@@ -120,7 +148,7 @@ fn write_preamble(
 //
 // Basic blocks
 
-pub fn write_arg(
+fn write_arg(
     w: &mut Write,
     func: &Function,
     regs: Option<&RegInfo>,
@@ -135,6 +163,12 @@ pub fn write_arg(
     Ok(())
 }
 
+/// Write out the basic block header, outdented:
+///
+///    ebb1:
+///    ebb1(v1: i32):
+///    ebb10(v4: f64, v5: b1):
+///
 pub fn write_ebb_header(
     w: &mut Write,
     func: &Function,
@@ -142,13 +176,6 @@ pub fn write_ebb_header(
     ebb: Ebb,
     indent: usize,
 ) -> fmt::Result {
-    // Write out the basic block header, outdented:
-    //
-    //    ebb1:
-    //    ebb1(v1: i32):
-    //    ebb10(v4: f64, v5: b1):
-    //
-
     // The `indent` is the instruction indentation. EBB headers are 4 spaces out from that.
     write!(w, "{1:0$}{2}", indent - 4, "", ebb)?;
 
@@ -171,10 +198,8 @@ pub fn write_ebb_header(
     writeln!(w, "):")
 }
 
-pub fn decorate_ebb<
+fn decorate_ebb<FW: FuncWriter>(
-    WL: FnMut(&mut Write, &Function, Option<&TargetIsa>, Inst, usize) -> fmt::Result,
+    func_w: &mut FW,
->(
-    closure: &mut WL,
     w: &mut Write,
     func: &Function,
     isa: Option<&TargetIsa>,
@@ -189,7 +214,7 @@ pub fn decorate_ebb<
 
     write_ebb_header(w, func, isa, ebb, indent)?;
     for inst in func.layout.ebb_insts(ebb) {
-        closure(w, func, isa, inst, indent)?;
+        func_w.write_instruction(w, func, isa, inst, indent)?;
     }
 
     Ok(())
@@ -556,8 +581,9 @@ impl<'a> fmt::Display for DisplayValuesWithDelimiter<'a> {
 
 #[cfg(test)]
 mod tests {
+    use cursor::{Cursor, CursorPosition, FuncCursor};
     use ir::types;
-    use ir::{ExternalName, Function, StackSlotData, StackSlotKind};
+    use ir::{ExternalName, Function, InstBuilder, StackSlotData, StackSlotKind};
     use std::string::ToString;
 
     #[test]
@@ -592,5 +618,15 @@ mod tests {
             f.to_string(),
             "function %foo() fast {\n    ss0 = explicit_slot 4\n\nebb0(v0: i8, v1: f32x4):\n}\n"
         );
+
+        {
+            let mut cursor = FuncCursor::new(&mut f);
+            cursor.set_position(CursorPosition::After(ebb));
+            cursor.ins().return_(&[])
+        };
+        assert_eq!(
+            f.to_string(),
+            "function %foo() fast {\n    ss0 = explicit_slot 4\n\nebb0(v0: i8, v1: f32x4):\n    return\n}\n"
+        );
     }
 }

lib/filetests/src/runone.rs

@@ -130,7 +130,7 @@ fn run_one_test<'a>(
     // Should we run the verifier before this test?
     if !context.verified && test.needs_verifier() {
         verify_function(&func, context.flags_or_isa())
-            .map_err(|e| pretty_verifier_error(&func, isa, &e))?;
+            .map_err(|e| pretty_verifier_error(&func, isa, None, &e))?;
         context.verified = true;
     }
 

lib/wasm/tests/wasm_testsuite.rs

@@ -77,7 +77,7 @@ fn handle_module(path: &Path, flags: &Flags) {
     translate_module(&data, &mut dummy_environ).unwrap();
     for func in &dummy_environ.info.function_bodies {
         verifier::verify_function(func, flags)
-            .map_err(|err| panic!(pretty_verifier_error(func, None, &err)))
+            .map_err(|err| panic!(pretty_verifier_error(func, None, None, &err)))
             .unwrap();
     }
 }