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

dual port blockram fails to synthesize ECP5 #3205

Closed
rowanG077 opened this issue Feb 16, 2022 · 8 comments · Fixed by #3189
Closed

dual port blockram fails to synthesize ECP5 #3205

rowanG077 opened this issue Feb 16, 2022 · 8 comments · Fixed by #3189

Comments

@rowanG077
Copy link

Steps to reproduce the issue

I have a true dual port block ram module as generated by the clash-compiler:

/* AUTOMATICALLY GENERATED VERILOG-2001 SOURCE CODE.
** GENERATED BY CLASH 1.6.1. DO NOT MODIFY.
*/
`timescale 100fs/100fs
module TopEntity_topEntity_trueDualPortBlockRamWrapper
    ( // Inputs
      input  clkA // clock
    , input  enA
    , input  weA
    , input [15:0] addrA
    , input [23:0] datA
    , input  clkB // clock
    , input  enB
    , input  weB
    , input [15:0] addrB
    , input [23:0] datB

      // Outputs
    , output wire [47:0] result
    );


  // trueDualPortBlockRam begin
  // Shared memory
  reg [24-1:0] mem [65536-1:0];

  reg [23:0] data_slow;
  reg [23:0] data_fast;

  // Port A
  always @(posedge clkA) begin
      if(enA) begin
          data_slow <= mem[addrA];
          if(weA) begin
              data_slow <= datA;
              mem[addrA] <= datA;
          end
      end
  end

  // Port B
  always @(posedge clkB) begin
      if(enB) begin
          data_fast <= mem[addrB];
          if(weB) begin
              data_fast <= datB;
              mem[addrB] <= datB;
          end
      end
  end

  assign result = {data_slow, data_fast};

  // end trueDualPortBlockRam
endmodule

Running yosys works without issues:

yosys -p "synth_ecp5 -top TopEntity_topEntity_trueDualPortBlockRamWrapper -json ./synth.json" ./TopEntity_topEntity_trueDualPortBlockRamWrapper.v

But when running nextpnr with the command:

nextpnr-ecp5 --json ./synth.json --25k --package CABGA256 --lpf-allow-unconstrained

It fails with the message:

ERROR: cell type '$mem_v2' is unsupported (instantiated as 'mem')

I assume this means ECP5 doesn't support a true dual port blockram right now? Is there anything I can do to help make it supported :)?

@Xiretza
Copy link
Contributor

Xiretza commented Feb 16, 2022

If you end up with any cell type starting with $ in your synth output, it means that yosys failed to map it to an architecture-specific cell. In this case, the output of the MEMORY_BRAM pass gives us the necessary information:

2.26. Executing MEMORY_BRAM pass (mapping $mem cells to block memories).
Processing TopEntity_topEntity_trueDualPortBlockRamWrapper.mem:
  Properties: ports=4 bits=1572864 rports=2 wports=2 dbits=24 abits=16 words=65536
  Checking rule #1 for bram type $__ECP5_PDPW16KD (variant 1):
    Bram geometry: abits=9 dbits=36 wports=0 rports=0
    Estimated number of duplicates for more read ports: dups=1
    Metrics for $__ECP5_PDPW16KD: awaste=0 dwaste=12 bwaste=6144 waste=6144 efficiency=66
    Rule #1 for bram type $__ECP5_PDPW16KD (variant 1) accepted.
    Mapping to bram type $__ECP5_PDPW16KD (variant 1):
      Shuffle bit order to accommodate enable buckets of size 9..
      Results of bit order shuffling: 0 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 -1 -1 -1
      Write port #0 is in clock domain \clkB.
        Mapped to bram port A1.
      Write port #1 is in clock domain \clkA.
        Failed to map write port #1.
    Mapping to bram type $__ECP5_PDPW16KD failed.
  Checking rule #2 for bram type $__ECP5_PDPW16KD (variant 1):
    Bram geometry: abits=9 dbits=36 wports=0 rports=0
    Estimated number of duplicates for more read ports: dups=1
  Checking rule #3 for bram type $__ECP5_PDPW16KD (variant 1):
    Bram geometry: abits=9 dbits=36 wports=0 rports=0
    Estimated number of duplicates for more read ports: dups=1
  Checking rule #4 for bram type $__ECP5_DP16KD (variant 1):
    Bram geometry: abits=10 dbits=18 wports=0 rports=0
    Estimated number of duplicates for more read ports: dups=1
    Metrics for $__ECP5_DP16KD: awaste=0 dwaste=12 bwaste=12288 waste=12288 efficiency=66
    Rule #4 for bram type $__ECP5_DP16KD (variant 1) accepted.
    Mapping to bram type $__ECP5_DP16KD (variant 1):
      Shuffle bit order to accommodate enable buckets of size 9..
      Results of bit order shuffling: 0 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 -1 -1 -1
      Write port #0 is in clock domain \clkB.
        Mapped to bram port A1.
      Write port #1 is in clock domain \clkA.
        Failed to map write port #1.
    Mapping to bram type $__ECP5_DP16KD failed.
  Checking rule #4 for bram type $__ECP5_DP16KD (variant 2):
    Bram geometry: abits=11 dbits=9 wports=0 rports=0
    Estimated number of duplicates for more read ports: dups=1
    Metrics for $__ECP5_DP16KD: awaste=0 dwaste=3 bwaste=6144 waste=6144 efficiency=88
    Rule #4 for bram type $__ECP5_DP16KD (variant 2) accepted.
    Mapping to bram type $__ECP5_DP16KD (variant 2):
      Shuffle bit order to accommodate enable buckets of size 9..
      Results of bit order shuffling: 0 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 -1 -1 -1
      Write port #0 is in clock domain \clkB.
        Mapped to bram port A1.
      Write port #1 is in clock domain \clkA.
        Failed to map write port #1.
    Mapping to bram type $__ECP5_DP16KD failed.
  Checking rule #4 for bram type $__ECP5_DP16KD (variant 3):
    Bram geometry: abits=12 dbits=4 wports=0 rports=0
    Estimated number of duplicates for more read ports: dups=1
    Metrics for $__ECP5_DP16KD: awaste=0 dwaste=0 bwaste=0 waste=0 efficiency=100
    Rule #4 for bram type $__ECP5_DP16KD (variant 3) accepted.
    Mapping to bram type $__ECP5_DP16KD (variant 3):
      Shuffle bit order to accommodate enable buckets of size 4..
      Results of bit order shuffling: 0 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23
      Write port #0 is in clock domain \clkB.
        Mapped to bram port A1.
      Write port #1 is in clock domain \clkA.
        Failed to map write port #1.
    Mapping to bram type $__ECP5_DP16KD failed.
  Checking rule #4 for bram type $__ECP5_DP16KD (variant 4):
    Bram geometry: abits=13 dbits=2 wports=0 rports=0
    Estimated number of duplicates for more read ports: dups=1
    Metrics for $__ECP5_DP16KD: awaste=0 dwaste=0 bwaste=0 waste=0 efficiency=100
    Rule #4 for bram type $__ECP5_DP16KD (variant 4) accepted.
    Mapping to bram type $__ECP5_DP16KD (variant 4):
      Shuffle bit order to accommodate enable buckets of size 2..
      Results of bit order shuffling: 0 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23
      Write port #0 is in clock domain \clkB.
        Mapped to bram port A1.
      Write port #1 is in clock domain \clkA.
        Failed to map write port #1.
    Mapping to bram type $__ECP5_DP16KD failed.
  Checking rule #4 for bram type $__ECP5_DP16KD (variant 5):
    Bram geometry: abits=14 dbits=1 wports=0 rports=0
    Estimated number of duplicates for more read ports: dups=1
    Metrics for $__ECP5_DP16KD: awaste=0 dwaste=0 bwaste=0 waste=0 efficiency=100
    Rule #4 for bram type $__ECP5_DP16KD (variant 5) accepted.
    Mapping to bram type $__ECP5_DP16KD (variant 5):
      Write port #0 is in clock domain \clkB.
        Mapped to bram port A1.
      Write port #1 is in clock domain \clkA.
        Failed to map write port #1.
    Mapping to bram type $__ECP5_DP16KD failed.
  Checking rule #5 for bram type $__ECP5_DP16KD (variant 1):
    Bram geometry: abits=10 dbits=18 wports=0 rports=0
    Estimated number of duplicates for more read ports: dups=1
  Checking rule #5 for bram type $__ECP5_DP16KD (variant 2):
    Bram geometry: abits=11 dbits=9 wports=0 rports=0
    Estimated number of duplicates for more read ports: dups=1
  Checking rule #5 for bram type $__ECP5_DP16KD (variant 3):
    Bram geometry: abits=12 dbits=4 wports=0 rports=0
    Estimated number of duplicates for more read ports: dups=1
  Checking rule #5 for bram type $__ECP5_DP16KD (variant 4):
    Bram geometry: abits=13 dbits=2 wports=0 rports=0
    Estimated number of duplicates for more read ports: dups=1
  Checking rule #5 for bram type $__ECP5_DP16KD (variant 5):
    Bram geometry: abits=14 dbits=1 wports=0 rports=0
    Estimated number of duplicates for more read ports: dups=1
  Checking rule #6 for bram type $__ECP5_DP16KD (variant 1):
    Bram geometry: abits=10 dbits=18 wports=0 rports=0
    Estimated number of duplicates for more read ports: dups=1
  Checking rule #6 for bram type $__ECP5_DP16KD (variant 2):
    Bram geometry: abits=11 dbits=9 wports=0 rports=0
    Estimated number of duplicates for more read ports: dups=1
  Checking rule #6 for bram type $__ECP5_DP16KD (variant 3):
    Bram geometry: abits=12 dbits=4 wports=0 rports=0
    Estimated number of duplicates for more read ports: dups=1
  Checking rule #6 for bram type $__ECP5_DP16KD (variant 4):
    Bram geometry: abits=13 dbits=2 wports=0 rports=0
    Estimated number of duplicates for more read ports: dups=1
  Checking rule #6 for bram type $__ECP5_DP16KD (variant 5):
    Bram geometry: abits=14 dbits=1 wports=0 rports=0
    Estimated number of duplicates for more read ports: dups=1
  No acceptable bram resources found.

I'm not entirely up to date on the current state of memory inference (afaik a lot of work has been put into it lately), but true dual port memories have historically always been a pain point with yosys.

@Lunaphied
Copy link
Contributor

The current BRAM mapper in Yosys does not support mapping to true dual-port BRAMs. This is part of the improvements that @mwkmwkmwk is working to bring with an improved pass at some point in the future.

@mwkmwkmwk
Copy link
Member

oh, I forgot about this issue, actually

this should be already working on my #3189 branch, would you like to be a test subject?

@rowanG077
Copy link
Author

rowanG077 commented May 4, 2022

@mwkmwkmwk I have tried to use your branch but ecppack fails with the message:

terminate called after throwing an instance of 'std::runtime_error'
  what():  no word named 'IOLOGICA.DELAY.DEL_VALUE'

See this gist for the output of yosys, nextpnr and ecppack one after the other. For reference I used these versions:

Essentially the most recent versions of everything at the time of writing except yosys using your branch.

What I did is take some input from pins route that to a tdp bram ports and route the output to other pins. I have attached the verilog files and the lpf. Exact commands I used can be seen in the gist.

repo.zip

@gatecat
Copy link
Member

gatecat commented May 4, 2022

That error doesn't seem at all related to the BRAM inference patch, I'll have a look.

@rowanG077
Copy link
Author

rowanG077 commented May 4, 2022

So I tried some more and it the error indeed has nothing to do with the blockrams. What I did was instantiate some input and output registers to read/write ram operations from/to. These primitives seem to be the problem.

Anyway I have been playing with it some more more and I have some observations:

@mwkmwkmwk
Copy link
Member

The pass still prints the final mapping rule picked, and using yosys -g dumps a bit more detail about the alternatives considered. Is there some other information you'd want to see in the output?

As for output registers, this pass supports a single register for synchronous ports just like the old one, only now with a few more features on it. Second output register (aka pipelining register) is still not supported (and is out of scope for memory_libmap specifically — it's been decided long ago to do it as a follow-up pass executed later in the flow)

@rowanG077
Copy link
Author

Is that something a person not really familiar with the codebase could work on? I would like to start contributing but honestly it's hard for me to follow the path from requirement to where I need to be in the codebase. Especially since the entire flow is essentially split over multiple repos.

KrystalDelusion pushed a commit to KrystalDelusion/yosys that referenced this issue May 9, 2022
KrystalDelusion pushed a commit to KrystalDelusion/yosys that referenced this issue Jul 6, 2022
KrystalDelusion pushed a commit to KrystalDelusion/yosys that referenced this issue Jul 6, 2022
KrystalDelusion pushed a commit to KrystalDelusion/yosys that referenced this issue Feb 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants