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

Fix Verilator 5.010 compilation error #2042

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

Conversation

leesum1
Copy link

@leesum1 leesum1 commented May 30, 2023

❯ verilator --version
Verilator 5.010 2023-04-30 rev UNKNOWN.REV

error log

ERROR: %Warning-MULTIDRIVEN: ../src/lowrisc_ibex_ibex_tracer_0.1/rtl/ibex_tracer.sv:762:5: Variable written to in always_comb also written by other process (IEEE 1800-2017 9.2.2.2): 'decoded_str'
                                                                                  : ... In instance ibex_simple_system.u_top.u_ibex_tracer
                      ../src/lowrisc_ibex_ibex_tracer_0.1/rtl/ibex_tracer.sv:762:5: 
  762 |     decoded_str = "";
      |     ^~~~~~~~~~~
                      ../src/lowrisc_ibex_ibex_tracer_0.1/rtl/ibex_tracer.sv:735:5: ... Location of other write
  735 |     decoded_str = $sformatf("fence\t%s,%s", predecessor, successor);
      |     ^~~~~~~~~~~
                      ... For warning description see https://verilator.org/warn/MULTIDRIVEN?v=5.010
                      ... Use "/* verilator lint_off MULTIDRIVEN */" and lint_on around source to disable this message.
%Warning-MULTIDRIVEN: ../src/lowrisc_ibex_ibex_tracer_0.1/rtl/ibex_tracer.sv:763:5: Variable written to in always_comb also written by other process (IEEE 1800-2017 9.2.2.2): 'data_accessed'
                                                                                  : ... In instance ibex_simple_system.u_top.u_ibex_tracer
                      ../src/lowrisc_ibex_ibex_tracer_0.1/rtl/ibex_tracer.sv:763:5: 
  763 |     data_accessed = 5'h0;
      |     ^~~~~~~~~~~~~
                      ../src/lowrisc_ibex_ibex_tracer_0.1/rtl/ibex_tracer.sv:702:7: ... Location of other write
  702 |       data_accessed = RS1 | RS2 | MEM;
      |       ^~~~~~~~~~~~~
%Warning-BLKSEQ: ../src/lowrisc_ibex_ibex_tracer_0.1/rtl/ibex_tracer.sv:119:19: Blocking assignment '=' in sequential logic process
                                                                              : ... In instance ibex_simple_system
                                                                              : ... Suggest using delayed assignment '<='
  119 |       file_handle = $fopen(file_name, "w");
      |                   ^
%Error: Exiting due to 3 warning(s)
make[1]: *** [Makefile:16:Vibex_simple_system.mk] 错误 1

ERROR: Failed to build lowrisc:ibex:ibex_simple_system:0 : '['make', '-j', '12']' exited with an error: 2

Verilator was throwing a compilation error due to
MULTIDRIVEN and BLKSEQ in ibex_tracer.sv

This change ensures successful compilation with Verilator 5.010

@rswarbrick
Copy link
Contributor

Hi there! Sorry for the slow response. I'm a bit confused because I don't see these errors when I build locally. When I start with a clean repository, I see something like this:

$ fusesoc --cores-root=. run --target=sim --setup --build lowrisc:ibex:ibex_simple_system --RV32E=0 --RV32M=ibex_pkg::RV32MFast
... <snip!> ...
INFO: Building simulation model
$ # It worked!
$ verilator --version
Verilator 5.006 2023-01-22 rev (Debian 5.006-3)

Maybe this means I'm doing something different from you? Can you share the way you're running Verilator that gets these errors? Many thanks.

@leesum1
Copy link
Author

leesum1 commented Jun 14, 2023

Hi there! Sorry for the slow response. I'm a bit confused because I don't see these errors when I build locally. When I start with a clean repository, I see something like this:

$ fusesoc --cores-root=. run --target=sim --setup --build lowrisc:ibex:ibex_simple_system --RV32E=0 --RV32M=ibex_pkg::RV32MFast
... <snip!> ...
INFO: Building simulation model
$ # It worked!
$ verilator --version
Verilator 5.006 2023-01-22 rev (Debian 5.006-3)

Maybe this means I'm doing something different from you? Can you share the way you're running Verilator that gets these errors? Many thanks.

I have followed the instructions provided in the "Ibex Simple System README" to build the system. Below are the versions of Python and Verilator that I am using:

❯ python --version
Python 3.10.11
❯ verilator --version
Verilator 5.010 2023-04-30 rev UNKNOWN.REV

When I executed the command:

fusesoc --cores-root=. run --target=sim --setup --build lowrisc:ibex:ibex_simple_system --RV32E=0 --RV32M=ibex_pkg::RV32MFast

or

make build-simple-system

I encountered a compilation error in Verilator. The error was specifically related to the usage of the keywords "MULTIDRIVEN" and "BLKSEQ" in the file "ibex_tracer.sv".

And if I change to Verilator 5.006, everything will be fine.

❯ verilator --version
Verilator 5.006 2023-01-22 rev v5.006

rswarbrick added a commit to rswarbrick/ibex that referenced this pull request Jun 14, 2023
This is done properly in that PR: see the comments there for extra
info (and don't merge this!)
@rswarbrick
Copy link
Contributor

Ahh! Sorry for the stupidity: I had misread the version number in your original report.

The error about decoded_str is a bit weird. As far as I can tell, Verilator is wrongly telling us that we're writing to decoded_str in multiple processes and one of them is an always_comb. This assertion isn't right: all the writes to the variable come about from the same always_comb block, but Verilator can't convince itself of this. I think that adding a "yeah yeah, don't worry about it" comment like you've done here is completely reasonable.

For those bits, I'm very happy with this change, but please could you also add a comment that says something the following? "Tell Verilator not to worry about writing to this variable. It thinks that we're doing so from several processes, contradicting IEEE 1800-2017 9.2.2.2, because it can't see that the other sites are only called from this process".

The other bit (where Verilator warns about a blocking assignment in a sequential logic process) is sort of similar: we happen to know that we're only writing and reading the variable in this process. I think we can write things in a way that Verilator will understand, though, which might be nicer than waiving the warning. Could you change the logic so that we only update file_handle with a non-blocking assignment, but use a local version (with the blocking assignment) in the meantime? I did a minimal tweak to check this worked locally, and did it by renaming the long-lived variable to xfile_handle. For the result, see the pr-2042-tweaked branch on my copy (here: https://github.com/rswarbrick/ibex/tree/pr-2042-tweaked).

Would you mind updating this PR correspondingly?

Many thanks!

@leesum1
Copy link
Author

leesum1 commented Jun 19, 2023

These days, I have identified the commit that caused the errors in Verilator. However, I am not familiar enough with Verilator's code to fix it. Additionally there is a similar issue

For now, I have decided to give up on fixing Verilator's code. Instead, I will update the PR correspondingly in this week.

Verilator was throwing a compilation error due to
MULTIDRIVEN and BLKSEQ in ibex_tracer.sv

This change ensures successful compilation with Verilator 5.010
@leesum1
Copy link
Author

leesum1 commented Jun 29, 2023

I have updated this commit. 👋

@leesum1
Copy link
Author

leesum1 commented Jun 29, 2023

Your suggestion makes verilator 5.010 happy,but makes the older version verilator (verilator4.228) sad. And can't pass Ibex CI.

Maybe the best sulution is just add a lint_off to verilator.

ERROR: %Error-BLKANDNBLK: ../src/lowrisc_ibex_ibex_tracer_0.1/rtl/ibex_tracer.sv:84:16: Unsupported: Blocked and non-blocking assignments to same variable: 'u_top.u_ibex_tracer.xfile_handle'
                                                                               : ... In instance ibex_simple_system
   84 |   reg [31:0]   xfile_handle;
      |                ^~~~~~~~~~~~
                   ../src/lowrisc_ibex_ibex_tracer_0.1/rtl/ibex_tracer.sv:754:15: ... Location of blocking assignment
  754 |       $fclose(xfile_handle);
      |               ^~~~~~~~~~~~
                   ../src/lowrisc_ibex_ibex_tracer_0.1/rtl/ibex_tracer.sv:125:7: ... Location of nonblocking assignment
  125 |       xfile_handle <= file_handle;
      |       ^~~~~~~~~~~~
                   ... For error description see https://verilator.org/warn/BLKANDNBLK?v=4.228
%Error: Exiting due to 1 error(s)
        ... See the manual at https://verilator.org/verilator_doc.html for more assistance.
make[1]: *** [Makefile:16:Vibex_simple_system.mk] 错误 1

ERROR: Failed to build lowrisc:ibex:ibex_simple_system:0 : '['make', '-j', '12']' exited with an error: 2

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.

2 participants