Repository navigation
test(suite): add timed script operations and the timing oracle - #1354
Conversation
Add `run_until`/`run_to_finish` script operations and `Design::timed`, the `timing` SystemVerilog group, and retained Icarus/Verilator results for it. A timed script begins with `run_until` or `run_to_finish`, so no case depends on host inputs staged before time-zero initial blocks, always_ff listeners or variable initializers. The external adapters compile timed designs unchanged: the testbench settles with an Icarus read/write callback or a Verilator clocking event, and a design `$finish` notice before the script reaches `run_to_finish` fails the run. Port shapes of timed designs come from their declarations with process bodies blanked for analysis only. Celox runs the group through a timed `Simulation` with explicit exclusions: the SV analyzer rejects procedural timing controls and plain `always`, host clock writes do not trigger flip-flops, there is no four-state timed read, and only the default backend builds a timed simulation.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 99176c3316
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
Queued — the merge queue status continues in this comment ↓. |
- Validate timed sources with the macros each simulator predefines (VERILATOR, VERILATOR_TIMING, __ICARUS__), so guarded scale directives cannot bypass the rejection. - Drop the $finish notice after the finishing marker before splicing the log, so a partial last line before run_to_finish stays comparable. - Reject a first run_until target that reads the design: adapters evaluate it at different points of time zero. - Reject final procedures in timed designs; they run in an arbitrary order relative to the checks after run_to_finish (IEEE 1800-2023 9.2.3).
1e5cc8b to
25baccf
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 25baccff1a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
- Reject timed sources whose preprocessed text depends on the simulator's predefined macros, so the declaration pass that shapes the generated testbench sees exactly what each simulator compiles. - Place the run_to_finish deadline one picosecond past RUN_UNTIL_LIMIT, so a design $finish due exactly at the limit runs before the sentinel instead of racing it in the same time slot.
Merge Queue Status
This pull request spent 35 minutes 20 seconds in the queue, including 32 minutes 58 seconds running CI. Required conditions to merge
|
Step 2 of the #1123 redesign: the timed-design oracle, cut from #1159.
What lands
run_until/run_to_finish,Design::timed, theBackend::run_until/run_to_finishadapter methods, and validation that timed sources keep the injected 1 ns / 1 ps scale (explicittimescale/timeunit/timeprecision/resetallare rejected).timing.vtest: 55 cases (53 kept from test(suite)!: drive timed designs from shared cases #1159, several rewritten, plus 2 new harness cases).verification/sv/{icarus,verilator}/timing.json: Icarus 55/55 pass; Verilator 53 pass, 2 four-state casesunsupported(as in test(suite)!: drive timed designs from shared cases #1159).crates/celox/tests/frontends/systemverilog/timing.rs) with an explicit reason on every exclusion.Narrowed suite contract (no source rewriter)
timed_ff.rs,timed_initializers.rsandtimed_connections.rsare not brought over. Instead:run_untilorrun_to_finish; the parser rejects any other first statement. Inputs keep their fresh values until time zero has been drained, so no case depends on host values staged before time-zero initial blocks,always_fflisteners or variable initializers. A design also does not call$finishat time zero. Documented in the suite README andscript/mod.rs.$celox_suite_settle), Verilator via a clocking event sampled in the observed region.$finish: Icarus finishes the current time slot after a$finish, so the testbench could still reach its end. The testbench now prints a@suite finishingmarker when it reachesrun_to_finish. A simulator$finishnotice from a staged source before that marker fails the run. Afterrun_to_finish, a delay toRUN_UNTIL_LIMITkeeps an idle design from ending the run by running out of events.Cases changed relative to #1159
$fatalcontinuous drivers). Under the new contract they are either meaningless or time-zero races.run_until 0:edge_waits_see_the_signals_the_script_drives,four_state_edges_count_transitions_through_unknown,run_until_drains_target_time_before_later_writes,conditional_writes_…,case_writes_…,negative_repeat_counts_do_not_run_the_body.@(...)are nowbitvariables instead of nets. The nine…distinct_activation_times/false_wait…cases arm their observer at time 1 and write at 2 and 3. Icarus counted an extra time-zero wakeup that test(suite)!: drive timed designs from shared cases #1159's staged startup had hidden.initial_blocks_run_at_time_zero,host_edges_after_time_zero_clock_flip_flops.Celox exclusions (removed by step 3/4)
alwaysprocesses (#431)SimulationSimulationSimulationbuilds only the default backendinitial_blocks_run_at_time_zeropasses on native today. Every excluded test fails when forced with--ignored, so the exclusions are real. No changes tocrates/celox/src(runtime and builder work is step 3).Validation
cargo test -p celox --features systemverilog --test systemverilog: 2473 passed, 294 ignored (timing exclusions plus 19 pre-existing).cargo test -p celox-test-suite --features icarus,verilator, includingretained_reports.--test oracles -- --ignoredand--test timed_protocol -- --ignored: early finish, finish at therun_untiltarget, finish during eval, and idle exhaustion are all rejected by both tools.verify-sv-icarus/verify-sv-verilator --fresh --filter timing::: results above.verify-sv-icarus --fresh --filter system_functions::matches the retained evidence; Verylcounter::on both tools.cargo clippywith CI's package and feature set;cargo fmt --check.