Cannot synthesis wb_common from FuseSoC

[Icenowy]

Steps to reproduce the issue

Let Yosys run read_verilog on the wb_common verilog file from wb_common. https://github.com/fusesoc/wb_common/blob/master/wb_common.v

Expected behavior

The file should be successfully processed.

Actual behavior

wb_common.v:36: Warning: System task `$display' outside initial block is unsupported.
wb_common.v:64: ERROR: Failed to evaluate system function `\$clog2' with non-constant value.

If define BROKEN_CLOG2, then there will be another error:

wb_common.v:36: Warning: System task `$display' outside initial block is unsupported.
wb_common.v:46: ERROR: 2nd expression of generate for-loop is not constant!

[ZipCPU]

The problem involved stems from the fact that $clog2 only works with compile time constants. These can be generated with literals, such as 25 or 32'hdead_beef, or from parameters. Further, if the compile time constant comes from a parameter, that parameter needs to have a default definition in order to avoid this error.

We would like to implement this feature in the future since other tools seem to be able to handle this construct. The 2002 Verilog synthesis standard, however, specifically states that system function calls are not supported for synthesis.

[cliffordwolf]

From the Verilog Synthesis standard (IEEE Std 1364.1):

Yosys, however, does support $clog2 (and a few other system functions), but only on simple compile time constants.

[ZipCPU]

@olofk -- please see above

[olofk]

As for this case, I'm pretty convinced no one has ever used it with something other than 32-bit buses. Not even sure the other stuff in wb_common works with anything else, but I'll do a new release where I rewrite the clog2 stuff to if (dw == 64) shift = 3; else if (dw == 32) shift = 2; else if (dw == 16) shift = 1; else shift = 0;

that should be enough to avoid this problem, right?

[olofk]

Released version v1.0.3 with this fixed

[udif]

1. If you thing only 32 bits are used then simply support only the 32bit case, and add an assertion to fail the other cases. Do you even support runtime switching of the bus size? If not, then this should have been a parameter in the first place.
2. @ZipCPU , if you check the verilog BNF, you will see that verilog parameters must have a default value, at least for the ANSI type module declaration.

[ZipCPU]

@udif : Thank you so much for that comment! I've been dealing with a couple pieces of (somebody else's) code that haven't had defaults for their parameters. Now I can report back to them that these reflect actual bugs in their designs.

@cliffordwolf : I think we can close this issue now.