Invalid counter synthesis

[RustamC]

I've found that Synlig, unlike Yosys¹, generates an incorrect netlist for simple counter if the increment value is an integer literal constant specified in hexadecimal format without optional size constant.

For example,

• out <= out + 'h1 gives wrong result,
• out <= out + 1'h1 or out <= out + 1 give correct results.

I've tested it using Sky130 library. The test example with testbench & syn script: counter.tar.gz (modified version of this example).

Steps to reproduce:

0. Unpack the archive with the test example

tar xzvf counter.tar.gz ./
cd ./counter

1. Original RTL Verilog simulation:

1. Run Icarus Verilog:

iverilog -g2012 -s tb_counter ./counter.sv tb_counter.v -o counter.rtl.out
vvp ./counter.rtl.out
mv dump.vcd rtl.vcd

2. Display results in GTKWave:

gtkwave rtl.vcd

2. Synthesis & simulation

1. Synthesis using synlig:

synlig -s ./run.sc

2. Simulation:

iverilog -g2012 -s tb_counter ./syn/counter.syn.v tb_counter.v ./verilog/primitives.v ./verilog/sky130_fd_sc_hd.v -o counter.syn.out -DFUNCTIONAL -DUNIT_DELAY=#1
mv dump.vcd syn.vcd

3. Results in GTKWave:

gtkwave syn.vcd

4. As you can see, now counter counts in the wrong direction.

Footnotes

1. If you read input counter.sv using Yosys read_verilog -sv command, generated netlist works & simulates correctly. ↩

[jeras]

Might be related to #2618

[kamilrakoczy]

I haven't tried exactly this test case, but it also might be related to: chipsalliance/Surelog#3979

If you want to investigate it further, you can add -debug flag to synlig to dump both surelog UHDM tree and converted yosys AST tree.

Then you can look at uhdmtopModules part of UHDM tree and check vpiSize of this variables.

[RustamC]

@kamilrakoczy, I've added -debug flag to read_systemverilog & got these results:

1. AST before simplification:

AST_CONSTANT <./counter.sv:0.0-0.0> [0x55db6df6f210] bits='1111'(4) range=[3:0] int=15

2. AST after simplification:

AST_CONSTANT <./counter.sv:0.0-0.0> [0x55db6df6f210] bits='1111'(4) basic_prep range=[3:0] int=15

3. Verilog AST before/after simplification:

    module counter(clk, rstn, out);
      input [0:0] clk;
      input [0:0] rstn;
      output reg [3:0] out;
      always @(posedge 1'(clk))
        case (|(!(rstn)))
          1'b 1:
            out <= 0;
          default:
            out <= (out)+(4'b 1111);
        endcase
    endmodule

P.S. Should I run Surelog separately to generate UHDM tree? I haven't found a command to dump UHDM tree like in your example using synlig.

[kamilrakoczy]

Oh, sorry, I forgot that now we generate UHDM tree in-memory and UHDM dump isn't generated by default.

Yes, you need to run Surelog separately to create UHDM file:

surelog -parse -elabuhdm counter.sv

after that you can use uhdm-dump (it is built with surelog binary) to display UHDM tree:

uhdm-dump slpp_all/surelog.uhdm

I confirmed, that Surelog expands unsized const to 4'b1111:

          |vpiElseStmt:
          \_assignment: , line:12:7, endln:12:23
            |vpiParent:
            \_if_else: , line:9:5, endln:12:24
            |vpiOpType:82
            |vpiRhs:
            \_operation: , line:12:14, endln:12:23
              |vpiParent:
              \_assignment: , line:12:7, endln:12:23
              |vpiOpType:24
              |vpiOperand:
              \_ref_obj: ([email protected]), line:12:14, endln:12:17
                |vpiParent:
                \_operation: , line:12:14, endln:12:23
                |vpiName:out
                |vpiFullName:[email protected]
                |vpiActual:
                \_logic_net: ([email protected]), line:3:35, endln:3:38
              |vpiOperand:
              \_constant: , line:12:20, endln:12:23
                |vpiDecompile:15
                |vpiSize:4
                |UINT:15
                |vpiConstType:9

I think, according to standard unsized const should have int size and i op j should have length max(L(i), L(j)).
Nevertheless, SystemVerilog standard isn't very specific and many tools doesn't follow it completely.

If you think it is a bug, please report it to Surelog, as I think it is a place where (if required) it should be fixed.

[RustamC]

@kamilrakoczy , thank you! I will report it to Surelog.