Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Memories doesn't synthetise to BRAM in Quartus toolchain #749

Closed
piotro888 opened this issue Nov 7, 2024 · 7 comments
Closed

Memories doesn't synthetise to BRAM in Quartus toolchain #749

piotro888 opened this issue Nov 7, 2024 · 7 comments
Labels
bug Something isn't working

Comments

@piotro888
Copy link
Member

Some memories fail to be synthesised to FPGA BRAM, [while running quartus toolchain targeted to DE2-115 (EP4CE115) with LiteX]

Instruction caches sythetises to 74k LUTs taking most of the device :(

There are two kinds errors in icache and some other blocks (including in the LiteX generated outside SoC!):

276007 : RAM logic is uninferred due to asynchronous read logic
276004: is uninferred due to inappropriate RAM size File (I'm ignoring this one for now)

Logs:

Info (276014): Found 44 instances of uninferred RAM logic
[...]
    Info (276007): RAM logic "top:top|top.main_module:main_module|top.main_module.frontend:frontend|top.main_module.frontend.icache:icache|top.main_module.frontend.icache.mem:mem|tag_mem_0" is uninferred due to asynchronous read logic File: build/terasic_de2_115/gateware/core.v Line: 102987
    Info (276004): RAM logic "top:top|top.main_module:main_module|top.main_module.frontend:frontend|top.main_module.frontend.icache:icache|top.main_module.frontend.icache.req_zipper:req_zipper|top.main_module.frontend.icache.req_zipper.fifo:fifo|buff" is uninferred due to inappropriate RAM size File: build/terasic_de2_115/gateware/core.v Line: 103484
    Info (276007): RAM logic "top:top|top.main_module:main_module|top.main_module.frontend:frontend|top.main_module.frontend.icache:icache|top.main_module.frontend.icache.mem:mem|tag_mem_1" is uninferred due to asynchronous read logic File: build/terasic_de2_115/gateware/core.v Line: 103130
    Info (276007): RAM logic "top:top|top.main_module:main_module|top.main_module.frontend:frontend|top.main_module.frontend.icache:icache|top.main_module.frontend.icache.mem:mem|data_mem_1" is uninferred due to asynchronous read logic File: build/terasic_de2_115/gateware/core.v Line: 101948
    Info (276007): RAM logic "top:top|top.main_module:main_module|top.main_module.frontend:frontend|top.main_module.frontend.icache:icache|top.main_module.frontend.icache.mem:mem|data_mem_0" is uninferred due to asynchronous read logic File: build/terasic_de2_115/gateware/core.v Line: 100909

Not recognized LiteX memory example:

//-----------------------------------------------------------------
-------------
// Memory storage_1: 16-words x 10-bit
//-----------------------------------------------------------------
-------------
// Port 0 | Read: Sync  | Write: Sync | Mode: Read-First  | Write-G
ranularity: 10 
// Port 1 | Read: Sync  | Write: ---- | 
reg [9:0] storage_1[0:15];
reg [9:0] storage_1_dat0;
reg [9:0] storage_1_dat1;
always @(posedge sys_clk) begin
        if (socdemo_uart_rx_fifo_wrport_we)
                storage_1[socdemo_uart_rx_fifo_wrport_adr] <= socde
mo_uart_rx_fifo_wrport_dat_w;
        storage_1_dat0 <= storage_1[socdemo_uart_rx_fifo_wrport_adr
];
end
always @(posedge sys_clk) begin
        if (socdemo_uart_rx_fifo_rdport_re)
                storage_1_dat1 <= storage_1[socdemo_uart_rx_fifo_rd
port_adr];
end
assign socdemo_uart_rx_fifo_wrport_dat_r = storage_1_dat0;
assign socdemo_uart_rx_fifo_rdport_dat_r = storage_1_dat1;

What's interesting - some of similiar LiteX rams are recognized correctly; and some are async on purpose and generate the same warning.

Not recognized Coreblocks (Amaranth) memory example:

  reg [31:0] data_mem_1 [1023:0];
  initial begin
    data_mem_1[0] = 32'd0;
    data_mem_1[1] = 32'd0;
    [...]
  end

  always @(posedge clk) begin
    if (\data_mem_wp__en$32 )
      data_mem_1[{ data_wr_addr[6:0], data_wr_addr[11:9] }] <= data
_mem_wp__data;
  end
  reg [31:0] _1_;
  always @(posedge clk) begin
    _1_ <= data_mem_1[{ data_rd_addr[6:0], data_rd_addr[11:9] }];
    if (\data_mem_wp__en$32  && { data_rd_addr[6:0], data_rd_addr[1
1:9] } == { data_wr_addr[6:0], data_wr_addr[11:9] })
      _1_ <= data_mem_wp__data;
  end
  initial _1_ = 32'd0;
  assign \$signal$27  = _1_;

This is probably an quartus issue, because it happens independently in different Amarnth and LiteX generated rams that look fine. Some workaround is needed.

@piotro888 piotro888 added the bug Something isn't working label Nov 7, 2024
@tilk
Copy link
Member

tilk commented Nov 8, 2024

The Quartus synthesis logic options might be of some help here. In particular, Add Pass-Through Logic to Inferred RAMs logic option might help with transparent RAMs (like the Coreblocks example), and Allow Any RAM Size For Recognition logic option should allow small block RAM synthesis (though I doubt this will help).

The asynchronous read warnings you get are weird - both of the examples you posted use synchronous reads.

@tilk
Copy link
Member

tilk commented Nov 14, 2024

I believe this might go away in future Amaranth versions, I found some related comments while working on updating Amaranth (like this one).

@tilk
Copy link
Member

tilk commented Nov 14, 2024

Could you test with the branch from PR #754?

@whitequark
Copy link

  always @(posedge clk) begin
    _1_ <= data_mem_1[{ data_rd_addr[6:0], data_rd_addr[11:9] }];
    if (\data_mem_wp__en$32  && { data_rd_addr[6:0], data_rd_addr[1
1:9] } == { data_wr_addr[6:0], data_wr_addr[11:9] })
      _1_ <= data_mem_wp__data;
  end

Giving this a very quick look, this pattern implements soft transparency logic and it's possible that Quartus dislikes it. Does it recognize properly if you disable transparency?

@piotro888
Copy link
Member Author

On amaranth 0.5.3 all memories are correctly inferred: 👍

Current logs for previously unrecognized transparent ones:
Warning (276020): Inferred RAM node "top:top|top.main_module:main_module|top.main_module.frontend:frontend|top.main_module.frontend.icache:icache|top.main_module.frontend.icache.mem:mem|data_mem_1_rtl_0" from synchronous design logic. Pass-through logic has been added to match the read-during-write behavior of the original design.

All nodes that synthetise to BRAMs in the core are: icache:data_mem, icache:tag_mem, free_rf_fifo, ROB, which seems correct.

Other Memory instances that we use are mostly in buffer FIFOs with depth of 2, making them too small to be worth converting (this is that 276004 warning, if needed it can be tuned). There are no more uninferred due to asynchronous read logs from coreblocks.


Some of identical LiteX SoC memories still generate incorrect uninferred due to asynchronous read warnings, but are inferred in next stage with correct size. weird....

    Info (276007): RAM logic "storage_9" is uninferred due to asynchronous read logic File: build/terasic_de2_115/gateware/terasic_de2_115.v Line: 13922
    Info (276029): Inferred altsyncram megafunction from the following design logic: "storage_9_rtl_0" 
        Info (286033): Parameter OPERATION_MODE set to DUAL_PORT
        Info (286033): Parameter WIDTH_A set to 10
        Info (286033): Parameter WIDTHAD_A set to 9
        Info (286033): Parameter NUMWORDS_A set to 512

comment in rtl source:

//------------------------------------------------------------------------------
// Memory storage_9: 512-words x 10-bit
//------------------------------------------------------------------------------
// Port 0 | Read: Sync  | Write: Sync | Mode: Read-First  | Write-Granularity: 10 
// Port 1 | Read: Sync  | Write: ---- | 

So both issues are (probably) resolved. I will further verify LiteX ones later

@whitequark
Copy link

On amaranth 0.5.3 all memories are correctly inferred: 👍

Nice!

@piotro888
Copy link
Member Author

I will further verify LiteX ones later

Warnings for mentioned LiteX turned out to be false-positive, as expected.
High register usage from LiteX SoC was related to completely different issue with LiteEth memories with multiple write enable signals, that were not detected by Quartus analysis entirely! It should be addresses by PR enjoy-digital/litex#2140 or setting full_memory_we option in add_ethernet.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants