util: add and use checked_vcd_output

crates/cpu/src/util.rs

@@ -1,8 +1,15 @@
 // SPDX-License-Identifier: LGPL-3.0-or-later
 // See Notices.txt for copyright information
 
-use fayalite::{expr::ops::ArrayLiteral, module::wire_with_loc, prelude::*};
+use fayalite::{
-use std::num::NonZero;
+    bundle::BundleType, expr::ops::ArrayLiteral, module::wire_with_loc, prelude::*,
+    sim::vcd::VcdWriterDecls, util::RcWriter,
+};
+use std::{
+    num::NonZero,
+    panic::Location,
+    path::{Path, PathBuf},
+};
 
 pub mod array_vec;
 pub mod tree_reduce;
@@ -310,3 +317,152 @@ impl LFSR31 {
         state
     }
 }
+
+pub struct CheckedVcdOutput {
+    writer: Option<RcWriter>,
+    expected_path: PathBuf,
+    expected_contents: Result<String, (Option<PathBuf>, std::io::Error)>,
+    location: &'static Location<'static>,
+}
+
+impl CheckedVcdOutput {
+    #[must_use]
+    #[track_caller]
+    pub fn new<T: BundleType>(sim: &mut Simulation<T>, expected_path: PathBuf) -> Self {
+        let writer = RcWriter::default();
+        sim.add_trace_writer(VcdWriterDecls::new(writer.clone()));
+        Self {
+            writer: Some(writer),
+            expected_contents: std::fs::read_to_string(&expected_path).map_err(|e| {
+                eprintln!(
+                    "error: failed to read expected VCD from: {}",
+                    expected_path.display(),
+                );
+                (std::env::current_dir().ok(), e)
+            }),
+            expected_path,
+            location: Location::caller(),
+        }
+    }
+    #[must_use]
+    #[track_caller]
+    #[doc(hidden)]
+    pub fn __checked_vcd_output_macro_helper<T: BundleType>(
+        sim: &mut Simulation<T>,
+        cargo_manifest_dir: &'static str,
+        path: &'static str,
+    ) -> Self {
+        Self::new(sim, Path::new(cargo_manifest_dir).join(path))
+    }
+    pub fn with_vcd_output<R>(&self, f: impl FnOnce(&str) -> R) -> R {
+        let Some(writer) = &self.writer else {
+            unreachable!();
+        };
+        writer.clone().borrow(|output| {
+            let Ok(output) = str::from_utf8(output) else {
+                unreachable!("VcdWriter writes valid UTF-8");
+            };
+            f(output)
+        })
+    }
+    #[track_caller]
+    pub fn finish(mut self) {
+        let Ok(()) = self.finish_impl(|msg| panic!("{msg}"));
+    }
+    fn finish_impl<E>(
+        &mut self,
+        error: impl FnOnce(std::fmt::Arguments<'_>) -> E,
+    ) -> Result<(), E> {
+        let Self {
+            writer: Some(writer),
+            expected_path,
+            expected_contents,
+            location,
+        } = self
+        else {
+            // already finished
+            return Ok(());
+        };
+        let Ok(vcd) = String::from_utf8(writer.take()) else {
+            unreachable!("VcdWriter writes valid UTF-8");
+        };
+        let expected_path_d = expected_path.display();
+        if expected_contents
+            .as_ref()
+            .is_ok_and(|expected_contents| *expected_contents == vcd)
+        {
+            // avoid written output from being split from threads interleaving writes to stdout
+            let _stdout = std::io::stderr().lock();
+            // use println to get output captured by tests
+            println!("\n{location}: generated VCD matches the expected VCD in {expected_path_d}");
+            return Ok(());
+        }
+        // avoid written output from being split from threads interleaving writes to stderr
+        let _stderr = std::io::stderr().lock();
+        let error = |msg: std::fmt::Arguments<'_>| {
+            // print msg at both beginning and end so it's easier to find when the vcd is huge
+            Err(error(format_args!(
+                "\n{msg}####### VCD:\n{vcd}\n#######\n{msg}"
+            )))
+        };
+        let error = |msg: std::fmt::Arguments<'_>| match &*expected_contents {
+            Ok(_) => error(format_args!(
+                "{location}: generated VCD doesn't match the expected VCD in {expected_path_d}\n\
+                {msg}",
+            )),
+            Err((Some(current_dir), e)) => error(format_args!(
+                "{location}: generated VCD doesn't match the expected VCD in {expected_path_d}\n\
+                error: failed to read: {e}\n\
+                current dir: {current_dir}\n\
+                {msg}",
+                current_dir = current_dir.display(),
+            )),
+            Err((None, e)) => error(format_args!(
+                "{location}: generated VCD doesn't match the expected VCD in {expected_path_d}\n\
+                error: failed to read: {e}\n\
+                {msg}",
+            )),
+        };
+        const OVERWRITE_VAR_NAME: &str = "OVERWRITE_EXPECTED_VCD";
+        const OVERWRITE_VAR_VALUE: &str = "overwrite";
+        match std::env::var_os(OVERWRITE_VAR_NAME) {
+            Some(v) if v == OVERWRITE_VAR_VALUE => match std::fs::write(&expected_path, &vcd) {
+                Ok(()) => error(format_args!(
+                    "warning: since `{OVERWRITE_VAR_NAME}={OVERWRITE_VAR_VALUE}` is set -- writing the generated VCD to {expected_path_d}\n"
+                )),
+                Err(e) => error(format_args!(
+                    "error: since `{OVERWRITE_VAR_NAME}={OVERWRITE_VAR_VALUE}` is set -- tried to write the generated VCD to {expected_path_d}\n\
+                    error: failed to write: {e}"
+                )),
+            },
+            _ => error(format_args!(
+                "note: rerun the test with the environment variable `{OVERWRITE_VAR_NAME}={OVERWRITE_VAR_VALUE}`\n\
+                to update the expected output to match the generated output.\n"
+            )),
+        }
+    }
+}
+
+impl Drop for CheckedVcdOutput {
+    #[track_caller]
+    fn drop(&mut self) {
+        let _ = self.finish_impl(|msg| {
+            if std::thread::panicking() {
+                eprintln!("{msg}"); // use eprintln to get output captured by tests
+            } else {
+                panic!("{msg}");
+            }
+        });
+    }
+}
+
+#[macro_export]
+macro_rules! checked_vcd_output {
+    ($sim:expr, $path_relative_to_manifest_dir:expr $(,)?) => {
+        $crate::util::CheckedVcdOutput::__checked_vcd_output_macro_helper(
+            $sim,
+            ::std::env!("CARGO_MANIFEST_DIR"),
+            ::std::concat!($path_relative_to_manifest_dir),
+        )
+    };
+}

crates/cpu/tests/rename_execute_retire.rs

@@ -2,6 +2,7 @@
 // See Notices.txt for copyright information
 
 use cpu::{
+    checked_vcd_output,
     config::{
         CpuConfig, CpuConfigFetchWidth, CpuConfigMaxUnitMaxInFlight, PhantomConstCpuConfig,
         UnitConfig,
@@ -30,9 +31,7 @@ use fayalite::{
     bundle::BundleType,
     module::instance_with_loc,
     prelude::*,
-    sim::vcd::VcdWriterDecls,
     ty::{OpaqueSimValue, StaticType},
-    util::RcWriter,
 };
 use serde::{Deserialize, Serialize};
 use std::{
@@ -4127,22 +4126,10 @@ fn test_rename_execute_retire_fibonacci_non_combinatorial() {
         false,
     );
     let mut sim = Simulation::new(m);
-    let writer = RcWriter::default();
+    let _checked_vcd_output = checked_vcd_output!(
-    sim.add_trace_writer(VcdWriterDecls::new(writer.clone()));
+        &mut sim,
-    struct DumpVcdOnDrop {
+        "tests/expected/rename_execute_retire_fibonacci_non_combinatorial.vcd",
-        writer: Option<RcWriter>,
+    );
-    }
-    impl Drop for DumpVcdOnDrop {
-        fn drop(&mut self) {
-            if let Some(mut writer) = self.writer.take() {
-                let vcd = String::from_utf8(writer.take()).unwrap();
-                println!("####### VCD:\n{vcd}\n#######");
-            }
-        }
-    }
-    let mut writer = DumpVcdOnDrop {
-        writer: Some(writer),
-    };
     sim.write_clock(sim.io().cd.clk, false);
     sim.write_reset(sim.io().cd.rst, true);
     for cycle in 0..200 {
@@ -4154,12 +4141,6 @@ fn test_rename_execute_retire_fibonacci_non_combinatorial() {
         sim.write_reset(sim.io().cd.rst, false);
     }
     assert!(sim.read_bool(sim.io().all_outputs_written));
-    // FIXME: vcd is just whatever rename_execute_retire does now, which isn't known to be correct
-    let vcd = String::from_utf8(writer.writer.take().unwrap().take()).unwrap();
-    println!("####### VCD:\n{vcd}\n#######");
-    if vcd != include_str!("expected/rename_execute_retire_fibonacci_non_combinatorial.vcd") {
-        panic!();
-    }
 }
 
 #[hdl]
@@ -4180,22 +4161,10 @@ fn test_rename_execute_retire_fibonacci_combinatorial() {
     let m =
         rename_execute_retire_test_harness::<FibonacciInsns>(PhantomConst::new_sized(config), true);
     let mut sim = Simulation::new(m);
-    let writer = RcWriter::default();
+    let _checked_vcd_output = checked_vcd_output!(
-    sim.add_trace_writer(VcdWriterDecls::new(writer.clone()));
+        &mut sim,
-    struct DumpVcdOnDrop {
+        "tests/expected/rename_execute_retire_fibonacci_combinatorial.vcd",
-        writer: Option<RcWriter>,
+    );
-    }
-    impl Drop for DumpVcdOnDrop {
-        fn drop(&mut self) {
-            if let Some(mut writer) = self.writer.take() {
-                let vcd = String::from_utf8(writer.take()).unwrap();
-                println!("####### VCD:\n{vcd}\n#######");
-            }
-        }
-    }
-    let mut writer = DumpVcdOnDrop {
-        writer: Some(writer),
-    };
     sim.write_clock(sim.io().cd.clk, false);
     sim.write_reset(sim.io().cd.rst, true);
     for cycle in 0..200 {
@@ -4207,12 +4176,6 @@ fn test_rename_execute_retire_fibonacci_combinatorial() {
         sim.write_reset(sim.io().cd.rst, false);
     }
     assert!(sim.read_bool(sim.io().all_outputs_written));
-    // FIXME: vcd is just whatever rename_execute_retire does now, which isn't known to be correct
-    let vcd = String::from_utf8(writer.writer.take().unwrap().take()).unwrap();
-    println!("####### VCD:\n{vcd}\n#######");
-    if vcd != include_str!("expected/rename_execute_retire_fibonacci_combinatorial.vcd") {
-        panic!();
-    }
 }
 
 struct SlowLoopInsns;
@@ -4313,22 +4276,10 @@ fn test_rename_execute_retire_slow_loop() {
     let m =
         rename_execute_retire_test_harness::<SlowLoopInsns>(PhantomConst::new_sized(config), true);
     let mut sim = Simulation::new(m);
-    let writer = RcWriter::default();
+    let _checked_vcd_output = checked_vcd_output!(
-    sim.add_trace_writer(VcdWriterDecls::new(writer.clone()));
+        &mut sim,
-    struct DumpVcdOnDrop {
+        "tests/expected/rename_execute_retire_slow_loop.vcd",
-        writer: Option<RcWriter>,
+    );
-    }
-    impl Drop for DumpVcdOnDrop {
-        fn drop(&mut self) {
-            if let Some(mut writer) = self.writer.take() {
-                let vcd = String::from_utf8(writer.take()).unwrap();
-                println!("####### VCD:\n{vcd}\n#######");
-            }
-        }
-    }
-    let mut writer = DumpVcdOnDrop {
-        writer: Some(writer),
-    };
     sim.write_clock(sim.io().cd.clk, false);
     sim.write_reset(sim.io().cd.rst, true);
     for cycle in 0..350 {
@@ -4342,12 +4293,6 @@ fn test_rename_execute_retire_slow_loop() {
     assert!(sim.read_bool(sim.io().all_outputs_written));
     // make sure we're actually testing L2 reg file ops
     assert!(sim.read_bool(sim.io().started_any_l2_reg_file_ops));
-    // FIXME: vcd is just whatever rename_execute_retire does now, which isn't known to be correct
-    let vcd = String::from_utf8(writer.writer.take().unwrap().take()).unwrap();
-    println!("####### VCD:\n{vcd}\n#######");
-    if vcd != include_str!("expected/rename_execute_retire_slow_loop.vcd") {
-        panic!();
-    }
 }
 
 /// equivalent of Unix's `head -n1`
@@ -4452,22 +4397,8 @@ fn test_rename_execute_retire_head_n1() {
     let m =
         rename_execute_retire_test_harness::<HeadN1Insns>(PhantomConst::new_sized(config), true);
     let mut sim = Simulation::new(m);
-    let writer = RcWriter::default();
+    let _checked_vcd_output =
-    sim.add_trace_writer(VcdWriterDecls::new(writer.clone()));
+        checked_vcd_output!(&mut sim, "tests/expected/rename_execute_retire_head_n1.vcd");
-    struct DumpVcdOnDrop {
-        writer: Option<RcWriter>,
-    }
-    impl Drop for DumpVcdOnDrop {
-        fn drop(&mut self) {
-            if let Some(mut writer) = self.writer.take() {
-                let vcd = String::from_utf8(writer.take()).unwrap();
-                println!("####### VCD:\n{vcd}\n#######");
-            }
-        }
-    }
-    let mut writer = DumpVcdOnDrop {
-        writer: Some(writer),
-    };
     sim.write_clock(sim.io().cd.clk, false);
     sim.write_reset(sim.io().cd.rst, true);
     for cycle in 0..300 {
@@ -4479,12 +4410,6 @@ fn test_rename_execute_retire_head_n1() {
         sim.write_reset(sim.io().cd.rst, false);
     }
     assert!(sim.read_bool(sim.io().all_outputs_written));
-    // FIXME: vcd is just whatever rename_execute_retire does now, which isn't known to be correct
-    let vcd = String::from_utf8(writer.writer.take().unwrap().take()).unwrap();
-    println!("####### VCD:\n{vcd}\n#######");
-    if vcd != include_str!("expected/rename_execute_retire_head_n1.vcd") {
-        panic!();
-    }
 }
 
 struct SaveRestoreGprsInsns;
@@ -4569,22 +4494,10 @@ fn test_rename_execute_retire_save_restore_gprs() {
         true,
     );
     let mut sim = Simulation::new(m);
-    let writer = RcWriter::default();
+    let _checked_vcd_output = checked_vcd_output!(
-    sim.add_trace_writer(VcdWriterDecls::new(writer.clone()));
+        &mut sim,
-    struct DumpVcdOnDrop {
+        "tests/expected/rename_execute_retire_save_restore_gprs.vcd",
-        writer: Option<RcWriter>,
+    );
-    }
-    impl Drop for DumpVcdOnDrop {
-        fn drop(&mut self) {
-            if let Some(mut writer) = self.writer.take() {
-                let vcd = String::from_utf8(writer.take()).unwrap();
-                println!("####### VCD:\n{vcd}\n#######");
-            }
-        }
-    }
-    let mut writer = DumpVcdOnDrop {
-        writer: Some(writer),
-    };
     sim.write_clock(sim.io().cd.clk, false);
     sim.write_reset(sim.io().cd.rst, true);
     for cycle in 0..700 {
@@ -4596,10 +4509,4 @@ fn test_rename_execute_retire_save_restore_gprs() {
         sim.write_reset(sim.io().cd.rst, false);
     }
     assert!(sim.read_bool(sim.io().all_outputs_written));
-    // FIXME: vcd is just whatever rename_execute_retire does now, which isn't known to be correct
-    let vcd = String::from_utf8(writer.writer.take().unwrap().take()).unwrap();
-    println!("####### VCD:\n{vcd}\n#######");
-    if vcd != include_str!("expected/rename_execute_retire_save_restore_gprs.vcd") {
-        panic!();
-    }
 }