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

Improve Verilator compatibility #964

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

StMiky
Copy link

@StMiky StMiky commented Oct 5, 2023

Improve Verilator compatibility

This pull request applies some changes to the RTL to avoid compilation errors with Verilator. In particular:

  • Use explicit comparison in test statements on parameters (e.g., if (DEBUG != 0) instead of if (DEBUG))
  • Use unpacked array definition when both concurrent and continuous assignments are performed on the same signal
  • Use downward ranges in signal definitions (e.g., logic [N-1] instead of logic [N]

Component

RTL:

  • rtl/cv32e40x_core.sv
  • rtl/cv32e40x_cs_registers.sv
  • rtl/cv32e40x_debug_triggers.sv
  • rtl/cv32e40x_pma.sv

Revision

The fixes are currently applied to revision f17028f (v0.9.0).

Limitations

This PR only solves a subset of suppressible Verilator warnings (e.g., through waiver files). In particular, the following waivers are known to be necessary to compile with no warnings:

lint_off -rule PINCONNECTEMPTY -file "*/carus_cpu_subsystem.sv" -match "Cell pin connected by name with empty reference: '*'"
lint_off -rule PINCONNECTEMPTY -file "*/cv32e40x_if_stage.sv" -match "Cell pin connected by name with empty reference: '*'"
lint_off -rule PINCONNECTEMPTY -file "*/cv32e40x_load_store_unit.sv" -match "Cell pin connected by name with empty reference: 'core_align_err_o'"
lint_off -rule DECLFILENAME -file "*/cv32e40x_sim_clock_gate.sv" -match "Filename '*' does not match MODULE name: '*'"
lint_off -rule UNUSED -file "*/cv32e40x_*.sv" -match "Parameter is not used: '*'"
lint_off -rule UNUSED -file "*/if_xif*.sv" -match "Parameter is not used: '*'"
lint_off -rule UNUSED -file "*/if_xif.sv" -match "Signal is not used: 'compressed_*'"
lint_off -rule UNUSED -file "*/if_xif.sv" -match "Signal is not used: 'mem_*'"
lint_off -rule UNUSED -file "*/cv32e40x_*.sv" -match "Bits of signal are not*used: *"
lint_off -rule UNUSED -file "*/cv32e40x_*.sv" -match "Signal is not*used: *"
lint_off -rule WIDTH -file "*/cv32e40x_*.sv" -match "Operator * expects *"
lint_off -rule LITENDIAN -file "*/cv32e40x_*.sv" -match "Little bit endian vector: left < right of bit range: *"
lint_off -rule UNOPTFLAT -file "*/cv32e40x_*.sv" -match "Signal unoptimizable: *"
lint_off -rule UNOPTFLAT -file "*/if_xif*.sv" -match "Signal unoptimizable: *"
lint_off -rule BLKANDNBLK -file "*/cv32e40x_csr.sv" -match "Unsupported: Blocked and non-blocking assignments to same variable: *"
lint_off -rule UNDRIVEN -file "*/cv32e40x_align_check.sv" -match "Bits of signal are not driven: 'core_resp_o'[2]"
lint_off -rule COMBDLY -file "*/cv32e40x_sim_clock_gate.sv"

Status

The included changes are currently applied to the cv32e40x core in X-HEEP . No bugs related to these changes are currently known.

@silabs-oysteink
Copy link
Contributor

Just running this quickly through our formal flow gives error on line 1817 in the figure:
image

Error message is "illegal operand for operator &" and "illegal operand for operator |". Have you run this with performance counters configured?

@Silabs-ArjanB
Copy link
Contributor

Hi @StMiky Thank you for your PR! I see you are not covered by a signed ECA. See above failing check about "Author did not have a signed ECA on file." Could you please address that?

- Explicit comparison in tests on parameters (e.g., `if (DEBUG != 0)`
- Use unpacked signals when both concurrent and continuous assignments are required
- Use downwards ranges in signals definition
@davideschiavone
Copy link
Contributor

Hi @StMiky Thank you for your PR! I see you are not covered by a signed ECA. See above failing check about "Author did not have a signed ECA on file." Could you please address that?

btw just for the context, @StMiky is part of Politecnico di Torino, which is a member of OpenHW Group

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

Successfully merging this pull request may close these issues.

4 participants