Yosys warning / openlane error on out-of-bound range select in serv_rf_ram.v

[chaufe]

Yosys reports a warning on a out-of-bound range select in serv_rf_ram.v.

/content/serv-1.2.1/rtl/serv_rf_ram.v:34: Warning: Range [5:-1] select out of bounds on signal `\i_raddr': Setting 5 LSB bits to undef.

Openlane checkes for that warning and converts it into an error.

[ERROR]: Synthesis failed. Range select out of bounds on some signals. Search for 'out of bounds on signal' in /content/runs/RUN_2023.05.25_06.25.13/logs/synthesis/1-synthesis.log

(Other synthesis solutions also error out on this out-of-bound range select.)

The error is caused by the parameter "width" that defaults to "0", thereby creating a div-by-zero when computing the default parameter "depth", as well as creating the mentioned out-of-bound range select.

module serv_rf_ram
  #(parameter width=0,
    parameter csr_regs=4,
    parameter depth=32*(32+csr_regs)/width)
[...]
   always @(posedge i_clk)
     regzero <= !(|i_raddr[$clog2(depth)-1:5-$clog2(width)]);
[...]

A simple fix would be changing the default of "width" to "2".

\cc @dhaentz1

[olofk]

My take on this is that it is quite useful (and common) to have illegal default values in cases that don't really have a sensible default, since Verilog requires a default value as opposed to VHDL where leaving a generic unassigned is a sign that the user needs to set this.

I don't agree with Yosys default behaviours here, but I also know it won't be fixed. (I filed a bug report about this about seven years ago). However, there is a way around this by setting verilog_defaults -add -defer. Edalize does this for all yosys backends and SiliconCompiler does something similar too. OpenLANE doesn't unfortunately and I haven't found a sensible way to add your own tcl hooks to the process.

So, ideally, I would have this fixed in either Yosys or OpenLANE, but I don't really expect anything to happen there so until there is a proper Edalize backend for this flow that can replace OpenLANE, I think the most realistic approach is to add a valid default value as you propose.

(I still don't like it though! 😠)