Skip to content

Conversation

@toddstrader
Copy link
Contributor

vcddiff currently only works on seekable files. That means while this obviously has no diffs:

vcddiff foo.vcd foo.vcd

this (incorrectly) does:

vcddiff <(cat foo.vcd) <(cat foo.vcd)

and more importantly so does:

vcddiff <(fst2vcd foo.fst) <(fst2vcd foo.fst)

This is not ideal for very large VCDs. We could instead make temporary files and open them instead but that's less ideal for cases which do reasonably fit in memory. In any case, this is less surprising than getting a diff between a file and itself.

Copy link
Member

@wsnyder wsnyder left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. Using a personal Verilator branch, edit ci/ci-install.bash to point to your branch with this change, and make sure all tests pass on all platforms. I'm especially worried about Windows.

@toddstrader
Copy link
Contributor Author

I'm with you that adding more system calls may run into complications on Windows. However, I don't think ci-install.bash is used for Verilator Windows CI. And it appears that we do very little testing in ci-win-test.ps1, mainly we just run examples/cmake_tracing_c:
https://github.com/toddstrader/verilator/actions/runs/19572878666/job/56050498001

I'd suggest getting test_regress going on Windows in the Verilator CI, but I'm guessing there are reasons that's not already happening. So would a better way forward here be to spin up some simple, multi-OS CI for this repo? This PR is clearly not urgent, but if so I can take a stab at that when time allows.

@wsnyder
Copy link
Member

wsnyder commented Nov 21, 2025

Yes, a pull to add here all of the same runner flavors as we use on Verilator would be appreciated. It was just that no one got to it.

@wsnyder
Copy link
Member

wsnyder commented Nov 25, 2025

Thanks for the CI tests, please merge master into this, add a test, and once passes will be good to merge.

@toddstrader
Copy link
Contributor Author

As predicted, this doesn't play nice with Windows. And based on my limited research, there's not a direct analogue to unix fifos in Windows. So I'm just leaving this as a non-Windows thing for now.

@wsnyder wsnyder merged commit 8a456a7 into veripool:master Nov 25, 2025
7 checks passed
@toddstrader toddstrader deleted the fifos branch November 25, 2025 20:07
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.

2 participants