ibex icon indicating copy to clipboard operation
ibex copied to clipboard

Fix Verilator 5.010 compilation error

Open leesum1 opened this issue 2 years ago • 6 comments

❯ 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

leesum1 avatar May 30 '23 16:05 leesum1

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.

rswarbrick avatar Jun 12 '23 09:06 rswarbrick

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

leesum1 avatar Jun 14 '23 07:06 leesum1

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!

rswarbrick avatar Jun 14 '23 14:06 rswarbrick

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.

leesum1 avatar Jun 19 '23 14:06 leesum1

I have updated this commit. 👋

leesum1 avatar Jun 29 '23 03:06 leesum1

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

leesum1 avatar Jun 29 '23 04:06 leesum1