Skip to content

fix(verilog): keep simtime monotonic in readdata - #604

Open
Sahil-u07 wants to merge 1 commit into
ControlCore-Project:devfrom
Sahil-u07:fix/verilog-read-simtime
Open

Sahil-u07 wants to merge 1 commit into
ControlCore-Project:devfrom
Sahil-u07:fix/verilog-read-simtime

Conversation

@Sahil-u07

Copy link
Copy Markdown
Contributor

readdata in concore.v was writing the file's timestamp straight into simtime, so an older value on another port moved simtime back, and a missing file reset it to whatever the init string had (usually 0). Every other binding does simtime = max(simtime, read_time), so this makes Verilog do the same. Same fix in concoredocker.v.

Also added tests/verilog/test_read_simtime.v, which runs the two read_file cases from the protocol fixtures against concore.v, and added it to CI in the job that already installs iverilog. Both cases fail on dev and pass with this change, so I marked those two verilog rows in the phase 2 matrix as observed_pass. The other verilog rows are params/initval/ZMQ which concore.v doesn't have, so I left them as is.

Tested locally with Icarus Verilog 14, pytest tests/test_protocol_conformance_phase2.py, and ruff.

readdata passed the module simtime straight to literal_eval as its
output, so every read overwrote simtime with the timestamp from the
file. Reading an older value moved simtime back, and a missing file
reset it to the timestamp in the init string (usually 0). The other
bindings all do simtime = max(simtime, read_time).

Parse into a local and only move simtime forward. Same change in
concoredocker.v.

Added tests/verilog/test_read_simtime.v with the two read_file cases
from the protocol fixtures, run it in CI next to the Verilog interop
test, and marked those two verilog rows in the phase 2 matrix as
observed_pass.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant