Merge #11

11: TIR test suite. r=ltratt a=vext01

This change adds a rustc flag `--emit yk-tir` which causes ykrustc to
dump the TIR in a textual format and exit.

Then we add a test new test suite kind which allows us to write tests
against the textual TIR dumps. It works in much the same way as the
existing MirOpt test suite.

Note that this needs a ykpack change to go in first. Raising a draft PR for initial discussion.

What do you think of the general approach?

[I think `generate_tir()` can be tidied up some in light of this change, but I didn't want to do that in this PR].

Co-authored-by: Edd Barrett <vext01@gmail.com>

Author: bors[bot]

Cargo.lock

@@ -4046,7 +4046,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index"
 [[package]]
 name = "ykpack"
 version = "0.1.0"
-source = "git+https://github.com/softdevteam/ykpack#358a7403a5b8c03809f5c5b07933a6850060afa4"
+source = "git+https://github.com/softdevteam/ykpack#9c5542d9d556ce745f0b21983a009a1297f371e4"
 dependencies = [
  "fallible-iterator 0.1.6 (registry+https://github.com/rust-lang/crates.io-index)",
  "rmp-serde 0.14.0 (git+https://github.com/3Hren/msgpack-rust?rev=40b3d480b20961e6eeceb416b32bcd0a3383846a)",

src/bootstrap/builder.rs

@@ -372,6 +372,7 @@ impl<'a> Builder<'a> {
                 test::RunFail,
                 test::RunPassValgrind,
                 test::MirOpt,
+                test::YkTir,
                 test::Codegen,
                 test::CodegenUnits,
                 test::Incremental,

src/bootstrap/test.rs

@@ -812,6 +812,12 @@ default_test!(MirOpt {
     suite: "mir-opt"
 });
 
+default_test!(YkTir {
+    path: "src/test/yk-tir",
+    mode: "yk-tir",
+    suite: "yk-tir"
+});
+
 default_test!(Codegen {
     path: "src/test/codegen",
     mode: "codegen",

src/librustc/session/config.rs

@@ -122,6 +122,7 @@ pub enum DebugInfo {
 
 #[derive(Clone, Copy, PartialEq, Eq, Hash, Debug, PartialOrd, Ord, RustcEncodable, RustcDecodable)]
 pub enum OutputType {
+    YkTir,
     Bitcode,
     Assembly,
     LlvmAssembly,
@@ -138,7 +139,8 @@ impl OutputType {
     fn is_compatible_with_codegen_units_and_single_output_file(&self) -> bool {
         match *self {
             OutputType::Exe | OutputType::DepInfo => true,
-            OutputType::Bitcode
+            OutputType::YkTir
+            | OutputType::Bitcode
             | OutputType::Assembly
             | OutputType::LlvmAssembly
             | OutputType::Mir
@@ -149,6 +151,7 @@ impl OutputType {
 
     fn shorthand(&self) -> &'static str {
         match *self {
+            OutputType::YkTir => "yk-tir",
             OutputType::Bitcode => "llvm-bc",
             OutputType::Assembly => "asm",
             OutputType::LlvmAssembly => "llvm-ir",
@@ -162,6 +165,7 @@ impl OutputType {
 
     fn from_shorthand(shorthand: &str) -> Option<Self> {
         Some(match shorthand {
+            "yk-tir" => OutputType::YkTir,
             "asm" => OutputType::Assembly,
             "llvm-ir" => OutputType::LlvmAssembly,
             "mir" => OutputType::Mir,
@@ -176,7 +180,8 @@ impl OutputType {
 
     fn shorthands_display() -> String {
         format!(
-            "`{}`, `{}`, `{}`, `{}`, `{}`, `{}`, `{}`, `{}`",
+            "`{}` `{}`, `{}`, `{}`, `{}`, `{}`, `{}`, `{}`, `{}`",
+            OutputType::YkTir.shorthand(),
             OutputType::Bitcode.shorthand(),
             OutputType::Assembly.shorthand(),
             OutputType::LlvmAssembly.shorthand(),
@@ -190,6 +195,7 @@ impl OutputType {
 
     pub fn extension(&self) -> &'static str {
         match *self {
+            OutputType::YkTir => "yktir",
             OutputType::Bitcode => "bc",
             OutputType::Assembly => "s",
             OutputType::LlvmAssembly => "ll",
@@ -253,7 +259,8 @@ impl OutputTypes {
     // True if any of the output types require codegen or linking.
     pub fn should_codegen(&self) -> bool {
         self.0.keys().any(|k| match *k {
-            OutputType::Bitcode
+            OutputType::YkTir
+            | OutputType::Bitcode
             | OutputType::Assembly
             | OutputType::LlvmAssembly
             | OutputType::Mir
@@ -1696,7 +1703,7 @@ pub fn rustc_short_optgroups() -> Vec<RustcOptGroup> {
             "emit",
             "Comma separated list of types of output for \
              the compiler to emit",
-            "[asm|llvm-bc|llvm-ir|obj|metadata|link|dep-info|mir]",
+            "[asm|llvm-bc|llvm-ir|obj|metadata|link|dep-info|mir|yk-tir]",
         ),
         opt::multi_s(
             "",

src/librustc_codegen_ssa/back/write.rs

@@ -425,6 +425,7 @@ pub fn start_async_codegen<B: ExtraBackendMethods>(
 
     for output_type in sess.opts.output_types.keys() {
         match *output_type {
+            OutputType::YkTir => {},
             OutputType::Bitcode => { modules_config.emit_bc = true; }
             OutputType::LlvmAssembly => { modules_config.emit_ir = true; }
             OutputType::Assembly => {
@@ -598,6 +599,7 @@ fn produce_final_output_artifacts(sess: &Session,
                 user_wants_objects = true;
                 copy_if_one_unit(OutputType::Object, true);
             }
+            OutputType::YkTir |
             OutputType::Mir |
             OutputType::Metadata |
             OutputType::Exe |

src/librustc_interface/passes.rs

@@ -46,7 +46,7 @@ use syntax::symbol::Symbol;
 use syntax::feature_gate::AttributeType;
 use syntax_pos::{FileName, hygiene};
 use syntax_ext;
-use rustc_yk_sections::mir_cfg::generate_tir;
+use rustc_yk_sections::emit_tir::{generate_tir, TirMode};
 use rustc_codegen_utils::link::out_filename;
 use rustc::util::nodemap::DefIdSet;
 
@@ -1040,12 +1040,20 @@ pub fn start_codegen<'tcx>(
 
     // Output Yorick debug sections into binary targets.
     if tcx.sess.crate_types.borrow().contains(&config::CrateType::Executable) {
-        let out_fname = out_filename(
-            tcx.sess, config::CrateType::Executable, &outputs,
-            &*tcx.crate_name(LOCAL_CRATE).as_str());
+        let tir_mode = if tcx.sess.opts.output_types.contains_key(&OutputType::YkTir) {
+            // The user passed "--emit yk-tir" so we will output textual TIR and stop.
+            TirMode::TextDump(outputs.path(OutputType::YkTir))
+        } else {
+            // TIR will be encoded into the compiled binary.
+            let out_fname = out_filename(
+                tcx.sess, config::CrateType::Executable, &outputs,
+                &*tcx.crate_name(LOCAL_CRATE).as_str());
+            TirMode::Default(out_fname)
+        };
 
-        match generate_tir(&tcx, &def_ids, out_fname) {
-            Ok(tir) => tcx.sess.yk_link_objects.borrow_mut().push(tir),
+        match generate_tir(&tcx, &def_ids, tir_mode) {
+            Ok(Some(obj)) => tcx.sess.yk_link_objects.borrow_mut().push(obj),
+            Ok(None) => (),
             Err(e) => {
                 tcx.sess.err(&format!("could not emit TIR: {}", e));
                 tcx.sess.abort_if_errors();

src/librustc_yk_sections/mir_cfg.rs renamed to src/librustc_yk_sections/emit_tir.rs

@@ -25,6 +25,7 @@ use std::path::PathBuf;
 use std::fs::File;
 use rustc_yk_link::YkExtraLinkObject;
 use std::fs;
+use std::io::Write;
 use std::error::Error;
 use std::cell::{Cell, RefCell};
 use std::mem::size_of;
@@ -37,6 +38,15 @@ use ykpack::LocalIndex as TirLocal;
 const SECTION_NAME: &'static str = ".yk_tir";
 const TMP_EXT: &'static str = ".yk_tir.tmp";
 
+/// Describes how to output MIR.
+pub enum TirMode {
+    /// Write MIR into an object file for linkage. The inner path should be the path to the main
+    /// executable (from this we generate a filename for the resulting object).
+    Default(PathBuf),
+    /// Write MIR in textual form the specified path.
+    TextDump(PathBuf),
+}
+
 /// A conversion context holds the state needed to perform the conversion to (pre-SSA) TIR.
 struct ConvCx<'a, 'tcx, 'gcx> {
     /// The compiler's god struct. Needed for queries etc.
@@ -111,16 +121,55 @@ impl<'a, 'tcx, 'gcx> ConvCx<'a, 'tcx, 'gcx> {
     }
 }
 
-/// Converts and serialises the specified DefIds, returning an linkable ELF object.
+/// Writes TIR to file for the specified DefIds, possibly returning a linkable ELF object.
 pub fn generate_tir<'a, 'tcx, 'gcx>(
-    tcx: &'a TyCtxt<'a, 'tcx, 'gcx>, def_ids: &DefIdSet, exe_filename: PathBuf)
-    -> Result<YkExtraLinkObject, Box<dyn Error>> {
+    tcx: &'a TyCtxt<'a, 'tcx, 'gcx>, def_ids: &DefIdSet, mode: TirMode)
+    -> Result<Option<YkExtraLinkObject>, Box<dyn Error>>
+{
+    let tir_path = do_generate_tir(tcx, def_ids, &mode)?;
+    match mode {
+        TirMode::Default(_) => {
+            // In this case the file at `tir_path` is a raw binary file which we use to make an
+            // object file for linkage.
+            let obj = YkExtraLinkObject::new(&tir_path, SECTION_NAME);
+            // Now we have our object, we can remove the temp file. It's not the end of the world
+            // if we can't remove it, so we allow this to fail.
+            fs::remove_file(tir_path).ok();
+            Ok(Some(obj))
+        },
+        TirMode::TextDump(_) => {
+            // In this case we have no object to link, and we keep the file at `tir_path` around,
+            // as this is the text dump the user asked for.
+            Ok(None)
+        }
+    }
+}
 
-    // The filename must be the same between builds for the reproducible build tests to pass.
-    let mut mir_path: String = exe_filename.to_str().unwrap().to_owned();
-    mir_path.push_str(TMP_EXT);
-    let mut fh = File::create(&mir_path)?;
-    let mut enc = ykpack::Encoder::from(&mut fh);
+fn do_generate_tir<'a, 'tcx, 'gcx>(
+    tcx: &'a TyCtxt<'a, 'tcx, 'gcx>, def_ids: &DefIdSet, mode: &TirMode)
+    -> Result<PathBuf, Box<dyn Error>>
+{
+    let (tir_path, mut default_file, textdump_file) = match mode {
+        TirMode::Default(exe_path) => {
+            // The default mode of operation dumps TIR in binary format to a temporary file, which
+            // is later converted into an ELF object. Note that the temporary file name must be the
+            // same between builds for the reproducible build tests to pass.
+            let mut tir_path = exe_path.clone();
+            tir_path.set_extension(TMP_EXT);
+            let mut file = File::create(&tir_path)?;
+            (tir_path, Some(file), None)
+        },
+        TirMode::TextDump(dump_path) => {
+            // In text dump mode we just write lines to a file and we don't need an encoder.
+            let mut file = File::create(&dump_path)?;
+            (dump_path.clone(), None, Some(file))
+        },
+    };
+
+    let mut enc = match default_file {
+        Some(ref mut f) => Some(ykpack::Encoder::from(f)),
+        _ => None,
+    };
 
     // To satisfy the reproducible build tests, the CFG must be written out in a deterministic
     // order, thus we sort the `DefId`s first.
@@ -141,17 +190,22 @@ pub fn generate_tir<'a, 'tcx, 'gcx>(
             let phied_pack = PhiInserter::new(mir, pre_ssa_pack, ccx.def_sites()).pack();
 
             // FIXME - rename variables with fresh SSA names.
-            enc.serialise(phied_pack)?;
+
+            if let Some(ref mut e) = enc {
+                e.serialise(phied_pack)?;
+            } else {
+                write!(textdump_file.as_ref().unwrap(), "{}", phied_pack)?;
+            }
         }
     }
-    enc.done()?;
 
-    // Now graft the resulting blob file into an object file.
-    let path = PathBuf::from(mir_path);
-    let ret = YkExtraLinkObject::new(&path, SECTION_NAME);
-    fs::remove_file(path)?;
+    if let Some(e) = enc {
+        // Now finalise the encoder and convert the resulting blob file into an object file for
+        // linkage into the main binary. Once we've converted, we no longer need the original file.
+        e.done()?;
+    }
 
-    Ok(ret)
+    Ok(tir_path)
 }
 
 /// Lazy computation of dominance frontiers.
@@ -327,7 +381,7 @@ impl<'tcx> ToPack<ykpack::Pack> for (&ConvCx<'_, 'tcx, '_>, &DefId, &Mir<'tcx>)
         let ser_def_id = ykpack::DefId::new(
             ccx.tcx.crate_hash(def_id.krate).as_u64(), def_id.index.as_raw_u32());
 
-        ykpack::Pack::Mir(ykpack::Mir::new(ser_def_id, ser_blks))
+        ykpack::Pack::Mir(ykpack::Mir::new(ser_def_id, ccx.tcx.item_path_str(**def_id), ser_blks))
     }
 }
 

src/librustc_yk_sections/lib.rs

@@ -16,4 +16,4 @@ extern crate ykpack;
 extern crate rustc_data_structures;
 #[macro_use] extern crate log;
 
-pub mod mir_cfg;
+pub mod emit_tir;

src/test/yk-tir/simple_phi.rs

@@ -0,0 +1,27 @@
+use std::env;
+
+#[no_trace]
+fn main() {
+    let x = env::args().count();
+    let mut res = 42;
+
+    if x > 4 {
+        res = 100;
+    }
+
+    println!("res: {}", res);
+}
+
+// END RUST SOURCE
+// [Begin TIR for main]
+// ...
+// bb2:
+// ...
+//     term: SwitchInt { target_bbs: [4, 3] }
+// bb3:
+//     Assign(Local(0), Unimplemented)
+//     term: Goto { target_bb: 4 }
+// bb4:
+//     Assign(Local(0), Phi([Local(0), Local(0)]))
+// ...
+// [End TIR for main]

src/tools/compiletest/src/common.rs

@@ -25,6 +25,7 @@ pub enum Mode {
     RunMake,
     Ui,
     MirOpt,
+    YkTir,
 }
 
 impl Mode {
@@ -60,6 +61,7 @@ impl FromStr for Mode {
             "run-make" => Ok(RunMake),
             "ui" => Ok(Ui),
             "mir-opt" => Ok(MirOpt),
+            "yk-tir" => Ok(YkTir),
             _ => Err(()),
         }
     }
@@ -83,6 +85,7 @@ impl fmt::Display for Mode {
             RunMake => "run-make",
             Ui => "ui",
             MirOpt => "mir-opt",
+            YkTir => "yk-tir",
         };
         fmt::Display::fmt(s, f)
     }