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
VPI systemtf #1470
Comments
Original Redmine Comment Note Verilator does support a language extension where arbitrary doller-user functions can instead be converted into DPI functions. This suggests a refinement of what you suggest, namely Verilator would spit out automatically code that converted DPI calls into old-school VPI calls. Another complication is verilator must know the data types of the user's functions. This is usually done by the simulator at verilation time needing to call into the user VPI routines/tables. This will be a pain. Having these VPI extensions would be a fine addition, but I wonder if this is really in the service of users runtime wise? Could cocotb switch to generating DPI functions? This is more modern, far cleaner, and would speed up cocotb not just with Verilator but with every simulator. |
Original Redmine Comment It is actually even worse: It only uses the systemtf to re-direct $display-style system functions to its output. As you say, undergoing the full exercise seems like a massive task just for this minor part of the complexity. But I cannot come up with an acceptable workaround for that so far. I think currently, the existing implementations all just turn the Unimplemented error of registering systemtf into an empty function.. |
Original Redmine Comment How would you fancy an implementation that only allows to overwrite systemtf for a limited set of systemtf (in particular the ones mentioned above)? |
Original Redmine Comment Adding a way to overload $display would be ok but you'd have a larger problem that display takes arguments and the DPI doesn't support vaargs. Plus then you'd need $write, etc, etc. Knowing it's just display helps enormously. I suggest using VPI was just a hack to get around that other simulators didn't have what was really wanted, namely an output-stream override. And Verilator has better alternative paths.
|
Original Redmine Comment Ah, you are absolutely right. Good, that I didn't start trying to implement anything so far :) I think we should be able to solve that on the cocotb side to reduce the complexity. Will have a look into it next week! |
Original Redmine Comment Hi, sorry, took much longer as I hoped to even look into it. Attached please find a first, rather trivial patch, that allows us to define vpi_register_systf in our Verilator-specific cocotb infrastructure and thereby have no impact on the common code. A change there would also not be overly complex, but I would like to get your comment on whether this can be accepted from your point of view. I see no negative implications, and it is probably the only strange workaround (which is great :) Cheers, |
Original Redmine Comment Weak seems somewhat hacky, but I can live with hacky. You do however need to do not break non-GCC compilers e.g. MSVCC. Please use e.g. PLI_DLLWEAK then declare that similar to how PLI_DLLISPEC is made. A search turned up this MSVC++ somewhat equivalent to weak: https://stackoverflow.com/questions/2290587/gcc-style-weak-linking-in-visual-studio however it might be safer just to set it to empty for MSVCC (the conditions similar to those that test to make PLL_DLLESPEC. |
Original Redmine Comment Thanks, Wilson, I was already thinking about the issue with other compilers but only came up with LLVM :) I think it is easier to handle this minor issue on the cocotb side, because it is cleaner there (define out the call for Verilator). Time to get to the actual stuff then, so please keep the issue open, thanks! |
Original Redmine Comment Okay, here is one that is equally hacky and elegant on both sides. Instead of making the symbol weak, how about just not error'ing on unimplemented functions. The application may choose to deliberately not have it error out, then use common code to probe for existence, as per the patch attached. An improvement would be to only do it for ones that allow probing (need to check), but register_systf allows it by returning 0. But ultimately, the application developer is fully aware of this and can track on its own because otherwise they wouldn't have set -DVL_SUPPRESS_UNIMP during the compilation of the runtime. Seems like a simple "compromise" in terms of hackiness on both sides. We can still fallback on the cocotb-side solution which requires larger #ifdef VERILATOR areas in the common code which is considered unfavored (but acceptable). |
Original Redmine Comment Perhaps when suppressed this should be setting the error return code so vpi_chk_error returns something sane e.g. level=vpiInternal message=Unsupported...? |
Original Redmine Comment Wilson Snyder wrote:
Awesome, thanks! Always new sections in the LRM to learn about :) Will get onto that over the weekend! |
Original Redmine Comment The first part is much easier than I thought, because @Verilated::fatalOnVpiError(false);@ does the job. Now I need to work on defining output, so please keep it open, thanks! |
@wallento is this issue still appropriate for cocotb or can we punt this until the nebulous "future"? Thanks. |
It is not an urgent requirement for cocotb, but useful in future as you suggest (I believe it will happen in 2020). |
The use case for this was resolved earlier. We'll wait for a new request for the more general VPI, but really should encourage DPI wherever possible as is a faster interface. |
Author Name: Stefan Wallentowitz (@wallento)
Original Redmine Issue: 1470 from https://www.veripool.org
Original Assignee: Stefan Wallentowitz (@wallento)
Hi,
VPI allows to add and overwrite System Tasks and Functions. I propose to add the infrastructure to allow for that in the following way:
Enabling this feature has a negative impact on performance, that's why it should definitely be optional.
I gave a very rough estimate in hours, happy to pick it up once I find time.
Best,
Stefan
The text was updated successfully, but these errors were encountered: