From 0d4310b549c519647a91c8ae889b309bdca7f902 Mon Sep 17 00:00:00 2001 From: Nikil-Shyamsunder Date: Thu, 12 Feb 2026 21:03:13 -0800 Subject: [PATCH 1/4] round-trip testing + interp and monitor changes to reconcile the two --- monitor/src/interpreter.rs | 27 +-- monitor/src/scheduler.rs | 52 ++++-- .../fpga-debugging/axi-stream-s2/s2_buggy.out | 19 -- .../fpga-debugging/axi-stream-s2/s2_fixed.out | 38 ---- protocols/src/scheduler.rs | 4 + scripts/roundtrip.out | 32 ++++ scripts/roundtrip.py | 169 ++++++++++++++++++ 7 files changed, 246 insertions(+), 95 deletions(-) create mode 100644 scripts/roundtrip.out create mode 100644 scripts/roundtrip.py diff --git a/monitor/src/interpreter.rs b/monitor/src/interpreter.rs index 9c565251..f59bd5a4 100644 --- a/monitor/src/interpreter.rs +++ b/monitor/src/interpreter.rs @@ -50,28 +50,15 @@ impl Interpreter { &self, struct_name: Option, ctx: &GlobalContext, - trace: &WaveSignalTrace, + _trace: &WaveSignalTrace, ) -> ProtocolApplication { let mut serialized_args = vec![]; for arg in &self.transaction.args { let symbol_id = arg.symbol(); - let name = self.symbol_table[symbol_id].full_name(&self.symbol_table); - let value = self.args_mapping.get(&symbol_id).unwrap_or_else(|| { - let time_str = if ctx.show_waveform_time { - trace.format_time(trace.time_step(), ctx.time_unit) - } else { - format!("cycle {}", self.trace_cycle_count) - }; - panic!( - "Transaction `{}`, {}: Unable to find value for {} ({}) in args_mapping, which is {{ {} }}", - self.transaction.name, - time_str, - name, - symbol_id, - serialize_args_mapping(&self.args_mapping, &self.symbol_table, ctx.display_hex) - ) - }); - serialized_args.push(serialize_bitvec(value, ctx.display_hex)); + match self.args_mapping.get(&symbol_id) { + Some(value) => serialized_args.push(serialize_bitvec(value, ctx.display_hex)), + None => serialized_args.push("?".to_string()), + } } ProtocolApplication { struct_name, @@ -488,7 +475,9 @@ impl Interpreter { } Stmt::Block(stmt_ids) => { if stmt_ids.is_empty() { - Ok(None) + // Empty block — use next_stmt_map to continue + // to the next statement in the parent scope + Ok(self.next_stmt_map[stmt_id]) } else { Ok(Some(stmt_ids[0])) } diff --git a/monitor/src/scheduler.rs b/monitor/src/scheduler.rs index 707d3ceb..e18f59aa 100644 --- a/monitor/src/scheduler.rs +++ b/monitor/src/scheduler.rs @@ -432,29 +432,31 @@ impl Scheduler { } let finished_thread = &finished[0]; - // ...and there shouldn't be any other threads in `next` - let next = threads_with_start_time(&self.next, start_cycle); - if !next.is_empty() { - let start_time_str = if ctx.show_waveform_time { - trace.format_time(finished_thread.start_time_step, ctx.time_unit) - } else { - format!("cycle {}", finished_thread.start_cycle) - }; - let error_context = anyhow!( - "Thread {} (`{}`) finished but there are other threads with the same start time ({}) in the `next` queue, namely {:?}", + // ...any other threads from the same start cycle still in `next` + // are slower siblings that lost the race — move them to `failed` + let sibling_names: Vec = self + .next + .iter() + .filter(|t| t.start_cycle == start_cycle) + .map(|t| t.transaction.name.clone()) + .collect(); + if !sibling_names.is_empty() { + info!( + "Thread {} (`{}`) finished; moving slower siblings from `next` to `failed`: {:?}", finished_thread.global_thread_id(ctx), finished_thread.transaction.name, - start_time_str, - next.iter() - .map(|t| t.transaction.name.clone()) - .collect::>() + sibling_names ); - - return Err(SchedulerError::NoTransactionsMatch { - struct_name: self.struct_name.clone(), - error_context, - }); } + let mut remaining_next = Queue::new(); + for thread in self.next.drain(..) { + if thread.start_cycle == start_cycle { + self.failed.push_back(thread); + } else { + remaining_next.push_back(thread); + } + } + self.next = remaining_next; } // Next, find the unique start cycles of all threads in `failed` @@ -748,6 +750,18 @@ impl Scheduler { } } Ok(None) => { + // Check if the last executed statement was `step()`. + // If not, this protocol is ill-formed and should be discarded. + if !matches!(thread.transaction[current_stmt_id], Stmt::Step) { + info!( + "Thread {} (`{}`) didn't end with `step()`, marking as failed.", + thread.global_thread_id(ctx), + self.format_transaction_name(ctx, thread.transaction.name.clone()) + ); + self.failed.push_back(thread); + return Ok(ThreadResult::Completed); + } + // Check if another thread has already finished in this cycle. // Invariant: Only one thread per struct can finish per cycle if let Some((first_start_cycle, first_thread_id, first_transaction_name)) = diff --git a/monitor/tests/fpga-debugging/axi-stream-s2/s2_buggy.out b/monitor/tests/fpga-debugging/axi-stream-s2/s2_buggy.out index 37f1e4de..c36bbd00 100644 --- a/monitor/tests/fpga-debugging/axi-stream-s2/s2_buggy.out +++ b/monitor/tests/fpga-debugging/axi-stream-s2/s2_buggy.out @@ -1,4 +1,3 @@ -Trace 0: reset() // [time: 0ns -> 12.5ns] (thread 0) wait_for_data() // [time: 337.5ns -> 362.5ns] (thread 19) wait_for_data() // [time: 387.5ns -> 412.5ns] (thread 21) @@ -13,21 +12,3 @@ recv(3) // [time: 912.5ns -> 937.5ns] (thread 42) recv(4) // [time: 937.5ns -> 962.5ns] (thread 43) recv(5) // [time: 962.5ns -> 987.5ns] (thread 44) recv(6) // [time: 987.5ns -> 1012.5ns] (thread 45) -stall(7, 0) // [time: 1012.5ns -> 1037.5ns] (thread 46) -stall(7, 1) // [time: 1037.5ns -> 1050ns] (thread 47) -Trace 1: -reset() // [time: 0ns -> 12.5ns] (thread 0) -wait_for_data() // [time: 337.5ns -> 362.5ns] (thread 19) -wait_for_data() // [time: 387.5ns -> 412.5ns] (thread 21) -wait_for_data() // [time: 512.5ns -> 537.5ns] (thread 26) -wait_for_data() // [time: 537.5ns -> 562.5ns] (thread 27) -wait_for_data() // [time: 612.5ns -> 637.5ns] (thread 30) -wait_for_data() // [time: 787.5ns -> 812.5ns] (thread 37) -wait_for_data() // [time: 837.5ns -> 862.5ns] (thread 39) -recv(1) // [time: 862.5ns -> 887.5ns] (thread 40) -recv(2) // [time: 887.5ns -> 912.5ns] (thread 41) -recv(3) // [time: 912.5ns -> 937.5ns] (thread 42) -recv(4) // [time: 937.5ns -> 962.5ns] (thread 43) -recv(5) // [time: 962.5ns -> 987.5ns] (thread 44) -recv(6) // [time: 987.5ns -> 1012.5ns] (thread 45) -stall(7, 1) // [time: 1037.5ns -> 1050ns] (thread 47) diff --git a/monitor/tests/fpga-debugging/axi-stream-s2/s2_fixed.out b/monitor/tests/fpga-debugging/axi-stream-s2/s2_fixed.out index 6e1b7d21..b6d80d73 100644 --- a/monitor/tests/fpga-debugging/axi-stream-s2/s2_fixed.out +++ b/monitor/tests/fpga-debugging/axi-stream-s2/s2_fixed.out @@ -1,41 +1,3 @@ -Trace 0: -reset() // [time: 0ns -> 12.5ns] (thread 0) -wait_for_data() // [time: 337.5ns -> 362.5ns] (thread 19) -wait_for_data() // [time: 387.5ns -> 412.5ns] (thread 21) -wait_for_data() // [time: 512.5ns -> 537.5ns] (thread 26) -wait_for_data() // [time: 537.5ns -> 562.5ns] (thread 27) -wait_for_data() // [time: 612.5ns -> 637.5ns] (thread 30) -wait_for_data() // [time: 787.5ns -> 812.5ns] (thread 37) -wait_for_data() // [time: 837.5ns -> 862.5ns] (thread 39) -recv(1) // [time: 862.5ns -> 887.5ns] (thread 40) -recv(2) // [time: 887.5ns -> 912.5ns] (thread 41) -recv(3) // [time: 912.5ns -> 937.5ns] (thread 42) -recv(4) // [time: 937.5ns -> 962.5ns] (thread 43) -recv(5) // [time: 962.5ns -> 987.5ns] (thread 44) -recv(6) // [time: 987.5ns -> 1012.5ns] (thread 45) -stall(7, 0) // [time: 1012.5ns -> 1037.5ns] (thread 46) -stall(7, 0) // [time: 1037.5ns -> 1062.5ns] (thread 47) -reset() // [time: 1062.5ns -> 1087.5ns] (thread 48) -reset() // [time: 1087.5ns -> 1100ns] (thread 49) -Trace 1: -reset() // [time: 0ns -> 12.5ns] (thread 0) -wait_for_data() // [time: 337.5ns -> 362.5ns] (thread 19) -wait_for_data() // [time: 387.5ns -> 412.5ns] (thread 21) -wait_for_data() // [time: 512.5ns -> 537.5ns] (thread 26) -wait_for_data() // [time: 537.5ns -> 562.5ns] (thread 27) -wait_for_data() // [time: 612.5ns -> 637.5ns] (thread 30) -wait_for_data() // [time: 787.5ns -> 812.5ns] (thread 37) -wait_for_data() // [time: 837.5ns -> 862.5ns] (thread 39) -recv(1) // [time: 862.5ns -> 887.5ns] (thread 40) -recv(2) // [time: 887.5ns -> 912.5ns] (thread 41) -recv(3) // [time: 912.5ns -> 937.5ns] (thread 42) -recv(4) // [time: 937.5ns -> 962.5ns] (thread 43) -recv(5) // [time: 962.5ns -> 987.5ns] (thread 44) -recv(6) // [time: 987.5ns -> 1012.5ns] (thread 45) -stall(7, 0) // [time: 1012.5ns -> 1037.5ns] (thread 46) -reset() // [time: 1062.5ns -> 1087.5ns] (thread 48) -reset() // [time: 1087.5ns -> 1100ns] (thread 49) -Trace 2: reset() // [time: 0ns -> 12.5ns] (thread 0) wait_for_data() // [time: 337.5ns -> 362.5ns] (thread 19) wait_for_data() // [time: 387.5ns -> 412.5ns] (thread 21) diff --git a/protocols/src/scheduler.rs b/protocols/src/scheduler.rs index bb87d0c0..aea09387 100644 --- a/protocols/src/scheduler.rs +++ b/protocols/src/scheduler.rs @@ -403,6 +403,10 @@ impl<'a> Scheduler<'a> { } } + // Final sim step so the FST waveform always has at least one time entry + // (needed for combinational-only designs where no cycle advances occur) + self.evaluator.sim_step(); + // Emit diagnostics for all errors after execution is complete self.emit_all_diagnostics(); self.results.clone() diff --git a/scripts/roundtrip.out b/scripts/roundtrip.out new file mode 100644 index 00000000..d47110ec --- /dev/null +++ b/scripts/roundtrip.out @@ -0,0 +1,32 @@ + +=== Round-trip results === + Passed: 28 / 33 + Failed: 5 / 33 + Skipped: 37 (transactions don't complete successfully) + +Monitor failures: + + --- protocols/tests/adders/adder_d1/busy_wait_pass.tx --- + thread 'main' panicked at monitor/src/interpreter.rs:466:17: + not yet implemented: Bounded loops is not yet implemented in the monitor + note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace + + --- protocols/tests/adders/adder_d1/loop_with_assigns.tx --- + thread 'main' panicked at monitor/src/interpreter.rs:466:17: + not yet implemented: Bounded loops is not yet implemented in the monitor + note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace + + --- protocols/tests/adders/adder_d1/nested_busy_wait.tx --- + thread 'main' panicked at monitor/src/interpreter.rs:466:17: + not yet implemented: Bounded loops is not yet implemented in the monitor + note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace + + --- protocols/tests/fifo/push_pop_loop_empty.tx --- + thread 'main' panicked at monitor/src/interpreter.rs:466:17: + not yet implemented: Bounded loops is not yet implemented in the monitor + note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace + + --- protocols/tests/fifo/push_pop_loop_not_empty.tx --- + thread 'main' panicked at monitor/src/interpreter.rs:466:17: + not yet implemented: Bounded loops is not yet implemented in the monitor + note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace diff --git a/scripts/roundtrip.py b/scripts/roundtrip.py new file mode 100644 index 00000000..dad5bb44 --- /dev/null +++ b/scripts/roundtrip.py @@ -0,0 +1,169 @@ +#!/usr/bin/env python3 +""" +Round-trip test: for each passing .tx test, run the interpreter to generate +an FST waveform, then run the monitor on that FST. Reports success/fail counts. + +Usage: python scripts/roundtrip.py + (or via justfile: just roundtrip) + +TODO: Compare monitor output against original .tx transactions. +""" + +import os +import re +import subprocess +import tempfile +from pathlib import Path +from typing import Optional + +PROJECT_ROOT = Path(__file__).resolve().parent.parent + + +def find_turnt_dir(start: Path) -> Optional[Path]: + """Walk up from `start` to find the nearest directory containing turnt.toml.""" + d = start + while d != d.parent: + if (d / "turnt.toml").exists(): + return d + d = d.parent + return None + + +def parse_arg(args: str, flag: str) -> Optional[str]: + """Extract value for --flag or --flag=value from an ARGS string.""" + m = re.search(rf"--{flag}[= ](\S+)", args) + return m.group(1) if m else None + + +def extract_struct_name(prot_path: Path) -> Optional[str]: + """Extract the first struct name from a .prot file.""" + m = re.search(r"^struct\s+([A-Za-z_]\w*)", prot_path.read_text(), re.MULTILINE) + return m.group(1) if m else None + + +def collect_tx_files() -> list[Path]: + """Find all .tx files under PROJECT_ROOT, sorted.""" + return sorted(PROJECT_ROOT.rglob("*.tx")) + + +def run_roundtrip(): + passed = 0 + failed = 0 + skipped = 0 + interp_failed = 0 + failed_entries: list[tuple[str, str]] = [] + interp_failed_files: list[str] = [] + + for tx_file in collect_tx_files(): + tx_rel = str(tx_file.relative_to(PROJECT_ROOT)) + text = tx_file.read_text() + + # Parse // ARGS: line + args_match = re.search(r"^// ARGS:\s*(.+)$", text, re.MULTILINE) + if not args_match: + skipped += 1 + continue + args = args_match.group(1) + + # Skip error tests (non-zero RETURN code) + return_match = re.search(r"^// RETURN:\s*(\d+)", text, re.MULTILINE) + if return_match and int(return_match.group(1)) != 0: + skipped += 1 + continue + + # Extract --protocol and --verilog paths + prot_rel = parse_arg(args, "protocol") + verilog_rel = parse_arg(args, "verilog") + if not prot_rel or not verilog_rel: + skipped += 1 + continue + + # ARGS paths are relative to the nearest turnt.toml directory + base_dir = find_turnt_dir(tx_file.parent) + if not base_dir: + skipped += 1 + continue + + prot_file = base_dir / prot_rel + if not prot_file.exists(): + skipped += 1 + continue + + # Extract struct name from .prot file + struct_name = extract_struct_name(prot_file) + if not struct_name: + skipped += 1 + continue + + # Instance name: --module if specified, otherwise verilog filename stem + module_name = parse_arg(args, "module") + instance_name = module_name if module_name else Path(verilog_rel).stem + + # Step 1: Run interpreter to generate FST + with tempfile.NamedTemporaryFile(suffix=".fst", delete=False) as tmp: + fst_file = tmp.name + + try: + interp_cmd = ( + f"cargo run --quiet --package protocols-interp -- " + f"--color never --transactions {tx_file} {args} --fst {fst_file}" + ) + result = subprocess.run( + interp_cmd, shell=True, cwd=base_dir, + capture_output=True, text=True, + ) + if result.returncode != 0: + interp_failed += 1 + interp_failed_files.append(tx_rel) + continue + + # Step 2: Run monitor on the generated FST + monitor_cmd = ( + f"cargo run --quiet --package protocols-monitor -- " + f"-p {prot_file} --wave {fst_file} " + f"--instances {instance_name}:{struct_name}" + ) + result = subprocess.run( + monitor_cmd, shell=True, cwd=base_dir, + capture_output=True, text=True, + ) + if result.returncode == 0: + passed += 1 + else: + failed += 1 + output = (result.stdout + result.stderr).strip() + failed_entries.append((tx_rel, output)) + finally: + try: + os.unlink(fst_file) + except OSError: + pass + + # Print results + total = passed + failed + print() + print("=== Round-trip results ===") + print(f" Passed: {passed} / {total}") + print(f" Failed: {failed} / {total}") + print(f" Skipped: {skipped} (transactions don't complete successfully)") + if interp_failed: + print(f" Interpreter failures: {interp_failed}") + + if interp_failed_files: + print() + print("Interpreter failures (unexpected — these should be passing tests):") + for f in interp_failed_files: + print(f" - {f}") + + if failed_entries: + print() + print("Monitor failures:") + for file, output in failed_entries: + print() + print(f" --- {file} ---") + for line in output.splitlines(): + print(f" {line}") + + +if __name__ == "__main__": + run_roundtrip() From e02ebadbbec54b8d57cd475c83da71c76cd3b2d0 Mon Sep 17 00:00:00 2001 From: Nikil-Shyamsunder Date: Thu, 12 Feb 2026 22:50:56 -0800 Subject: [PATCH 2/4] remove justfile comment --- scripts/roundtrip.py | 1 - 1 file changed, 1 deletion(-) diff --git a/scripts/roundtrip.py b/scripts/roundtrip.py index dad5bb44..dc7bec1a 100644 --- a/scripts/roundtrip.py +++ b/scripts/roundtrip.py @@ -4,7 +4,6 @@ an FST waveform, then run the monitor on that FST. Reports success/fail counts. Usage: python scripts/roundtrip.py - (or via justfile: just roundtrip) TODO: Compare monitor output against original .tx transactions. """ From ea2618e6d02872a3dee71f6fbcab33f93f408b9c Mon Sep 17 00:00:00 2001 From: Nikil-Shyamsunder Date: Fri, 13 Feb 2026 18:26:32 -0800 Subject: [PATCH 3/4] remove monitor logic to kill transactions of different finishing times, add fork() step() to end of a protocol --- monitor/src/scheduler.rs | 27 ------------------- .../axi-stream-s2/s2_fixed.prot | 2 ++ 2 files changed, 2 insertions(+), 27 deletions(-) diff --git a/monitor/src/scheduler.rs b/monitor/src/scheduler.rs index e18f59aa..e0a0d7bc 100644 --- a/monitor/src/scheduler.rs +++ b/monitor/src/scheduler.rs @@ -430,33 +430,6 @@ impl Scheduler { .collect::>() ))); } - let finished_thread = &finished[0]; - - // ...any other threads from the same start cycle still in `next` - // are slower siblings that lost the race — move them to `failed` - let sibling_names: Vec = self - .next - .iter() - .filter(|t| t.start_cycle == start_cycle) - .map(|t| t.transaction.name.clone()) - .collect(); - if !sibling_names.is_empty() { - info!( - "Thread {} (`{}`) finished; moving slower siblings from `next` to `failed`: {:?}", - finished_thread.global_thread_id(ctx), - finished_thread.transaction.name, - sibling_names - ); - } - let mut remaining_next = Queue::new(); - for thread in self.next.drain(..) { - if thread.start_cycle == start_cycle { - self.failed.push_back(thread); - } else { - remaining_next.push_back(thread); - } - } - self.next = remaining_next; } // Next, find the unique start cycles of all threads in `failed` diff --git a/monitor/tests/fpga-debugging/axi-stream-s2/s2_fixed.prot b/monitor/tests/fpga-debugging/axi-stream-s2/s2_fixed.prot index afb0eccc..cbb5bcd1 100644 --- a/monitor/tests/fpga-debugging/axi-stream-s2/s2_fixed.prot +++ b/monitor/tests/fpga-debugging/axi-stream-s2/s2_fixed.prot @@ -90,6 +90,8 @@ prot stall(out data: u32, out last: u1) { // Verify outputs remained stable during the stall assert_eq(DUT.i_tdata, data); assert_eq(DUT.i_tlast, last); + fork(); + step(); } // WAIT_FOR_DATA: Receiver is ready but no data is available From d4c98e0bcbd15e6346f0e8bf0231daed1a5c151d Mon Sep 17 00:00:00 2001 From: Nikil-Shyamsunder Date: Fri, 13 Feb 2026 18:31:22 -0800 Subject: [PATCH 4/4] add new behavior for s2_fixed to snapshot tests --- .../fpga-debugging/axi-stream-s2/s2_fixed.out | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/monitor/tests/fpga-debugging/axi-stream-s2/s2_fixed.out b/monitor/tests/fpga-debugging/axi-stream-s2/s2_fixed.out index b6d80d73..0b74c939 100644 --- a/monitor/tests/fpga-debugging/axi-stream-s2/s2_fixed.out +++ b/monitor/tests/fpga-debugging/axi-stream-s2/s2_fixed.out @@ -1,3 +1,22 @@ +Trace 0: +reset() // [time: 0ns -> 12.5ns] (thread 0) +wait_for_data() // [time: 337.5ns -> 362.5ns] (thread 19) +wait_for_data() // [time: 387.5ns -> 412.5ns] (thread 21) +wait_for_data() // [time: 512.5ns -> 537.5ns] (thread 26) +wait_for_data() // [time: 537.5ns -> 562.5ns] (thread 27) +wait_for_data() // [time: 612.5ns -> 637.5ns] (thread 30) +wait_for_data() // [time: 787.5ns -> 812.5ns] (thread 37) +wait_for_data() // [time: 837.5ns -> 862.5ns] (thread 39) +recv(1) // [time: 862.5ns -> 887.5ns] (thread 40) +recv(2) // [time: 887.5ns -> 912.5ns] (thread 41) +recv(3) // [time: 912.5ns -> 937.5ns] (thread 42) +recv(4) // [time: 937.5ns -> 962.5ns] (thread 43) +recv(5) // [time: 962.5ns -> 987.5ns] (thread 44) +recv(6) // [time: 987.5ns -> 1012.5ns] (thread 45) +stall(7, 0) // [time: 1012.5ns -> 1062.5ns] (thread 46) +reset() // [time: 1062.5ns -> 1087.5ns] (thread 48) +reset() // [time: 1087.5ns -> 1100ns] (thread 49) +Trace 1: reset() // [time: 0ns -> 12.5ns] (thread 0) wait_for_data() // [time: 337.5ns -> 362.5ns] (thread 19) wait_for_data() // [time: 387.5ns -> 412.5ns] (thread 21)