-
Notifications
You must be signed in to change notification settings - Fork 24
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
Use of SystemVerilog Interface should be strongly discouraged #227
Comments
I strongly disagree with this. Use of SystemVerilog interfaces should actually be encouraged for certain uses (including this one). Removing myself from this issue. |
i think cv-x-if should neither encourage or discourage particular coding styles are choices. |
Hi @Silabs-ArjanB, can you provide a specific example of where/how SV Interfaces can be safely used in RTL? This issue was motivated by a recent discussion within the CVA6 team in which both myself and @Jbalkind expressed concerns over the use of SV Interfaces in RTL. We have both (independently) had specific issues with the EDA toolchain. In my case, a tool used for RTL-to-gates Equivalence Checking failed to properly handle the SV Interfaces. Admittedly that was several years ago. @Jbalkind, can you comment on the issue you had with SV interfaces? I will reiterate that the OpenHW Cores Task Group adopted the lowRISC coding style guidelines which indicate that SystemVerilog Interfaces are problematic constructs and their use is discouraged. |
https://github.com/openhwgroup/cv32e40x/blob/master/rtl/cv32e40x_core.sv
We treat the lowRISC coding style guidelines as exactly that, i.e. guidelines. Mostly these are good guidelines, but we don't take them too literally. They were only adopted because nobody wanted to spend time/effort in writing our own guidelines and/or opening up internal guidelines. |
I agree that the use of the SV interfaces should neither be encouraged nor discouraged and that the lowRISC coding style is a guideline, but not a strict rule. There are many reasons why SV interfaces are the preferred choice, X-IF would be one example (although can be done with structures as well) |
I have to say that I disagree with the suggestion we should be embracing interfaces. When this was discussed in the CVA6 meeting, there were multiple responses from OpenHW members saying that they had designs which were broken by the use of interfaces. One member has a chip which does not function correctly because Genus didn't handle interfaces correctly. We should not be distributing code which has known problems when used with well known tools. I like interfaces and I wish that they were better supported but I do not think we should be including them in our codebases until we are past the point that they are a common concern. If we are giving types using interfaces then we should also be giving associated structs as examples to avoid what has already happened (contributors wasting their time to use interfaces when their use shouldn't be merged). |
This discussion has gone stale and it would be great to come to a concrete resolution. It seems @davideschiavone has the right position here:
Having said that, @Jbalkind makes a good point, so it would be useful to provide specific guidance about using SV interfaces in RTL to avoid problems with gate models sythesized from SV interfaces. @Silabs-ArjanB, can you share whatever synthesis guidelines your team uses to avoid these pitfalls? |
I can't share internal rules and guidelines |
Is there an existing CV-X-IF task for this?
Task Description
This repo hosts an example SystemVerilog Interface of the CV-X-IF in src/core_v_xif.sv. It should be made clear that this is a simulation only example and should not be used in any CORE-V RTL IP.
The CV32E40P coding style guidelines, which are essentially the lowRISC coding style guidelines, indicate that SystemVerilog Interfaces are problematic constructs and their use is discouraged.
Description of Done
core_v_xif.sv is a useful example, so it is proposed that we simply add the following to the comment header of that file:
The text was updated successfully, but these errors were encountered: