common_cells
common_cells copied to clipboard
Disable complex addr decoder assertions in Verilator
The addr_decode_dync and multiaddr_decode modules feature some complex assertions (actually assumes to be precise) that are quite complex — too complex for Verilator it seems, which fails in unpredictable ways when these modules are present. Therefore, this PR disables those assertions in Verilator (based on whether the VERILATOR pre-processor macro is defined).
@michael-platzer Thanks for reporting this. With what Verilator version did you observe the failure?
@colluca With version 5.018:
Verilator 5.018 2023-10-30 rev v5.018
I have not tested it with the latest version though. Do you happen to use a newer version for which these are handled correctly?
Update: I tested the current code from master with the latest release of Verilator (5.028), which already fails at elaboration of addr_decode_dync.sv:
%Error: /workspaces/hw.riscv/.bender/git/checkouts/common_cells-2220a26021f1ec02/src/addr_decode_dync.sv:149:12: Value too wide for 64-bits expected in this context 224'h8000000000c00000000000000100100000000010010000
149 | always @(addr_map_i or config_ongoing_i) #0 begin : proc_check_addr_map
| ^~~~~~~~~~
%Error: /workspaces/hw.riscv/.bender/git/checkouts/common_cells-2220a26021f1ec02/src/addr_decode_dync.sv:149:12: Value too wide for 64-bits expected in this context 132'h24180000000121000000000008
149 | always @(addr_map_i or config_ongoing_i) #0 begin : proc_check_addr_map
| ^~~~~~~~~~
%Error: Exiting due to 2 error(s)
Maybe this is just Verilator not properly supporting these complex assertions, but it could also be an indication there is something wrong and in need of fixing. @WRoenninger Could you please have a look?
I did some investigation with @WRoenninger and we found that Verilator has issues with wide signals in sensitivity lists. With the latest version (5.028) all issues can be resolved by using always @* instead of a hand-crafted sensitivity list. Most tools are pretty smart when it comes to figuring out the dependencies of always @* blocks, so this should not have performance implications. Also, with a wildcard sensitivity list it is not possible to forget adding signals to the sensitivity list as new signals are added to the assertions later on. I adapted the files accordingly.
Changed always @* to always_comb to make the reviewdog happy :dog: