WIP: next_pc test fails

crates/cpu/src/next_pc.rs

@@ -85,7 +85,102 @@ impl WipDecodedInsnKind {
     }
 }
 
-#[hdl]
+// TODO: replace with #[hdl(cmp_eq)] when that's implemented for enums
+impl HdlPartialEqImpl<Self> for WipDecodedInsnKind {
+    #[track_caller]
+    fn cmp_value_eq(
+        lhs: Self,
+        lhs_value: Cow<'_, Self::SimValue>,
+        rhs: Self,
+        rhs_value: Cow<'_, Self::SimValue>,
+    ) -> bool {
+        *Self::cmp_sim_value_eq(
+            Cow::Owned(SimValue::from_value(lhs, lhs_value.into_owned())),
+            Cow::Owned(SimValue::from_value(rhs, rhs_value.into_owned())),
+        )
+    }
+
+    #[hdl]
+    #[track_caller]
+    fn cmp_sim_value_eq(
+        lhs: Cow<'_, SimValue<Self>>,
+        rhs: Cow<'_, SimValue<Self>>,
+    ) -> SimValue<Bool> {
+        let clear_unused_bits = |v: Cow<'_, SimValue<Self>>| {
+            #[hdl(sim)]
+            match &*v {
+                Self::NonBranch =>
+                {
+                    #[hdl(sim)]
+                    Self::NonBranch()
+                }
+                Self::Branch(target) =>
+                {
+                    #[hdl(sim)]
+                    Self::Branch(target)
+                }
+                Self::BranchCond(target) =>
+                {
+                    #[hdl(sim)]
+                    Self::BranchCond(target)
+                }
+                Self::IndirectBranch =>
+                {
+                    #[hdl(sim)]
+                    Self::IndirectBranch()
+                }
+                Self::Call(target) =>
+                {
+                    #[hdl(sim)]
+                    Self::Call(target)
+                }
+                Self::CallCond(target) =>
+                {
+                    #[hdl(sim)]
+                    Self::CallCond(target)
+                }
+                Self::IndirectCall =>
+                {
+                    #[hdl(sim)]
+                    Self::IndirectCall()
+                }
+                Self::Ret =>
+                {
+                    #[hdl(sim)]
+                    Self::Ret()
+                }
+                Self::RetCond =>
+                {
+                    #[hdl(sim)]
+                    Self::RetCond()
+                }
+                Self::Interrupt(target) =>
+                {
+                    #[hdl(sim)]
+                    Self::Interrupt(target)
+                }
+                Self::Unknown => v.into_owned(),
+            }
+        };
+        (SimValue::bits(&clear_unused_bits(lhs)) == SimValue::bits(&clear_unused_bits(rhs)))
+            .to_sim_value()
+    }
+
+    #[track_caller]
+    fn cmp_sim_value_ne(
+        lhs: Cow<'_, SimValue<Self>>,
+        rhs: Cow<'_, SimValue<Self>>,
+    ) -> SimValue<Bool> {
+        !Self::cmp_sim_value_eq(lhs, rhs)
+    }
+
+    #[track_caller]
+    fn cmp_expr_eq(lhs: Expr<Self>, rhs: Expr<Self>) -> Expr<Bool> {
+        todo!()
+    }
+}
+
+#[hdl(cmp_eq)]
 /// WIP version of decoded instruction just good enough to represent stuff needed for [`next_pc()`]
 /// since the actual instruction definition isn't finalized yet.
 /// This will be replaced at a later point.
@@ -3868,7 +3963,7 @@ pub fn next_pc(config: PhantomConst<CpuConfig>) {
         m.input(DecodeToPostDecodeInterface[config]);
     #[hdl]
     let post_decode_output: PostDecodeOutputInterface<PhantomConst<CpuConfig>> =
-        m.input(PostDecodeOutputInterface[config]);
+        m.output(PostDecodeOutputInterface[config]);
     #[hdl]
     let from_retire: RetireToNextPcInterface<PhantomConst<CpuConfig>> =
         m.input(RetireToNextPcInterface[config]);

crates/cpu/tests/next_pc.rs

@@ -4,9 +4,10 @@
 use cpu::{
     config::{CpuConfig, UnitConfig},
     next_pc::{
-        DecodeToPostDecodeInterface, DecodeToPostDecodeInterfaceInner, FETCH_BLOCK_ID_WIDTH,
-        NextPcToFetchInterface, NextPcToFetchInterfaceInner, WipDecodedInsn, WipDecodedInsnKind,
-        next_pc,
+        CallStackOp, DecodeToPostDecodeInterface, DecodeToPostDecodeInterfaceInner,
+        FETCH_BLOCK_ID_WIDTH, NextPcToFetchInterface, NextPcToFetchInterfaceInner,
+        PostDecodeOutputInterface, RetireToNextPcInterface, RetireToNextPcInterfaceInner,
+        RetireToNextPcInterfacePerInsn, WipDecodedInsn, WipDecodedInsnKind, next_pc,
     },
     unit::UnitKind,
     util::array_vec::ArrayVec,
@@ -14,8 +15,9 @@ use cpu::{
 use fayalite::{prelude::*, sim::vcd::VcdWriterDecls, util::RcWriter};
 use std::{
     cell::Cell,
-    collections::{BTreeMap, VecDeque},
+    collections::{BTreeMap, BTreeSet, VecDeque},
     num::NonZeroUsize,
+    u64,
 };
 
 #[derive(Copy, Clone, Debug)]
@@ -37,6 +39,123 @@ impl MockInsn {
             MockInsn::Ret => 4,
         }
     }
+    const INSNS: &'static [(u64, Self)] = &[
+        (0x0, MockInsn::Nop4),
+        (0x4, MockInsn::Nop4),
+        (0x8, MockInsn::CondBranch { target: 0x4 }),
+        (0xC, MockInsn::Call { target: 0x18 }),
+        (0x10, MockInsn::Jump { target: 0x14 }),
+        (0x14, MockInsn::Jump { target: 0x10 }),
+        (0x18, MockInsn::Jump { target: 0x1C }),
+        (0x1C, MockInsn::Ret),
+    ];
+    const RETIRE_SEQ_INIT: &'static [RetireSeqEntry] = &[
+        RetireSeqEntry {
+            pc: 0x0,
+            cond_br_taken: None,
+            kind: MockInsn::Nop4,
+        },
+        RetireSeqEntry {
+            pc: 0x4,
+            cond_br_taken: None,
+            kind: MockInsn::Nop4,
+        },
+        RetireSeqEntry {
+            pc: 0x8,
+            cond_br_taken: Some(true),
+            kind: MockInsn::CondBranch { target: 0x4 },
+        },
+        RetireSeqEntry {
+            pc: 0x4,
+            cond_br_taken: None,
+            kind: MockInsn::Nop4,
+        },
+        RetireSeqEntry {
+            pc: 0x8,
+            cond_br_taken: Some(true),
+            kind: MockInsn::CondBranch { target: 0x4 },
+        },
+        RetireSeqEntry {
+            pc: 0x4,
+            cond_br_taken: None,
+            kind: MockInsn::Nop4,
+        },
+        RetireSeqEntry {
+            pc: 0x8,
+            cond_br_taken: Some(true),
+            kind: MockInsn::CondBranch { target: 0x4 },
+        },
+        RetireSeqEntry {
+            pc: 0x4,
+            cond_br_taken: None,
+            kind: MockInsn::Nop4,
+        },
+        RetireSeqEntry {
+            pc: 0x8,
+            cond_br_taken: Some(false),
+            kind: MockInsn::CondBranch { target: 0x4 },
+        },
+        RetireSeqEntry {
+            pc: 0xC,
+            cond_br_taken: None,
+            kind: MockInsn::Call { target: 0x18 },
+        },
+        RetireSeqEntry {
+            pc: 0x18,
+            cond_br_taken: None,
+            kind: MockInsn::Jump { target: 0x1C },
+        },
+        RetireSeqEntry {
+            pc: 0x1C,
+            cond_br_taken: None,
+            kind: MockInsn::Ret,
+        },
+    ];
+    const RETIRE_SEQ_CYCLE: &'static [RetireSeqEntry] = &[
+        RetireSeqEntry {
+            pc: 0x10,
+            cond_br_taken: None,
+            kind: MockInsn::Jump { target: 0x14 },
+        },
+        RetireSeqEntry {
+            pc: 0x14,
+            cond_br_taken: None,
+            kind: MockInsn::Jump { target: 0x10 },
+        },
+    ];
+}
+
+#[derive(Copy, Clone, Debug)]
+struct RetireSeqEntry {
+    pc: u64,
+    cond_br_taken: Option<bool>,
+    kind: MockInsn,
+}
+
+#[derive(Clone)]
+struct RetireSeq {
+    iter: std::iter::Chain<
+        std::slice::Iter<'static, RetireSeqEntry>,
+        std::iter::Cycle<std::slice::Iter<'static, RetireSeqEntry>>,
+    >,
+}
+
+impl RetireSeq {
+    fn new() -> Self {
+        Self {
+            iter: MockInsn::RETIRE_SEQ_INIT
+                .iter()
+                .chain(MockInsn::RETIRE_SEQ_CYCLE.iter().cycle()),
+        }
+    }
+}
+
+impl Iterator for RetireSeq {
+    type Item = RetireSeqEntry;
+
+    fn next(&mut self) -> Option<Self::Item> {
+        self.iter.next().copied()
+    }
 }
 
 #[derive(Debug)]
@@ -47,16 +166,7 @@ struct MockInsns {
 impl MockInsns {
     fn new() -> Self {
         Self {
-            insns: BTreeMap::from_iter([
-                (0x0, MockInsn::Nop4),
-                (0x4, MockInsn::Nop4),
-                (0x8, MockInsn::CondBranch { target: 0x4 }),
-                (0xC, MockInsn::Call { target: 0x18 }),
-                (0x10, MockInsn::Jump { target: 0x10 }),
-                (0x14, MockInsn::Jump { target: 0x10 }),
-                (0x18, MockInsn::Jump { target: 0x1C }),
-                (0x1C, MockInsn::Ret),
-            ]),
+            insns: BTreeMap::from_iter(MockInsn::INSNS.iter().copied()),
         }
     }
     fn fetch_block(&self, pc_range: std::ops::Range<u64>) -> impl Iterator<Item = (u64, MockInsn)> {
@@ -300,18 +410,383 @@ fn mock_fetch_pipe(config: PhantomConst<CpuConfig>) {
     }
 }
 
+const EXECUTE_RETIRE_PIPE_QUEUE_SIZE: usize = 15;
+
+#[hdl]
+struct ExecuteRetirePipeQueueEntry {
+    insn: WipDecodedInsn,
+    cycles_left: UInt<8>,
+}
+
+impl ExecuteRetirePipeQueueEntry {
+    #[hdl]
+    fn default_sim(self) -> SimValue<Self> {
+        #[hdl(sim)]
+        Self {
+            insn: #[hdl(sim)]
+            WipDecodedInsn {
+                fetch_block_id: 0u8,
+                id: 0_hdl_u12,
+                pc: 0xEEEE_EEEE_EEEE_EEEEu64,
+                size_in_bytes: 0_hdl_u4,
+                kind: WipDecodedInsnKind.NonBranch(),
+            },
+            cycles_left: 0u8,
+        }
+    }
+    fn get_next_delay(delay_sequence_index: &Cell<u64>) -> u8 {
+        let index = delay_sequence_index.get();
+        delay_sequence_index.set(delay_sequence_index.get().wrapping_add(1));
+        // make a pseudo-random number deterministically based on index
+        let random = index
+            .wrapping_add(1)
+            .wrapping_mul(0x39FF446D8BFB75BB) // random prime
+            .rotate_left(32)
+            .wrapping_mul(0x73161B54984B1C21) // random prime
+            .rotate_right(60);
+        (random % 16) as u8
+    }
+}
+
+/// an arbitrary value
+const END_PC: u64 = u64::from_be_bytes(*b"EndInsns");
+
+#[derive(Clone)]
+struct MockExecuteState {
+    queue: VecDeque<SimValue<ExecuteRetirePipeQueueEntry>>,
+    used_ids: BTreeSet<SimValue<UInt<12>>>,
+    retire_seq: RetireSeq,
+    config: PhantomConst<CpuConfig>,
+}
+
+impl MockExecuteState {
+    fn new(config: PhantomConst<CpuConfig>) -> Self {
+        Self {
+            queue: VecDeque::new(),
+            used_ids: BTreeSet::new(),
+            retire_seq: RetireSeq::new(),
+            config,
+        }
+    }
+    fn on_clock_cycle(&mut self) {
+        for entry in &mut self.queue {
+            if entry.cycles_left.as_int() > 0 {
+                entry.cycles_left = (entry.cycles_left.as_int() - 1u8).to_sim_value();
+            }
+        }
+    }
+    #[hdl]
+    fn do_retire(
+        &mut self,
+        entry: SimValue<ExecuteRetirePipeQueueEntry>,
+    ) -> Result<SimValue<RetireToNextPcInterfacePerInsn<PhantomConst<CpuConfig>>>, String> {
+        #[hdl(sim)]
+        let ExecuteRetirePipeQueueEntry {
+            insn,
+            cycles_left: _,
+        } = entry;
+        self.used_ids.remove(&insn.id);
+        let RetireSeqEntry {
+            pc,
+            cond_br_taken,
+            kind,
+        } = self
+            .retire_seq
+            .next()
+            .ok_or_else(|| "expected no more instructions to retire")?;
+        let next_pc = self
+            .retire_seq
+            .clone()
+            .next()
+            .map(|v| v.pc)
+            .unwrap_or(END_PC);
+        let (expected_kind, call_stack_op) = match kind {
+            MockInsn::Nop4 => (
+                #[hdl(sim)]
+                WipDecodedInsnKind::NonBranch(),
+                #[hdl(sim)]
+                CallStackOp::None(),
+            ),
+            MockInsn::Jump { target } => (
+                #[hdl(sim)]
+                WipDecodedInsnKind::Branch(target),
+                #[hdl(sim)]
+                CallStackOp::None(),
+            ),
+            MockInsn::CondBranch { target } => (
+                #[hdl(sim)]
+                WipDecodedInsnKind::BranchCond(target),
+                #[hdl(sim)]
+                CallStackOp::None(),
+            ),
+            MockInsn::Call { target } => (
+                #[hdl(sim)]
+                WipDecodedInsnKind::Call(target),
+                #[hdl(sim)]
+                CallStackOp::Push(pc.wrapping_add(kind.byte_len())),
+            ),
+            MockInsn::Ret => (
+                #[hdl(sim)]
+                WipDecodedInsnKind::Ret(),
+                #[hdl(sim)]
+                CallStackOp::Pop(),
+            ),
+        };
+        let expected_insn = #[hdl(sim)]
+        WipDecodedInsn {
+            fetch_block_id: &insn.fetch_block_id,
+            id: &insn.id,
+            pc,
+            size_in_bytes: kind.byte_len().cast_to_static::<UInt<4>>(),
+            kind: expected_kind,
+        };
+        if *expected_insn.cmp_ne(&insn) {
+            return Err(format!(
+                "insn doesn't match expected:\ninsn: {insn:?}\nexpected insn: {expected_insn:?}"
+            ));
+        }
+        Ok(
+            #[hdl(sim)]
+            RetireToNextPcInterfacePerInsn::<_> {
+                id: &insn.id,
+                next_pc,
+                call_stack_op,
+                cond_br_taken: if let Some(cond_br_taken) = cond_br_taken {
+                    #[hdl(sim)]
+                    HdlSome(cond_br_taken)
+                } else {
+                    #[hdl(sim)]
+                    HdlNone()
+                },
+                config: self.config,
+            },
+        )
+    }
+    #[hdl]
+    fn try_retire(
+        &mut self,
+    ) -> Option<Result<SimValue<RetireToNextPcInterfacePerInsn<PhantomConst<CpuConfig>>>, String>>
+    {
+        if self.queue.front()?.cycles_left.as_int() != 0 {
+            return None;
+        }
+        let entry = self.queue.pop_front()?;
+        Some(self.do_retire(entry))
+    }
+    fn space_available(&self) -> usize {
+        EXECUTE_RETIRE_PIPE_QUEUE_SIZE.saturating_sub(self.queue.len())
+    }
+    #[hdl]
+    fn start(&mut self, insn: &SimValue<WipDecodedInsn>, delay_sequence_index: &Cell<u64>) {
+        if !self.used_ids.insert(insn.id.clone()) {
+            panic!("next_pc gave a duplicate insn id: {insn:?}");
+        }
+        self.queue.push_back(
+            #[hdl(sim)]
+            ExecuteRetirePipeQueueEntry {
+                insn,
+                cycles_left: ExecuteRetirePipeQueueEntry::get_next_delay(delay_sequence_index),
+            },
+        );
+    }
+}
+
+#[hdl_module(extern)]
+fn mock_execute_retire_pipe(config: PhantomConst<CpuConfig>) {
+    #[hdl]
+    let cd: ClockDomain = m.input();
+    #[hdl]
+    let from_post_decode: PostDecodeOutputInterface<PhantomConst<CpuConfig>> =
+        m.input(PostDecodeOutputInterface[config]);
+    #[hdl]
+    let retire_output: RetireToNextPcInterface<PhantomConst<CpuConfig>> =
+        m.output(RetireToNextPcInterface[config]);
+    #[hdl]
+    let queue_debug: ArrayVec<
+        ExecuteRetirePipeQueueEntry,
+        ConstUsize<{ EXECUTE_RETIRE_PIPE_QUEUE_SIZE }>,
+    > = m.output();
+    m.register_clock_for_past(cd.clk);
+    m.extern_module_simulation_fn(
+        (cd, from_post_decode, retire_output, queue_debug),
+        |(cd, from_post_decode, retire_output, queue_debug), mut sim| async move {
+            // intentionally have a different sequence each time we're reset
+            let delay_sequence_index = Cell::new(0);
+            sim.resettable(
+                cd,
+                async |mut sim| {
+                    sim.write(from_post_decode.ready, 0usize).await;
+                    sim.write(
+                        retire_output.inner.data,
+                        retire_output.ty().inner.data.HdlNone(),
+                    )
+                    .await;
+                    sim.write(
+                        queue_debug,
+                        queue_debug
+                            .ty()
+                            .new_sim(ExecuteRetirePipeQueueEntry.default_sim()),
+                    )
+                    .await;
+                },
+                |sim, ()| {
+                    run_fn(
+                        cd,
+                        from_post_decode,
+                        retire_output,
+                        queue_debug,
+                        &delay_sequence_index,
+                        sim,
+                    )
+                },
+            )
+            .await;
+        },
+    );
+    #[hdl]
+    async fn run_fn(
+        cd: Expr<ClockDomain>,
+        from_post_decode: Expr<PostDecodeOutputInterface<PhantomConst<CpuConfig>>>,
+        retire_output: Expr<RetireToNextPcInterface<PhantomConst<CpuConfig>>>,
+        queue_debug: Expr<
+            ArrayVec<ExecuteRetirePipeQueueEntry, ConstUsize<{ EXECUTE_RETIRE_PIPE_QUEUE_SIZE }>>,
+        >,
+        delay_sequence_index: &Cell<u64>,
+        mut sim: ExternModuleSimulationState,
+    ) {
+        let config = from_post_decode.config.ty();
+        let mut state = MockExecuteState::new(config);
+        let empty_retire_insn = #[hdl(sim)]
+        RetireToNextPcInterfacePerInsn::<_> {
+            id: 0_hdl_u12,
+            next_pc: 0u64,
+            call_stack_op: #[hdl(sim)]
+            CallStackOp::None(),
+            cond_br_taken: #[hdl(sim)]
+            HdlNone(),
+            config,
+        };
+        let retire_vec_ty = retire_output.inner.data.ty().HdlSome.insns;
+        loop {
+            state.on_clock_cycle();
+            let mut sim_queue = queue_debug
+                .ty()
+                .new_sim(ExecuteRetirePipeQueueEntry.default_sim());
+            for entry in &state.queue {
+                ArrayVec::try_push_sim(&mut sim_queue, entry)
+                    .ok()
+                    .expect("queue is known to be small enough");
+            }
+            sim.write(queue_debug, sim_queue).await;
+            let mut retiring = retire_vec_ty.new_sim(&empty_retire_insn);
+            let mut peek_state = state.clone();
+            while let Some(peek_retire) = peek_state.try_retire() {
+                if peek_retire.is_err() && **ArrayVec::len_sim(&retiring) > 0 {
+                    break;
+                }
+                let peek_retire = peek_retire.unwrap_or_else(|_| {
+                    #[hdl(sim)]
+                    RetireToNextPcInterfacePerInsn::<_> {
+                        id: 0_hdl_u12,
+                        next_pc: u64::from_be_bytes(*b"ErrError"),
+                        call_stack_op: #[hdl(sim)]
+                        CallStackOp::None(),
+                        cond_br_taken: #[hdl(sim)]
+                        HdlNone(),
+                        config,
+                    }
+                });
+                let Ok(_) = ArrayVec::try_push_sim(&mut retiring, peek_retire) else {
+                    break;
+                };
+            }
+            sim.write(
+                retire_output.inner.data,
+                if **ArrayVec::len_sim(&retiring) > 0 {
+                    #[hdl(sim)]
+                    (retire_output.inner.data.ty()).HdlSome(
+                        #[hdl(sim)]
+                        RetireToNextPcInterfaceInner::<_> {
+                            insns: retiring,
+                            config,
+                        },
+                    )
+                } else {
+                    #[hdl(sim)]
+                    (retire_output.inner.data.ty()).HdlNone()
+                },
+            )
+            .await;
+            sim.write(
+                from_post_decode.ready,
+                state.space_available().min(config.get().fetch_width.get()),
+            )
+            .await;
+            sim.wait_for_clock_edge(cd.clk).await;
+            if sim.read_past_bool(retire_output.inner.ready, cd.clk).await {
+                for _ in 0..**ArrayVec::len_sim(&retiring) {
+                    match state.try_retire() {
+                        Some(Ok(_)) => {}
+                        Some(Err(e)) => panic!("retire error: {e}"),
+                        None => unreachable!(),
+                    }
+                }
+            }
+            let mut new_insns = sim.read_past(from_post_decode.insns, cd.clk).await;
+            ArrayVec::truncate_sim(
+                &mut new_insns,
+                *sim.read_past(from_post_decode.ready, cd.clk).await,
+            );
+            for insn in ArrayVec::elements_sim_ref(&new_insns) {
+                state.start(insn, delay_sequence_index);
+            }
+        }
+    }
+}
+
 #[hdl_module]
 fn dut(config: PhantomConst<CpuConfig>) {
     #[hdl]
     let cd: ClockDomain = m.input();
     #[hdl]
     let next_pc = instance(next_pc(config));
-    connect(next_pc.cd, cd);
+    #[hdl]
+    let next_pc {
+        cd: next_pc_cd,
+        to_fetch: next_pc_to_fetch,
+        from_decode: next_pc_from_decode,
+        post_decode_output: next_pc_post_decode_output,
+        from_retire: next_pc_from_retire,
+        state_for_debug: _,
+    } = next_pc;
+    connect(next_pc_cd, cd);
     #[hdl]
     let mock_fetch_pipe = instance(mock_fetch_pipe(config));
-    connect(mock_fetch_pipe.cd, cd);
-    connect(mock_fetch_pipe.from_fetch, next_pc.to_fetch);
-    connect(next_pc.from_decode, mock_fetch_pipe.to_post_decode);
+    #[hdl]
+    let mock_fetch_pipe {
+        cd: mock_fetch_pipe_cd,
+        from_fetch: mock_fetch_pipe_from_fetch,
+        to_post_decode: mock_fetch_pipe_to_post_decode,
+        queue_debug: _,
+    } = mock_fetch_pipe;
+    connect(mock_fetch_pipe_cd, cd);
+    connect(mock_fetch_pipe_from_fetch, next_pc_to_fetch);
+    connect(next_pc_from_decode, mock_fetch_pipe_to_post_decode);
+    #[hdl]
+    let mock_execute_retire_pipe = instance(mock_execute_retire_pipe(config));
+    #[hdl]
+    let mock_execute_retire_pipe {
+        cd: mock_execute_retire_pipe_cd,
+        from_post_decode: mock_execute_retire_pipe_from_post_decode,
+        retire_output: mock_execute_retire_pipe_retire_output,
+        queue_debug: _,
+    } = mock_execute_retire_pipe;
+    connect(mock_execute_retire_pipe_cd, cd);
+    connect(next_pc_from_retire, mock_execute_retire_pipe_retire_output);
+    connect(
+        mock_execute_retire_pipe_from_post_decode,
+        next_pc_post_decode_output,
+    );
 }
 
 #[hdl]
@@ -330,6 +805,20 @@ fn test_next_pc() {
     let mut sim = Simulation::new(m);
     let mut writer = RcWriter::default();
     sim.add_trace_writer(VcdWriterDecls::new(writer.clone()));
+    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 {
@@ -340,7 +829,7 @@ fn test_next_pc() {
         sim.write_reset(sim.io().cd.rst, false);
     }
     // FIXME: vcd is just whatever next_pc does now, which isn't known to be correct
-    let vcd = String::from_utf8(writer.take()).unwrap();
+    let vcd = String::from_utf8(writer.writer.take().unwrap().take()).unwrap();
     println!("####### VCD:\n{vcd}\n#######");
     if vcd != include_str!("expected/next_pc.vcd") {
         panic!();