search

zachjs / sv2v

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

Converted Verilog Outputs Different Result Compared to Original SysVerilog #266

Closed marchuang6272 closed 5 months ago

marchuang6272 commented 6 months ago

Hello! I used this repo to convert my adder_tree.sv into adder_tree.v. I ran the same testbench for both modules and got different outputs. Could I get help on this issue? Thanks. The adder tree simply adds of its inputs together. The testbench should output 28.

Here is the converted Verilog code from sv2v module adder_tree ( clk, nrst, idata, odata ); parameter INPUTS_NUM = 125; parameter IDATA_WIDTH = 16; parameter STAGES_NUM = $clog2(INPUTS_NUM); parameter INPUTS_NUM_INT = 2 ** STAGES_NUM; parameter ODATA_WIDTH = IDATA_WIDTH + STAGES_NUM; input clk; input nrst; input wire [(INPUTS_NUM * IDATA_WIDTH) - 1:0] idata; output wire [ODATA_WIDTH - 1:0] odata; reg [((STAGES_NUM >= 0 ? ((STAGES_NUM + 1) * INPUTS_NUM_INT) - 1 : ((1 - STAGES_NUM) * INPUTS_NUM_INT) + ((STAGES_NUM * INPUTS_NUM_INT) - 1)) >= (STAGES_NUM >= 0 ? 0 : STAGES_NUM * INPUTS_NUM_INT) ? ((((STAGES_NUM >= 0 ? ((STAGES_NUM + 1) * INPUTS_NUM_INT) - 1 : ((1 - STAGES_NUM) * INPUTS_NUM_INT) + ((STAGES_NUM * INPUTS_NUM_INT) - 1)) - (STAGES_NUM >= 0 ? 0 : STAGES_NUM * INPUTS_NUM_INT)) + 1) * ODATA_WIDTH) + (((STAGES_NUM >= 0 ? 0 : STAGES_NUM * INPUTS_NUM_INT) * ODATA_WIDTH) - 1) : ((((STAGES_NUM >= 0 ? 0 : STAGES_NUM * INPUTS_NUM_INT) - (STAGES_NUM >= 0 ? ((STAGES_NUM + 1) * INPUTS_NUM_INT) - 1 : ((1 - STAGES_NUM) * INPUTS_NUM_INT) + ((STAGES_NUM * INPUTS_NUM_INT) - 1))) + 1) * ODATA_WIDTH) + (((STAGES_NUM >= 0 ? ((STAGES_NUM + 1) * INPUTS_NUM_INT) - 1 : ((1 - STAGES_NUM) * INPUTS_NUM_INT) + ((STAGES_NUM * INPUTS_NUM_INT) - 1)) * ODATA_WIDTH) - 1)):((STAGES_NUM >= 0 ? ((STAGES_NUM + 1) * INPUTS_NUM_INT) - 1 : ((1 - STAGES_NUM) * INPUTS_NUM_INT) + ((STAGES_NUM * INPUTS_NUM_INT) - 1)) >= (STAGES_NUM >= 0 ? 0 : STAGES_NUM * INPUTS_NUM_INT) ? (STAGES_NUM >= 0 ? 0 : STAGES_NUM * INPUTS_NUM_INT) * ODATA_WIDTH : (STAGES_NUM >= 0 ? ((STAGES_NUM + 1) * INPUTS_NUM_INT) - 1 : ((1 - STAGES_NUM) * INPUTS_NUM_INT) + ((STAGES_NUM * INPUTS_NUM_INT) - 1)) * ODATA_WIDTH)] data; genvar _gv_stage_1; genvar _gv_adder_1; generate for (_gv_stage_1 = 0; _gv_stage_1 <= STAGES_NUM; _gv_stage_1 = _gv_stage_1 + 1) begin : stage_gen localparam stage = _gv_stage_1; localparam ST_OUT_NUM = INPUTS_NUM_INT >> stage; localparam ST_WIDTH = IDATA_WIDTH + stage; if (stage == {32 {1'sb0}}) begin : genblk1 for (_gv_adder_1 = 0; _gv_adder_1 < ST_OUT_NUM; _gv_adder_1 = _gv_adder_1 + 1) begin : inputs_gen localparam adder = _gv_adder_1; always @(*) if (adder < INPUTS_NUM) begin data[(((STAGES_NUM >= 0 ? ((STAGES_NUM + 1) * INPUTS_NUM_INT) - 1 : ((1 - STAGES_NUM) * INPUTS_NUM_INT) + ((STAGES_NUM * INPUTS_NUM_INT) - 1)) >= (STAGES_NUM >= 0 ? 0 : STAGES_NUM * INPUTS_NUM_INT) ? ((STAGES_NUM >= 0 ? stage : STAGES_NUM - stage) * INPUTS_NUM_INT) + adder : (STAGES_NUM >= 0 ? 0 : STAGES_NUM * INPUTS_NUM_INT) - ((((STAGES_NUM >= 0 ? stage : STAGES_NUM - stage) * INPUTS_NUM_INT) + adder) - (STAGES_NUM >= 0 ? ((STAGES_NUM + 1) * INPUTS_NUM_INT) - 1 : ((1 - STAGES_NUM) * INPUTS_NUM_INT) + ((STAGES_NUM * INPUTS_NUM_INT) - 1)))) * ODATA_WIDTH) + (ST_WIDTH - 1)-:ST_WIDTH] <= idata[(adder * IDATA_WIDTH) + (ST_WIDTH - 1)-:ST_WIDTH]; data[(((STAGES_NUM >= 0 ? ((STAGES_NUM + 1) * INPUTS_NUM_INT) - 1 : ((1 - STAGES_NUM) * INPUTS_NUM_INT) + ((STAGES_NUM * INPUTS_NUM_INT) - 1)) >= (STAGES_NUM >= 0 ? 0 : STAGES_NUM * INPUTS_NUM_INT) ? ((STAGES_NUM >= 0 ? stage : STAGES_NUM - stage) * INPUTS_NUM_INT) + adder : (STAGES_NUM >= 0 ? 0 : STAGES_NUM * INPUTS_NUM_INT) - ((((STAGES_NUM >= 0 ? stage : STAGES_NUM - stage) * INPUTS_NUM_INT) + adder) - (STAGES_NUM >= 0 ? ((STAGES_NUM + 1) * INPUTS_NUM_INT) - 1 : ((1 - STAGES_NUM) * INPUTS_NUM_INT) + ((STAGES_NUM * INPUTS_NUM_INT) - 1)))) * ODATA_WIDTH) + ((ODATA_WIDTH - 1) >= ST_WIDTH ? ODATA_WIDTH - 1 : ((ODATA_WIDTH - 1) + ((ODATA_WIDTH - 1) >= ST_WIDTH ? ((ODATA_WIDTH - 1) - ST_WIDTH) + 1 : (ST_WIDTH - (ODATA_WIDTH - 1)) + 1)) - 1)-:((ODATA_WIDTH - 1) >= ST_WIDTH ? ((ODATA_WIDTH - 1) - ST_WIDTH) + 1 : (ST_WIDTH - (ODATA_WIDTH - 1)) + 1)] <= 1'sb0; end else data[(((STAGES_NUM >= 0 ? ((STAGES_NUM + 1) * INPUTS_NUM_INT) - 1 : ((1 - STAGES_NUM) * INPUTS_NUM_INT) + ((STAGES_NUM * INPUTS_NUM_INT) - 1)) >= (STAGES_NUM >= 0 ? 0 : STAGES_NUM * INPUTS_NUM_INT) ? ((STAGES_NUM >= 0 ? stage : STAGES_NUM - stage) * INPUTS_NUM_INT) + adder : (STAGES_NUM >= 0 ? 0 : STAGES_NUM * INPUTS_NUM_INT) - ((((STAGES_NUM >= 0 ? stage : STAGES_NUM - stage) * INPUTS_NUM_INT) + adder) - (STAGES_NUM >= 0 ? ((STAGES_NUM + 1) * INPUTS_NUM_INT) - 1 : ((1 - STAGES_NUM) * INPUTS_NUM_INT) + ((STAGES_NUM * INPUTS_NUM_INT) - 1)))) * ODATA_WIDTH) + (ODATA_WIDTH - 1)-:ODATA_WIDTH] <= 1'sb0; end end else begin : genblk1 for (_gv_adder_1 = 0; _gv_adder_1 < ST_OUT_NUM; _gv_adder_1 = _gv_adder_1 + 1) begin : adder_gen localparam adder = _gv_adder_1; always @(posedge clk) if (~nrst) data[(((STAGES_NUM >= 0 ? ((STAGES_NUM + 1) * INPUTS_NUM_INT) - 1 : ((1 - STAGES_NUM) * INPUTS_NUM_INT) + ((STAGES_NUM * INPUTS_NUM_INT) - 1)) >= (STAGES_NUM >= 0 ? 0 : STAGES_NUM * INPUTS_NUM_INT) ? ((STAGES_NUM >= 0 ? stage : STAGES_NUM - stage) * INPUTS_NUM_INT) + adder : (STAGES_NUM >= 0 ? 0 : STAGES_NUM * INPUTS_NUM_INT) - ((((STAGES_NUM >= 0 ? stage : STAGES_NUM - stage) * INPUTS_NUM_INT) + adder) - (STAGES_NUM >= 0 ? ((STAGES_NUM + 1) * INPUTS_NUM_INT) - 1 : ((1 - STAGES_NUM) * INPUTS_NUM_INT) + ((STAGES_NUM * INPUTS_NUM_INT) - 1)))) * ODATA_WIDTH) + (ODATA_WIDTH - 1)-:ODATA_WIDTH] <= 1'sb0; else data[(((STAGES_NUM >= 0 ? ((STAGES_NUM + 1) * INPUTS_NUM_INT) - 1 : ((1 - STAGES_NUM) * INPUTS_NUM_INT) + ((STAGES_NUM * INPUTS_NUM_INT) - 1)) >= (STAGES_NUM >= 0 ? 0 : STAGES_NUM * INPUTS_NUM_INT) ? ((STAGES_NUM >= 0 ? stage : STAGES_NUM - stage) * INPUTS_NUM_INT) + adder : (STAGES_NUM >= 0 ? 0 : STAGES_NUM * INPUTS_NUM_INT) - ((((STAGES_NUM >= 0 ? stage : STAGES_NUM - stage) * INPUTS_NUM_INT) + adder) - (STAGES_NUM >= 0 ? ((STAGES_NUM + 1) * INPUTS_NUM_INT) - 1 : ((1 - STAGES_NUM) * INPUTS_NUM_INT) + ((STAGES_NUM * INPUTS_NUM_INT) - 1)))) * ODATA_WIDTH) + (ST_WIDTH - 1)-:ST_WIDTH] <= data[(((STAGES_NUM >= 0 ? ((STAGES_NUM + 1) * INPUTS_NUM_INT) - 1 : ((1 - STAGES_NUM) * INPUTS_NUM_INT) + ((STAGES_NUM * INPUTS_NUM_INT) - 1)) >= (STAGES_NUM >= 0 ? 0 : STAGES_NUM * INPUTS_NUM_INT) ? ((STAGES_NUM >= 0 ? stage - 1 : STAGES_NUM - (stage - 1)) * INPUTS_NUM_INT) + (adder * 2) : (STAGES_NUM >= 0 ? 0 : STAGES_NUM * INPUTS_NUM_INT) - ((((STAGES_NUM >= 0 ? stage - 1 : STAGES_NUM - (stage - 1)) * INPUTS_NUM_INT) + (adder * 2)) - (STAGES_NUM >= 0 ? ((STAGES_NUM + 1) * INPUTS_NUM_INT) - 1 : ((1 - STAGES_NUM) * INPUTS_NUM_INT) + ((STAGES_NUM * INPUTS_NUM_INT) - 1)))) * ODATA_WIDTH) + ((ST_WIDTH - 2) >= 0 ? ST_WIDTH - 2 : ((ST_WIDTH - 2) + ((ST_WIDTH - 2) >= 0 ? ST_WIDTH - 1 : 3 - ST_WIDTH)) - 1)-:((ST_WIDTH - 2) >= 0 ? ST_WIDTH - 1 : 3 - ST_WIDTH)] + data[(((STAGES_NUM >= 0 ? ((STAGES_NUM + 1) * INPUTS_NUM_INT) - 1 : ((1 - STAGES_NUM) * INPUTS_NUM_INT) + ((STAGES_NUM * INPUTS_NUM_INT) - 1)) >= (STAGES_NUM >= 0 ? 0 : STAGES_NUM * INPUTS_NUM_INT) ? ((STAGES_NUM >= 0 ? stage - 1 : STAGES_NUM - (stage - 1)) * INPUTS_NUM_INT) + ((adder * 2) + 1) : (STAGES_NUM >= 0 ? 0 : STAGES_NUM * INPUTS_NUM_INT) - ((((STAGES_NUM >= 0 ? stage - 1 : STAGES_NUM - (stage - 1)) * INPUTS_NUM_INT) + ((adder * 2) + 1)) - (STAGES_NUM >= 0 ? ((STAGES_NUM + 1) * INPUTS_NUM_INT) - 1 : ((1 - STAGES_NUM) * INPUTS_NUM_INT) + ((STAGES_NUM * INPUTS_NUM_INT) - 1)))) * ODATA_WIDTH) + ((ST_WIDTH - 2) >= 0 ? ST_WIDTH - 2 : ((ST_WIDTH - 2) + ((ST_WIDTH - 2) >= 0 ? ST_WIDTH - 1 : 3 - ST_WIDTH)) - 1)-:((ST_WIDTH - 2) >= 0 ? ST_WIDTH - 1 : 3 - ST_WIDTH)]; end end end endgenerate assign odata = data[((STAGES_NUM >= 0 ? ((STAGES_NUM + 1) * INPUTS_NUM_INT) - 1 : ((1 - STAGES_NUM) * INPUTS_NUM_INT) + ((STAGES_NUM * INPUTS_NUM_INT) - 1)) >= (STAGES_NUM >= 0 ? 0 : STAGES_NUM * INPUTS_NUM_INT) ? (STAGES_NUM >= 0 ? STAGES_NUM : STAGES_NUM - STAGES_NUM) * INPUTS_NUM_INT : (STAGES_NUM >= 0 ? 0 : STAGES_NUM * INPUTS_NUM_INT) - (((STAGES_NUM >= 0 ? STAGES_NUM : STAGES_NUM - STAGES_NUM) * INPUTS_NUM_INT) - (STAGES_NUM >= 0 ? ((STAGES_NUM + 1) * INPUTS_NUM_INT) - 1 : ((1 - STAGES_NUM) * INPUTS_NUM_INT) + ((STAGES_NUM * INPUTS_NUM_INT) - 1)))) * ODATA_WIDTH+:ODATA_WIDTH]; endmodule

and its original SystemVerilog counterpart:

`module adder_tree #( parameter INPUTS_NUM = 125, parameter IDATA_WIDTH = 16,

parameter STAGES_NUM = $clog2(INPUTS_NUM), parameter INPUTS_NUM_INT = 2 ** STAGES_NUM, parameter ODATA_WIDTH = IDATA_WIDTH + STAGES_NUM )( input clk, input nrst, input logic [INPUTS_NUM-1:0][IDATA_WIDTH-1:0] idata, output logic [ODATA_WIDTH-1:0] odata );

logic [STAGES_NUM:0][INPUTS_NUM_INT-1:0][ODATA_WIDTH-1:0] data;

// generating tree genvar stage, adder; generate for( stage = 0; stage <= STAGES_NUM; stage++ ) begin: stage_gen

localparam ST_OUT_NUM = INPUTS_NUM_INT >> stage;
localparam ST_WIDTH = IDATA_WIDTH + stage;

if( stage == '0 ) begin
  // stege 0 is actually module inputs
  for( adder = 0; adder < ST_OUT_NUM; adder++ ) begin: inputs_gen

    always_comb begin
      if( adder < INPUTS_NUM ) begin
        data[stage][adder][ST_WIDTH-1:0] <= idata[adder][ST_WIDTH-1:0];
        data[stage][adder][ODATA_WIDTH-1:ST_WIDTH] <= '0;
      end else begin
        data[stage][adder][ODATA_WIDTH-1:0] <= '0;
      end
    end // always_comb

  end // for
end else begin
  // all other stages hold adders outputs
  for( adder = 0; adder < ST_OUT_NUM; adder++ ) begin: adder_gen

    //always_comb begin       // is also possible here
    always_ff@(posedge clk) begin
      if( ~nrst ) begin
        data[stage][adder][ODATA_WIDTH-1:0] <= '0;
      end else begin
        data[stage][adder][ST_WIDTH-1:0] <=
                data[stage-1][adder*2][(ST_WIDTH-1)-1:0] +
                data[stage-1][adder*2+1][(ST_WIDTH-1)-1:0];
      end
    end // always

  end // for
end // if stage

end // for endgenerate

assign odata = data[STAGES_NUM][0];

endmodule `

lastly, here is the testbench I use to test both.

`

`timescale 1ns / 1ps

module adder_tree_tb();

logic rst; initial begin

0 rst = 1'b1;

10 rst = 1'b0;

5 rst = 1'b1;

end

logic clk; initial begin

0 clk = 1'b0;

forever

2.5 clk = ~clk;

end // Module under test ========================================================== initial begin

30 $display("%0d", at.odata);

end adder_tree #( .INPUTS_NUM( 7 ), .IDATA_WIDTH( 16 ) ) at ( .clk( clk ), .nrst( rst), .idata( { 16'd1, 16'd2, 16'd3, 16'd4, 16'd5, 16'd6, 16'd7 }), .odata() );

endmodule `

zachjs commented 6 months ago

Thank you for filing this issue! The difference results from a shortcoming in sv2v's conversion of always_comb, where the resulting always block doesn't necessarily execute at time zero as the standard requires.

You have three options:

  1. If your downstream flow supports always_comb (Yosys does), you can pass -E always to sv2v to exclude the problematic conversion.
  2. Wait a couple weeks to give me time to fix the issue in sv2v. I may do this by generating a synthetic variable to trigger the always block at time zero, similar to the way processes are handled in the Yosys Verilog backend.
  3. You can change your code (temporarily) to workaround the issue, e.g., by moving the else data[stage][adder][ODATA_WIDTH-1:0] <= '0; into an initial block.
zachjs commented 6 months ago

@marchuang6272 I just pushed f9917d94da51a09812cb1356ab2cd4971b86b88a, which ensures that always_comb and always_latch execute at time zero. I confirmed your test case now produces the expected output when converted. Please let me know if it works for you. Thank you!

zachjs commented 5 months ago

I'm closing this because this issue should be resolved. I appreciate you taking the time to file an issue! Please feel free to reopen or open a new issue as appropriate.