axi_adapter: fix compilation with verilator

[sergachev]

Verilator always compiles both parts of code with EXPAND = 0 and 1 - but some parameters were only valid for one of them (because only one of them is actually executed after compilation). This is now fixed by setting safe values of these parameters for the non-active part of the code.

[alexforencich]

What version of Verilator are you using? I know some of these issues were addressed in verilator at some point.

[sergachev]

What version of Verilator are you using? I know some of these issues were addressed in verilator at some point.

Latest: 4.225 devel rev v4.224-82-gcbe1b8e2

[alexforencich]

Interesting, I guess they haven't fixed all of the issues yet. Specifically which errors are you getting from verilator, and is there a verilator issue relating to this?

[sergachev]

I would not call this an issue of verilator, it just refuses to compile code with invalid parameters like zero or negative replication values.

Depending on the EXPAND value - either:

%Warning-SELRANGE: verilog_axi/verilog/rtl/axi_adapter_rd.v:339:83: [0:2] Range extract has backward bit ordering, perhaps you wanted [2:0]
                                                                                                                                          : ... In instance sim.axi_adapter_v.axi_adapter_rd_inst
  339 |                             m_axi_arlen_next = ({1'b0, s_axi_arlen} + s_axi_araddr[M_ADDR_BIT_OFFSET-1:S_ADDR_BIT_OFFSET]) >> $clog2(SEGMENT_COUNT);
      |                                                                                   ^
                   ... For warning description see https://verilator.org/warn/SELRANGE?v=4.225
                   ... Use "/* verilator lint_off SELRANGE */" and lint_on around source to disable this message.
%Warning-SELRANGE: verilog_axi/verilog/rtl/axi_adapter_rd.v:368:63: [0:2] Range extract has backward bit ordering, perhaps you wanted [2:0]
                                                                                                                                          : ... In instance sim.axi_adapter_v.axi_adapter_rd_inst
  368 |                     s_axi_rdata_int = m_axi_rdata >> (addr_reg[M_ADDR_BIT_OFFSET-1:S_ADDR_BIT_OFFSET] * S_DATA_WIDTH);
      |                                                               ^
%Warning-SELRANGE: verilog_axi/verilog/rtl/axi_adapter_rd.v:393:63: [0:2] Range extract has backward bit ordering, perhaps you wanted [2:0]
                                                                                                                                          : ... In instance sim.axi_adapter_v.axi_adapter_rd_inst
  393 |                     s_axi_rdata_int = m_axi_rdata >> (addr_reg[M_ADDR_BIT_OFFSET-1:S_ADDR_BIT_OFFSET] * S_DATA_WIDTH);
      |                                                               ^
%Warning-SELRANGE: verilog_axi/verilog/rtl/axi_adapter_rd.v:420:60: [0:2] Range extract has backward bit ordering, perhaps you wanted [2:0]
                                                                                                                                          : ... In instance sim.axi_adapter_v.axi_adapter_rd_inst
  420 |                     s_axi_rdata_int = data_reg >> (addr_reg[M_ADDR_BIT_OFFSET-1:S_ADDR_BIT_OFFSET] * S_DATA_WIDTH);
      |                                                            ^
%Error: verilog_axi/verilog/rtl/axi_adapter_wr.v:358:55: Replication value of 0 is only legal under a concatenation (IEEE 1800-2017 11.4.12.1)
                                                                                                                               : ... In instance sim.axi_adapter_v.axi_adapter_wr_inst
  358 |         m_axi_wdata_int = {(M_WORD_WIDTH/S_WORD_WIDTH){s_axi_wdata}};
      |                                                       ^
%Error: Exiting due to too many errors encountered; --error-limit=1

or:

%Warning-SELRANGE: verilog_axi/verilog/rtl/axi_adapter_rd.v:502:39: [1:3] Range extract has backward bit ordering, perhaps you wanted [3:1]
                                                                                                                                          : ... In instance sim.axi_adapter_v.axi_adapter_rd_inst
  502 |                     data_next[addr_reg[S_ADDR_BIT_OFFSET-1:M_ADDR_BIT_OFFSET]*SEGMENT_DATA_WIDTH +: SEGMENT_DATA_WIDTH] = m_axi_rdata;
      |                                       ^
                   ... For warning description see https://verilator.org/warn/SELRANGE?v=4.225
                   ... Use "/* verilator lint_off SELRANGE */" and lint_on around source to disable this message.
%Warning-SELRANGE: verilog_axi/verilog/rtl/axi_adapter_wr.v:551:63: [1:3] Range extract has backward bit ordering, perhaps you wanted [3:1]
                                                                                                                                          : ... In instance sim.axi_adapter_v.axi_adapter_wr_inst
  551 |                     m_axi_wdata_int = s_axi_wdata >> (addr_reg[S_ADDR_BIT_OFFSET-1:M_ADDR_BIT_OFFSET] * M_DATA_WIDTH);
      |                                                               ^
%Warning-SELRANGE: verilog_axi/verilog/rtl/axi_adapter_wr.v:552:63: [1:3] Range extract has backward bit ordering, perhaps you wanted [3:1]
                                                                                                                                          : ... In instance sim.axi_adapter_v.axi_adapter_wr_inst
  552 |                     m_axi_wstrb_int = s_axi_wstrb >> (addr_reg[S_ADDR_BIT_OFFSET-1:M_ADDR_BIT_OFFSET] * M_STRB_WIDTH);
      |                                                               ^
%Warning-SELRANGE: verilog_axi/verilog/rtl/axi_adapter_wr.v:579:60: [1:3] Range extract has backward bit ordering, perhaps you wanted [3:1]
                                                                                                                                          : ... In instance sim.axi_adapter_v.axi_adapter_wr_inst
  579 |                     m_axi_wdata_int = data_reg >> (addr_reg[S_ADDR_BIT_OFFSET-1:M_ADDR_BIT_OFFSET] * M_DATA_WIDTH);
      |                                                            ^
%Warning-SELRANGE: verilog_axi/verilog/rtl/axi_adapter_wr.v:580:60: [1:3] Range extract has backward bit ordering, perhaps you wanted [3:1]
                                                                                                                                          : ... In instance sim.axi_adapter_v.axi_adapter_wr_inst
  580 |                     m_axi_wstrb_int = strb_reg >> (addr_reg[S_ADDR_BIT_OFFSET-1:M_ADDR_BIT_OFFSET] * M_STRB_WIDTH);
      |                                                            ^
%Warning-WIDTHCONCAT: verilog_axi/verilog/rtl/axi_adapter_rd.v:468:69: More than a 8k bit replication is probably wrong: 4294967295
                                                                                                                                             : ... In instance sim.axi_adapter_v.axi_adapter_rd_inst
  468 |                         if ({s_axi_arlen, {S_BURST_SIZE-M_BURST_SIZE{1'b1}}} >> (S_BURST_SIZE-s_axi_arsize) > 255) begin
      |                                                                     ^
Segmentation fault

[alexforencich]

It is a verilator bug, it should not report any errors/warnings for anything other than syntax errors in code that's not reachable by parameter value. No other tool that I have used so far works in this way (icarus verilog, quartus, ISE, and vivado), so this is an issue specific to verilator's highly pedantic linter.

This sort of thing has previously been reported and adjustments made, for example https://github.com/verilator/verilator/issues/2625 . It looks like the "Replication value of 0" error should also be suppressed.

Anyway, I can't do anything with Verilator personally until https://github.com/verilator/verilator/issues/2778 is fixed, as this prevents verilator from working with cocotb.

[alexforencich]

Also, I'm not necessarily opposed to making some changes of this nature, particularly if it helps to make the code more organized/readable/understandable etc. But the issue is that while this particular module isn't too bad, the broken-tool-workaround hacks necessary for some other modules would be a complete nightmare. So the correct solution is to fix the tool.

[sergachev]

I see. I'll try to fix this behavior in verilator some day and will propose @enjoy-digital to use a fork of verilog-axi for https://github.com/enjoy-digital/litex_verilog_axi_test in the meanwhile then.