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.

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

Question: Why are VPI routines not exported on Windows? #395

Closed
ktbarrett opened this issue Nov 23, 2020 · 10 comments
Closed

Question: Why are VPI routines not exported on Windows? #395

ktbarrett opened this issue Nov 23, 2020 · 10 comments

Comments

@ktbarrett
Copy link

I see there has been a change to how VPI routines are intended to be used in this project. It seems to me this was done to make writing VPI libraries on Windows a little easier. The relevant parts have been done ahead-of-time for the user and wrapped into a nice interface. This is fine.

However, the VPI routines have had their exportation removed. This has broken cocotb; which generates an import library for the VPI from a .def file and does not and cannot use the icarus-vpi tool. Removing the exportation of routines from a DLL is a breaking interface change and, in this case, is wholly unnecessary. Was there a justification for this decision?

@martinwhitaker
Copy link
Collaborator

For the rationale for the change, see the thread starting at https://sourceforge.net/p/iverilog/mailman/message/36789170/, which was triggered by the problems described in the thread starting at https://sourceforge.net/p/iverilog/mailman/message/36783845/.

Windows wasn't a reason for making the change; in fact it was the main obstacle to making the change. In the old scheme of things, under Windows it wasn't possible for the compiler to load the VPI modules because they had been linked against the VPI routines in vvp. So I had to come up with a different scheme for dynamically linking the VPI routines.

vvp isn't a library (and so doesn't have a defined API/ABI), so I didn't see a need to retain the old exports.

@ktbarrett
Copy link
Author

So, correct me if I'm wrong in my summation. There are two separate entities in this project that consume a user's VPI app: the compiler (iverilog) and the runtime (vvp). VPI is necessary in the compiler for registering and running compiletf callbacks. This causes a problem with Windows since the VPI user library has to load the VPI from two different objects, while traditional tools for creating import libraries on Windows only support linking against one. As a result, the user would have to build their app twice, once targeting the compiler object and another targeting the runtime object. To avoid that you have them compile it once and feed them a table of function pointers to the VPI functions. libvpi.c basically does that work for them.

Now that I understand the problem a bit better I can understand why this change was made. In fact we are encountering the same issue within cocotb where we have to generate multiple instances of the VPI library for each potential target simulator.

I think our problem is now you have a non-standardized interface for working around the issue; and we are going to have to make a point solution to support it. If we decide to take a similar approach to how we load VPI routines (so we can have a single VPI implementation for all simulators), it will probably look like your solution. Have you thought about standardizing it?

@steveicarus
Copy link
Owner

Wait a minute. Are we saying that vvp under windows cannot load VPI modules? That can't be right?

@martinwhitaker
Copy link
Collaborator

@steveicarus, the VPI tests run fine under Windows - take a look at the CI results.

@ktbarrett, in fact the compiler just registers the user-defined task/functions. It does that to learn the function return types, which saves the user from creating a seperate SFT file to describe them. This step is optional - the support for SFT files is still present.

I was not expecting this to cause an issue. If you compile your VPI module using the Icarus vpi_user.h and link against libvpi, it should work. But maybe I'm forgetting some subtle detail - it's a year since I worked on this,

@ktbarrett
Copy link
Author

If you compile your VPI module using the Icarus vpi_user.h and link against libvpi

We don't link against libvpi and we use the vpi_user.h definition from the standard, not a simulator-specific one. That would require Icarus to be present before we compile the VPI extension, which we currently don't require. And the result would be an Icarus-specific VPI app. Ideally, we would have a single VPI implementation for all simulators that support VPI.

@ktbarrett
Copy link
Author

And it would have been nice to deal with this before the release, but that isn't any of your faults. We just got around to testing Icarus on master in CI on Windows, Mac, and Linux. We don't really have the manpower to chase down all possible issues.

@martinwhitaker
Copy link
Collaborator

If you can generate a working VPI module without linking to libvpi, you must have found a better solution to the problem than I did. Care to share how you do it? I'm willing to make changes to make your job easier if it can be done without breaking the existing ABI (we only allow ABI changes on major releases).

@ktbarrett
Copy link
Author

We don't define any system tasks in cocotb, so we never bothered to pass the VPI app to the compiler. We kept a .def file for generating an import library that pointed to the vvp. That worked for us in 10.3.

martinwhitaker added a commit that referenced this issue Nov 24, 2020
This is to support cocotb, who don't use our vpi_user.h and libvpi.a,
instead building their own import library to directly link to vvp.
martinwhitaker added a commit that referenced this issue Nov 24, 2020
This is to support cocotb, who don't use our vpi_user.h and libvpi.a,
instead building their own import library to directly link to vvp.

(cherry picked from commit 159af4d)
@martinwhitaker
Copy link
Collaborator

OK, I've restored the export of the VPI functions from vvp.exe in both the master and v11 branches. I've verified cocotb works in Windows with the head of the master branch.

@ktbarrett
Copy link
Author

@martinwhitaker thank you!

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

No branches or pull requests

3 participants