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

Use of SystemVerilog Interface should be strongly discouraged #227

Open
1 task done
MikeOpenHWGroup opened this issue Jun 12, 2024 · 8 comments
Open
1 task done
Assignees
Labels
documentation Improvements or additions to documentation enhancement New feature or request good first issue Good for newcomers

Comments

@MikeOpenHWGroup
Copy link
Member

Is there an existing CV-X-IF task for this?

  • I have searched the existing task issues

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:

// EXAMPLE ONLY
// The lowRISC coding style guidelines (https://github.com/lowRISC/style-guides/blob/master/VerilogCodingStyle.md)
// indicate that SystemVerilog Interfaces are problematic constructs and their use in CORE-V RTL IP is strongly discouraged.
@MikeOpenHWGroup MikeOpenHWGroup added documentation Improvements or additions to documentation enhancement New feature or request good first issue Good for newcomers labels Jun 12, 2024
@Silabs-ArjanB
Copy link
Contributor

I strongly disagree with this. Use of SystemVerilog interfaces should actually be encouraged for certain uses (including this one). Removing myself from this issue.

@Silabs-ArjanB Silabs-ArjanB removed their assignment Jun 13, 2024
@christian-herber-nxp
Copy link
Contributor

i think cv-x-if should neither encourage or discourage particular coding styles are choices.

@MikeOpenHWGroup
Copy link
Member Author

MikeOpenHWGroup commented Jun 13, 2024

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.

@Silabs-ArjanB
Copy link
Contributor

Hi @MikeOpenHWGroup

can you provide a specific example of where/how SV Interfaces can be safely used in RTL?

https://github.com/openhwgroup/cv32e40x/blob/master/rtl/cv32e40x_core.sv

I will reiterate that the OpenHW Cores Task Group adopted the [lowRISC coding style guidelines]

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.

@davideschiavone
Copy link
Contributor

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)

@Jbalkind
Copy link

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).

@MikeOpenHWGroup
Copy link
Member Author

This discussion has gone stale and it would be great to come to a concrete resolution. It seems @davideschiavone has the right position here:

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.

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?

@Silabs-ArjanB
Copy link
Contributor

@Silabs-ArjanB, can you share whatever synthesis guidelines your team uses to avoid these pitfalls?

I can't share internal rules and guidelines

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation enhancement New feature or request good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

7 participants