Skip to content

fix: pass the IEEE 1800 finish number to $fatal in cve2_top_tracing - #334

Merged
MikeOpenHWGroup merged 1 commit into
openhwfoundation:mainfrom
avinashkollu-git:fix/321-fatal-finish-number
Jul 17, 2026
Merged

MikeOpenHWGroup merged 1 commit into
openhwfoundation:mainfrom
avinashkollu-git:fix/321-fatal-finish-number

Conversation

@avinashkollu-git

Copy link
Copy Markdown

What

Change the $fatal call in rtl/cve2_top_tracing.sv from $fatal("...") to $fatal(1, "...").

Why

$fatal is an elaboration-time severity system task, and per IEEE 1800 its first argument must be the finish number (0, 1, or 2), not the message. The existing call passes the message string where the finish number is expected, which standards-strict simulators reject with "finish number argument must be numeric" (issue #321). This matches the approach confirmed by the maintainer in the issue discussion.

How tested

Reproduced and verified with Icarus Verilog:

  1. On the original line, elaboration fails: Elaboration task $fatal() finish number argument must be numeric.
  2. After the fix, with RVFI undefined, the elaboration-time fatal fires cleanly and prints the message, so the fail-fast behaviour is preserved.
  3. With +define+RVFI (the normal build), the guard is compiled out entirely, so there is no regression.

Closes #321

$fatal is an elaboration-time severity task whose first argument must be
the finish number (IEEE 1800), not the message. The existing call passed
the message string as that argument, which standards-strict simulators
reject ("finish number argument must be numeric"). Pass 1 so the
elaboration-time fatal is accepted by conformant tools; the message and
the fail-fast behaviour are unchanged, and the call is compiled out
entirely when RVFI is defined (the normal build).

Closes openhwfoundation#321

Signed-off-by: Avinash Kollu <avinashkollu123@gmail.com>
@MikeOpenHWGroup

Copy link
Copy Markdown

Hi @avinashkollu-git, thanks for your interest in the CVE2. A couple of things:

  1. What simulator (and version) are you using?
  2. In order to accept this PR, you must sign the ECA. See CONTRIBUTING for the details.

@avinashkollu-git

Copy link
Copy Markdown
Author

Hi @MikeOpenHWGroup, thanks for the quick review.

Simulator: reproduced and verified with Icarus Verilog 13.0 (stable). On the original line, elaboration fails with Elaboration task $fatal() finish number argument must be numeric. With the fix, $fatal(1, "...") elaborates cleanly (the fatal fires and prints the message when RVFI is undefined), and with +define+RVFI the guard is compiled out, so there is no regression.

ECA: signed just now under the same email as my commit (avinashkollu123@gmail.com).

Thanks!

@MikeOpenHWGroup

Copy link
Copy Markdown

Thanks for signing the ECA.

It is interesting that you are able to use Icarus for this project - it is not a simulator that OpenHW has typically supported, but I am pleased to learn that it can handle the CVE2. Having said that, you are right about the LRM. Also, I have tested this PR against Verilator v5.050 and Questa Sim-64 vsim 2025.3_2, and both produce expected, although not identical, results.

So I will merge this in. Congratulations @avinashkollu-git, you are now an official OpenHW Foundation Contributor.

@MikeOpenHWGroup
MikeOpenHWGroup merged commit 527da10 into openhwfoundation:main Jul 17, 2026
3 checks passed
@avinashkollu-git

Copy link
Copy Markdown
Author

Thanks @MikeOpenHWGroup, and thanks for taking the time to test it on Verilator and Questa before merging.

On the Icarus point: I have only used it for elaboration and lint level checks on CVE2 rather than full simulation, so I would not claim broad support. But Icarus 13.0 does elaborate the RTL cleanly, which made it easy to catch this particular LRM issue. Happy to share details if that is ever useful to the project.

I would like to keep contributing. I have since opened a documentation PR on core-v-mcu (lowRISC#371) covering the APB Timer's use as MTIME. If there are areas of CVE2 or the wider OpenHW projects where help is useful, especially anything verification related, I would be glad to pick something up.

Thanks again for the review.

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.

[BUG] $fatal call in cve2_top_tracing.sv not following latest SystemVerilog IEEE standard (1800-2012)

2 participants