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.

Sign up for GitHub

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

Jump to bottom

Support RW ports on BRAMs #1614

Open
andrewb1999 opened this issue Jul 24, 2020 · 4 comments
Open

Support RW ports on BRAMs #1614

andrewb1999 opened this issue Jul 24, 2020 · 4 comments

Comments

@andrewb1999
Copy link
Collaborator

Example verilog that should be able to be mapped to a BRAM:

module ram2 (input clk,
             input             sel,
             input             we,
             input [SIZE-1:0]  adr,
             input [63:0]      dat_i,
             output reg [63:0] dat_o);
   parameter  SIZE = 8; // Address size

   reg [63:0] mem [0:(1 << SIZE)-1];
   integer    i;

   initial begin
      for (i = 0; i < (1<<SIZE) - 1; i = i + 1)
        mem[i] <= 0;
   end

   always @(posedge clk)
     if (sel) begin
       if (~we)
         dat_o <= mem[adr];
       else
         mem[adr] <= dat_i;
     end
endmodule

One potential issue is that the yosys bram map always maps the A port to read and the B port to write. TDP brams should be able to use either port as a read or a write, but my suspicion is that this is not supported by prjxray and symbiflow yet. Manually switching the read and write ports in the bram map resulted in a non working design for xc/xc7/tests/bram_test/bram_test_36, although it's very possible I switched polarities incorrectly.
RAMB36 TDP techmap before swapping ports:

                RAMB36E1 #(
                        .RAM_MODE("TDP"),
                        .READ_WIDTH_A(CFG_DBITS),
                        .READ_WIDTH_B(CFG_DBITS),
                        .WRITE_WIDTH_A(CFG_DBITS),
                        .WRITE_WIDTH_B(CFG_DBITS),
                        .WRITE_MODE_A("READ_FIRST"),
                        .WRITE_MODE_B("READ_FIRST"),
                        .IS_CLKARDCLK_INVERTED(!CLKPOL2),
                        .IS_CLKBWRCLK_INVERTED(!CLKPOL3),
                        `include "brams_init_36.vh"
                        .SIM_DEVICE("7SERIES")
                ) _TECHMAP_REPLACE_ (
                        .DIADI(32'd0),
                        .DIPADIP(4'd0),
                        .DOADO(DO[31:0]),
                        .DOPADOP(DOP[3:0]),
                        .ADDRARDADDR(A1ADDR_16),
                        .CLKARDCLK(CLK2),
                        .ENARDEN(A1EN),
                        .REGCEAREGCE(|1),
                        .RSTRAMARSTRAM(|0),
                        .RSTREGARSTREG(|0),
                        .WEA(4'b0),

                        .DIBDI(DI),
                        .DIPBDIP(DIP),
                        .DOBDO(DOBDO),
                        .DOPBDOP(DOPBDOP),
                        .ADDRBWRADDR(B1ADDR_16),
                        .CLKBWRCLK(CLK3),
                        .ENBWREN(|1),
                        .REGCEB(|0),
                        .RSTRAMB(|0),
                        .RSTREGB(|0),
                        .WEBWE(B1EN_8)
                );

After swapping ports:

       RAMB36E1 #(
            .RAM_MODE("TDP"),
            .READ_WIDTH_A(CFG_DBITS),
            .READ_WIDTH_B(CFG_DBITS),
            .WRITE_WIDTH_A(CFG_DBITS),
            .WRITE_WIDTH_B(CFG_DBITS),
            .WRITE_MODE_A("READ_FIRST"),
            .WRITE_MODE_B("READ_FIRST"),
            .IS_CLKARDCLK_INVERTED(!CLKPOL3),
            .IS_CLKBWRCLK_INVERTED(!CLKPOL2),
            `include "brams_init_36.vh"
            .SIM_DEVICE("7SERIES")
        ) _TECHMAP_REPLACE_ (
            .DIADI(DI),
            .DIPADIP(DIP),
            .DOADO(DOBDO),
            .DOPADOP(DOPBDOP),
            .ADDRARDADDR(B1ADDR_16),
            .CLKARDCLK(CLK3),
            .ENARDEN(|1),
            .REGCEAREGCE(|0),
            .RSTRAMARSTRAM(|0),
            .RSTREGARSTREG(|0),
            .WEA(B1EN_8),

            .DIBDI(32'd0),
            .DIPBDIP(4'd0),
            .DOBDO(DO[31:0]),
            .DOPBDOP(DOP[3:0]),
            .ADDRBWRADDR(A1ADDR_16),
            .CLKBWRCLK(CLK2),
            .ENBWREN(~A1EN),
            .REGCEB(|1),
            .RSTRAMB(|0),
            .RSTREGB(|0),
            .WEBWE(4'b0)
        );

Therefore, I think a first step in supporting RW ports is ensuring port A can be used as a write and port B can be used as a read. Could you give some input on whether prjxray and symbiflow support this functionality?
@litghost
Copy link
Contributor

litghost commented Jul 24, 2020 •
edited

Depends on how the failure manifests. In terms of concrete recommendation, here is what to check:

  1. Does your modified design work in Vivado as expected in hardware?
    1a. If you decode the bitstream from step 1 with bit2fasm --verbose are there any unknown bits?
  2. Does your modified design pass the "bram_test_36_vivado_diff_fasm" test?
  3. If you compare the FASM features for the BRAM tile from the Vivado bitstream and from the VPR bitstream do you notice any differences?

It is my belief the prjxray has the bits required for TDP BRAM36. I don't not believe we have fully debugged support for TDP BRAM36 in symbiflow-arch-defs. I found at least one bug earlier this week, see: https://github.com/SymbiFlow/symbiflow-arch-defs/pull/1612/files#diff-92df80b2f65bb8fef2dfa652e646d34e

GitHub
This PR attempts to fix #1597 by change the strategy used to decongest SLICEL CARRY4 outputs. Previously, all ALU CO uses were modeled as a O ^ S, but yosys's abc9 pass is now good enough to u...

@daveshah1
Copy link
Contributor

See also YosysHQ/yosys#1959

@litghost
Copy link
Contributor

@andrewb1999 Can you please see if you can replicate this issue? It may be fixed now?

@andrewb1999
Copy link
Collaborator Author

@litghost Looks like this issue has been fixed at least somewhat. I am able to map the provided verilog that I was unable to map before, I will try some more in depth tests soon.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@litghost @daveshah1 @andrewb1999