Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix simulation failure with Verilator v5.x on Windows #1618

Open
wants to merge 2 commits into
base: dev
Choose a base branch
from
Open

Fix simulation failure with Verilator v5.x on Windows #1618

wants to merge 2 commits into from

Conversation

du33169
Copy link

@du33169 du33169 commented Dec 10, 2024

Closes #1572

Context, Motivation & Description

Simulation with verilator v5.x on Windows will fail with the following error:

Program 'verilator_bin.exe' failed to run: The specified executable is not a valid application for this OS platform.

It is caused by invoking verilator_bin.exe directly to get its version:

SpinalHDL/sim/src/main/scala/spinal/sim/VerilatorBackend.scala

Line 576 in fb2a8df

val verilatorVersionProcess = Process(Seq(verilatorBinFilename, "--version"), new File(workspacePath))

However, since verilator v5.x, the verilator_bin.exe file in /mingw64/bin installed with the MSYS2 package is actually a perl script. It runs as a normal perl script in the Unix-like MSYS2 shell, but will fail if run directly on Windows.

So it might be better to specify the shell, just like how spinalHDL runs verilatorScript.sh later:

SpinalHDL/sim/src/main/scala/spinal/sim/VerilatorBackend.scala

Lines 688 to 691 in fb2a8df

val shCommand = if(isWindows) "sh.exe" else "sh"
val logger = new Logger()
assert(Process(Seq(shCommand, "verilatorScript.sh"),
new File(workspacePath)).! (logger) == 0, "Verilator invocation failed\n" + logger.outStr.toString())

This way we can also use a single val verilatorBinFilename = "verilator" for both Windows and Linux.

Impact on code generation

No impact, ideally.

Checklist

  • Unit tests were added
  • API changes are or will be documented:

No changes to unit tests or API.

Additional Contents

It works for simple cases on my machine. But there are still two problems:

  • Even without this patch, some tests will fail on Windows for the dev branch, and I failed to build tag v1.10.2. Is there a stable branch to start with?
  • The task hangs strangely after the simulation run is done. But debugging and testing can finish successfully.

@du33169 du33169 mentioned this pull request Dec 10, 2024
Open
@Readon
Copy link
Collaborator

Readon commented Dec 10, 2024

So, the assumption is that sh.exe is installed in the environment.
It would be best to test this on various cases. Windows with MSYS2 setting, MSYS2, Linux, or even MacOS.

@du33169
Copy link
Author

du33169 commented Dec 10, 2024

This PR is not the first to introduce sh.exe, it has been used by spinalHDL to run verilatorScript.sh for a long time. If it worked before, it should not be a problem here.

@Dolu1990
Copy link
Member

This PR is not the first to introduce sh.exe, it has been used by spinalHDL to run verilatorScript.sh for a long time. If it worked before, it should not be a problem here.

Ahhh right sh.exe is alread used when generating the cache, should be all good then.

All good for merge @Readon ?

@Readon
Copy link
Collaborator

Readon commented Dec 11, 2024

Unfortunately, I could not make it work on MSYS2 environment.
It reports

Can't exec "$MSYS2_ROOT$/share/verilator//verilator_bin": No such file or directory at /mingw64/bin/verilator line 188.er / Test / testOnly 90s

However, in dev branch it works.

@Readon
Copy link
Collaborator

Readon commented Dec 11, 2024 •
edited
Loading

Unfortunately, I could not make it work on MSYS2 environment. It reports

Can't exec "$MSYS2_ROOT$/share/verilator//verilator_bin": No such file or directory at /mingw64/bin/verilator line 188.er / Test / testOnly 90s

However, in dev branch it works.

So I find that when this PR is applied, then VERILATOR_ROOT should not be set.
After unset VERILATOR_ROOT, the verilator in both versions 4.228 and 5.024 work.

@du33169
Copy link
Author

du33169 commented Dec 12, 2024

Yes, in this way the original documentation still applies. Forget about any additional operations from the issue.

The only problem is, the simulation task won't exit on my machine. Is that reproducible on yours? This only happens for Verilator v5.x.

@Readon
Copy link
Collaborator

Readon commented Dec 13, 2024

In some cases, it would not stop too. That's why I always recommand do not use the version 5.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Problems when running simulation with verilator v5.xxx on Windows
3 participants
@du33169 @Readon @Dolu1990