zachjs / sv2v

SystemVerilog to Verilog conversion
BSD 3-Clause "New" or "Revised" License
497 stars 50 forks source link

Mutidimensional Packed Arrays Handler Does Not Match Commercial Synthesis Tools #271

Open sifferman opened 5 months ago

sifferman commented 5 months ago

Hello!

When sv2v processes multidimensional arrays, the output differs compared to passing the SystemVerilog directly to commercial tools.

Original SystemVerilog

logic [1:0][1:0] mem [2][2];

Original SystemVerilog passed into sv2v

reg [3:0] mem [0:1][0:1];

Original SystemVerilog passed into Genus

wire [1:0] \mem[1][1][0] ;
wire [1:0] \mem[0][1][0] ;
wire [1:0] \mem[0][0][1] ;
wire [1:0] \mem[1][0][0] ;
wire [1:0] \mem[1][0][1] ;
wire [1:0] \mem[0][0][0] ;
wire [1:0] \mem[1][1][1] ;
wire [1:0] \mem[0][1][1] ;

Issues

  1. sv2v causes synthesis to no longer match commercial tool behavior.
  2. sv2v puts faith in the synthesis tool optimizers to efficiently handle multiplications of array indices, i.e. mem[2*i+:2].

Bigger Example

(Source code for provided examples: sv2v_array_example.tar.gz).

This is a summary of how sv2v, Genus, Vivado, and DC handle multidimensional arrays:

logic [1:0] mem; sv2v ```verilog reg [1:0] mem; ``` Genus ```verilog wire [1:0] mem; ``` Vivado ```verilog wire \mem[0]_i_1_n_0 ; wire \mem[1]_i_1_n_0 ; ``` DC ```verilog wire \mem\[1\] , \mem\[0\]; ```
logic [1:0][1:0] mem; sv2v ```verilog reg [3:0] mem; ``` Genus ```verilog wire [1:0] \mem[1] ; wire [1:0] \mem[0] ; ``` Vivado ```verilog wire \mem[0][0]_i_1_n_0 ; wire \mem[0][1]_i_1_n_0 ; wire \mem[1][0]_i_1_n_0 ; wire \mem[1][1]_i_1_n_0 ; ``` DC ```verilog wire \mem\[1\]\[1\] , \mem\[1\]\[0\] , \mem\[0\]\[1\] , \mem\[0\]\[0\]; ```
logic [1:0] mem [2]; sv2v ```verilog reg [1:0] mem [0:1]; ``` Genus ```verilog wire [1:0] \mem[1] ; wire [1:0] \mem[0] ; ``` Vivado ```verilog wire \mem[0][0]_i_1_n_0 ; wire \mem[0][1]_i_1_n_0 ; wire \mem[1][0]_i_1_n_0 ; wire \mem[1][1]_i_1_n_0 ; ``` DC ```verilog wire \mem\[0\]\[1\] , \mem\[0\]\[0\] , \mem\[1\]\[1\] , \mem\[1\]\[0\]; ```
logic [1:0][1:0] mem [2]; sv2v ```verilog reg [3:0] mem [0:1]; ``` Genus ```verilog wire [1:0] \mem[0][0] ; wire [1:0] \mem[0][1] ; wire [1:0] \mem[1][0] ; wire [1:0] \mem[1][1] ; ``` Vivado ```verilog wire \mem[0][0][0]_i_1_n_0 ; wire \mem[0][0][1]_i_1_n_0 ; wire \mem[0][1][0]_i_1_n_0 ; wire \mem[0][1][1]_i_1_n_0 ; wire \mem[1][0][0]_i_1_n_0 ; wire \mem[1][0][1]_i_1_n_0 ; wire \mem[1][1][0]_i_1_n_0 ; wire \mem[1][1][1]_i_1_n_0 ; ``` DC ```verilog wire \mem\[0\]\[1\]\[1\] , \mem\[0\]\[1\]\[0\] , \mem\[0\]\[0\]\[1\] , \mem\[0\]\[0\]\[0\] , \mem\[1\]\[1\]\[1\] , \mem\[1\]\[1\]\[0\] , \mem\[1\]\[0\]\[1\] , \mem\[1\]\[0\]\[0\]; ```
logic [1:0] mem [2][2]; sv2v ```verilog reg [1:0] mem [0:1][0:1]; ``` Genus ```verilog wire [1:0] \mem[0][0] ; wire [1:0] \mem[0][1] ; wire [1:0] \mem[1][0] ; wire [1:0] \mem[1][1] ; ``` Vivado ```verilog wire \mem[0][0][0]_i_1_n_0 ; wire \mem[0][0][1]_i_1_n_0 ; wire \mem[0][1][0]_i_1_n_0 ; wire \mem[0][1][1]_i_1_n_0 ; wire \mem[1][0][0]_i_1_n_0 ; wire \mem[1][0][1]_i_1_n_0 ; wire \mem[1][1][0]_i_1_n_0 ; wire \mem[1][1][1]_i_1_n_0 ; ``` DC ```verilog wire \mem\[0\]\[0\]\[1\] , \mem\[0\]\[0\]\[0\] , \mem\[0\]\[1\]\[1\] , \mem\[0\]\[1\]\[0\] , \mem\[1\]\[0\]\[1\] , \mem\[1\]\[0\]\[0\] , \mem\[1\]\[1\]\[1\] , \mem\[1\]\[1\]\[0\]; ```
logic [1:0][1:0] mem [2][2]; sv2v ```verilog reg [3:0] mem [0:1][0:1]; ``` Genus ```verilog wire [1:0] \mem[1][1][0] ; wire [1:0] \mem[0][1][0] ; wire [1:0] \mem[0][0][1] ; wire [1:0] \mem[1][0][0] ; wire [1:0] \mem[1][0][1] ; wire [1:0] \mem[0][0][0] ; wire [1:0] \mem[1][1][1] ; wire [1:0] \mem[0][1][1] ; ``` Vivado ```verilog wire \mem[0][0][0][0]_i_1_n_0 ; wire \mem[0][0][0][1]_i_1_n_0 ; wire \mem[0][0][1][0]_i_1_n_0 ; wire \mem[0][0][1][1]_i_1_n_0 ; wire \mem[0][1][0][0]_i_1_n_0 ; wire \mem[0][1][0][1]_i_1_n_0 ; wire \mem[0][1][1][0]_i_1_n_0 ; wire \mem[0][1][1][1]_i_1_n_0 ; wire \mem[1][0][0][0]_i_1_n_0 ; wire \mem[1][0][0][0]_i_2_n_0 ; wire \mem[1][0][0][1]_i_1_n_0 ; wire \mem[1][0][0][1]_i_2_n_0 ; wire \mem[1][0][1][0]_i_1_n_0 ; wire \mem[1][0][1][0]_i_2_n_0 ; wire \mem[1][0][1][1]_i_1_n_0 ; wire \mem[1][0][1][1]_i_2_n_0 ; wire \mem[1][1][0][0]_i_1_n_0 ; wire \mem[1][1][0][1]_i_1_n_0 ; wire \mem[1][1][1][0]_i_1_n_0 ; wire \mem[1][1][1][1]_i_1_n_0 ; ``` DC ```verilog wire \mem\[0\]\[0\]\[1\]\[1\] , \mem\[0\]\[0\]\[1\]\[0\] , \mem\[0\]\[0\]\[0\]\[1\] , \mem\[0\]\[0\]\[0\]\[0\] , \mem\[0\]\[1\]\[1\]\[1\] , \mem\[0\]\[1\]\[1\]\[0\] , \mem\[0\]\[1\]\[0\]\[1\] , \mem\[0\]\[1\]\[0\]\[0\] , \mem\[1\]\[0\]\[1\]\[1\] , \mem\[1\]\[0\]\[1\]\[0\] , \mem\[1\]\[0\]\[0\]\[1\] , \mem\[1\]\[0\]\[0\]\[0\] , \mem\[1\]\[1\]\[1\]\[1\] , \mem\[1\]\[1\]\[1\]\[0\] , \mem\[1\]\[1\]\[0\]\[1\] , \mem\[1\]\[1\]\[0\]\[0\]; ```

Possible Solution

I wonder if it would make sense to convert all but 1 packed dimensions into unpacked dimensions:

Input

logic [1:0][1:0] mem [2][2];

Output

reg [1:0] mem [0:1][0:1][1:0];

One issue I see is that BRAM inference will change: the word size will be the 1st packed array dimension size, not the total size of the packed array. However, this may be fine because Vivado only supports BRAM inference on SystemVerilog with 1 packed array dimension (source), so this change will make sv2v more similar to Vivado. I haven't tested other tools.

Another possible issue is I'm not sure of tool support for mixing big-endian and little-endian unpacked arrays.

Thanks!

sifferman commented 5 months ago

Actually, one issue I see is that Verilog ports don't support unpacked arrays. This makes me wonder if it would be better to flatten the ports, and assign to them with a generate for. It seems this is what used to be done (fb3d68e339d49494b77a46456077f9a8999b61a3). What was the reason for switching to flattening?