0

In the code below, the line: mem_reg[wr_cmd_addr[SEG_ADDR_WIDTH*n +: INT_ADDR_WIDTH]][i*8 +: 8] <= wr_cmd_data[SEG_DATA_WIDTH*n+i*8 +: 8];

The index "i" is an integer type. It is being synthesized right??

I was under the impression that integer variables are only used for simulations in the initial procedural block

Also, the BRAM reg [SEG_DATA_WIDTH-1:0] mem_reg[2**INT_ADDR_WIDTH-1:0]; is being synthesized the number of times the genvar variable "n" loops in the for loop? The multiple generated BRAMs mem_reg will have the same names? And they cannot be accessed separately by name with something like: mem_reg[n] right?

`resetall
`timescale 1ns / 1ps
`default_nettype none

/*
 * DMA parallel simple dual port RAM
 */
module dma_psdpram #
(
    // RAM size
    parameter SIZE = 4096,
    // RAM segment count
    parameter SEG_COUNT = 2,
    // RAM segment data width
    parameter SEG_DATA_WIDTH = 128,
    // RAM segment byte enable width
    parameter SEG_BE_WIDTH = SEG_DATA_WIDTH/8,
    // RAM segment address width
    parameter SEG_ADDR_WIDTH = $clog2(SIZE/(SEG_COUNT*SEG_BE_WIDTH)),
    // Read data output pipeline stages
    parameter PIPELINE = 2
)
(
    input  wire                                clk,
    input  wire                                rst,

    /*
     * Write port
     */
    input  wire [SEG_COUNT*SEG_BE_WIDTH-1:0]   wr_cmd_be,
    input  wire [SEG_COUNT*SEG_ADDR_WIDTH-1:0] wr_cmd_addr,
    input  wire [SEG_COUNT*SEG_DATA_WIDTH-1:0] wr_cmd_data,
    input  wire [SEG_COUNT-1:0]                wr_cmd_valid,
    output wire [SEG_COUNT-1:0]                wr_cmd_ready,
    output wire [SEG_COUNT-1:0]                wr_done,

    /*
     * Read port
     */
    input  wire [SEG_COUNT*SEG_ADDR_WIDTH-1:0] rd_cmd_addr,
    input  wire [SEG_COUNT-1:0]                rd_cmd_valid,
    output wire [SEG_COUNT-1:0]                rd_cmd_ready,
    output wire [SEG_COUNT*SEG_DATA_WIDTH-1:0] rd_resp_data,
    output wire [SEG_COUNT-1:0]                rd_resp_valid,
    input  wire [SEG_COUNT-1:0]                rd_resp_ready
);

parameter INT_ADDR_WIDTH = $clog2(SIZE/(SEG_COUNT*SEG_BE_WIDTH));

// check configuration
initial begin
    if (SEG_ADDR_WIDTH < INT_ADDR_WIDTH) begin
        $error("Error: SEG_ADDR_WIDTH not sufficient for requested size (min %d for size %d) (instance %m)", INT_ADDR_WIDTH, SIZE);
        $finish;
    end
end

generate

genvar n;

for (n = 0; n < SEG_COUNT; n = n + 1) begin

    (* ramstyle = "no_rw_check" *)
    reg [SEG_DATA_WIDTH-1:0] mem_reg[2**INT_ADDR_WIDTH-1:0];

    reg wr_done_reg = 1'b0;

    reg [PIPELINE-1:0] rd_resp_valid_pipe_reg = 0;
    reg [SEG_DATA_WIDTH-1:0] rd_resp_data_pipe_reg[PIPELINE-1:0];

    integer i, j;

    initial begin
        // two nested loops for smaller number of iterations per loop
        // workaround for synthesizer complaints about large loop counts
        for (i = 0; i < 2**INT_ADDR_WIDTH; i = i + 2**(INT_ADDR_WIDTH/2)) begin
            for (j = i; j < i + 2**(INT_ADDR_WIDTH/2); j = j + 1) begin
                mem_reg[j] = 0;
            end
        end

        for (i = 0; i < PIPELINE; i = i + 1) begin
            rd_resp_data_pipe_reg[i] = 0;
        end
    end

    always @(posedge clk) begin
        wr_done_reg <= 1'b0;

        for (i = 0; i < SEG_BE_WIDTH; i = i + 1) begin
            if (wr_cmd_valid[n] && wr_cmd_be[n*SEG_BE_WIDTH+i]) begin
                mem_reg[wr_cmd_addr[SEG_ADDR_WIDTH*n +: INT_ADDR_WIDTH]][i*8 +: 8] <= wr_cmd_data[SEG_DATA_WIDTH*n+i*8 +: 8];
                wr_done_reg <= 1'b1;
            end
        end

        if (rst) begin
            wr_done_reg <= 1'b0;
        end
    end

    assign wr_cmd_ready[n] = 1'b1;
    assign wr_done[n] = wr_done_reg;

    always @(posedge clk) begin
        if (rd_resp_ready[n]) begin
            rd_resp_valid_pipe_reg[PIPELINE-1] <= 1'b0;
        end

        for (j = PIPELINE-1; j > 0; j = j - 1) begin
            if (rd_resp_ready[n] || ((~rd_resp_valid_pipe_reg) >> j)) begin
                rd_resp_valid_pipe_reg[j] <= rd_resp_valid_pipe_reg[j-1];
                rd_resp_data_pipe_reg[j] <= rd_resp_data_pipe_reg[j-1];
                rd_resp_valid_pipe_reg[j-1] <= 1'b0;
            end
        end

        if (rd_cmd_valid[n] && rd_cmd_ready[n]) begin
            rd_resp_valid_pipe_reg[0] <= 1'b1;
            rd_resp_data_pipe_reg[0] <= mem_reg[rd_cmd_addr[SEG_ADDR_WIDTH*n +: INT_ADDR_WIDTH]];
        end

        if (rst) begin
            rd_resp_valid_pipe_reg <= 0;
        end
    end

    assign rd_cmd_ready[n] = rd_resp_ready[n] || ~rd_resp_valid_pipe_reg;

    assign rd_resp_valid[n] = rd_resp_valid_pipe_reg[PIPELINE-1];
    assign rd_resp_data[SEG_DATA_WIDTH*n +: SEG_DATA_WIDTH] = rd_resp_data_pipe_reg[PIPELINE-1];
end

endgenerate

endmodule

`resetall
AZ123
  • 156
  • 2
  • 11
  • It is running in simulation. It is from a famous repository: https://github.com/corundum/corundum This is the file to be exact: https://github.com/corundum/corundum/blob/master/fpga/lib/pcie/rtl/dma_psdpram.v – AZ123 Jul 19 '22 at 15:02
  • Looks like 'i' and other iterators are elaboration/compile time constants. (no circuits in hardware are needed to determine those values) - so synthesizeability isnt much of a concern here. Same as asking 'is the integer constant 3 synthesizable?' or something like that – Julian Kemmerer Jul 19 '22 at 20:58
  • @JulianKemmerer circuits are being made in the code below using the for loop with integer i, no? (mem_reg is being connected to different data bits of wr_cmd_data, based on the values of integer "i" and genvar "n"). (The is the code =>) for (i = 0; i < SEG_BE_WIDTH; i = i + 1) begin if (wr_cmd_valid[n] && wr_cmd_be[n*SEG_BE_WIDTH+i]) begin mem_reg[wr_cmd_addr[SEG_ADDR_WIDTH*n +: INT_ADDR_WIDTH]][i*8 +: 8] <= wr_cmd_data[SEG_DATA_WIDTH*n+i*8 +: 8]; – AZ123 Jul 19 '22 at 21:38
  • @AZ123 `based on the values of integer "i" and genvar "n"` those values are known at compile time. There is no circuit , no LUTs, etc that calculates the 'i' value. For example, where you see `[i*8 +: 8]` you can replace that expressions with i=the constant iterator value, ex. from unrolling to second iteration, i=1 so it is equivalent to [15:8] - there is nothing non-synthesizeable about selecting bits 15 downto 8 etc. Any time stuff/expressions evaluate to a constant value you shouldn't need to worry about synthesizability. – Julian Kemmerer Jul 19 '22 at 23:01
  • @JulianKemmerer okay but the for loop with "i" is being unrolled at compile time right? I thought that integer data types were ONLY for simulation purposes, but here the logic is being made that x[i*8 +: 8] e.g., x[15:8] corresponds to input[15:8] bits of the incoming data – AZ123 Jul 19 '22 at 23:07
  • 1
    @AZ123 Yes all loops are unrolled. Instead of "integer data types were ONLY for simulation purposes," think "non-constant integer data types should not be synthesized" as a better rule of thumb/general guideline. But even then some tools will allow non-constant/variable integers to be synthesized as logic on signed 32b numbers so exceptions, etc – Julian Kemmerer Jul 20 '22 at 14:25
  • @JulianKemmerer thanks it is clearer now. Also can you look at the second question **The multiple generated BRAMs mem_reg will have the same names? And they cannot be accessed separately by name with something like: mem_reg[n] right?** These BRAMs with same names can only be accessed inside the genvar "n" for loop right? – AZ123 Jul 20 '22 at 14:43

1 Answers1

1

try to use named blocks:

for (n = 0; n < SEG_COUNT; n = n + 1) begin : blkname
  reg [SEG_DATA_WIDTH-1:0] mem_reg [2**INT_ADDR_WIDTH-1:0];
end

And access as:

assign x = blkname[i].mem_reg[j];
Mogwaika
  • 46
  • 7