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

Symbiflow-classroom: "Message: Failed to find matching architecture model for 'RAMB36E1'" #262

Closed
nelsobe opened this issue Mar 8, 2022 · 24 comments

Comments

@nelsobe
Copy link
Contributor

nelsobe commented Mar 8, 2022

SYMBIFLOW-CLASSROOM-PROJECT

Using original Yosys frontend.

When compiling a design containing a RAMB36E1 primitive instantiation we get the following error:

Message: Failed to find matching architecture model for 'RAMB36E1'
`

The error comes from the bramMacro.v file which is trying to instantiate a RAMB36E1 primitive.

We find this curious because in the .../opt/f4pga/xc7/install/share/symbiflow/techmaps/xc7_vpr/techmap/cellsw_map.v file the RAMB36E1 is clearly defined.

This behavior is identical with both a symbiflow-examples clone from early January as well as with a f4pga-examples clone today.

@mkurc-ant @litghost @acomodi Any ideas if it is an error on our end or if these primitives are not supported?

///////////////////////////////////////////////////////////////////////////
// 
// bramMacro.v
//
// Macro for BRAM module instantiation (reduce ports)
//
///////////////////////////////////////////////////////////////////////////

module bramMacro (clka, clkb, a_addr,b_addr,a_we,a_din,a_dout,b_dout);

  input wire clka;
  input wire clkb;
  input [11:0] a_addr;
  input [11:0] b_addr;
  input a_we;
  input [7:0] a_din;
  output [7:0] a_dout;
  output [7:0] b_dout;

  wire [15:0] a_addr_i, b_addr_i;
  wire [31:0] a_din_i;
  assign a_addr_i = {1'b1,a_addr,3'h0};
  assign b_addr_i = {1'b1,b_addr,3'h0};
  assign a_din_i = {24'h0,a_din};
  wire [31:0] a_dout_i, b_dout_i;
  assign a_dout = a_dout_i[7:0];
  assign b_dout = b_dout_i[7:0];
  wire [3:0] a_we_i;
  assign a_we_i = {3'b000,a_we};

  // Instance bram module
  RAMB36E1 bram(
          .DOADO(a_dout_i),
          .DOBDO(b_dout_i),
          .CASCADEINA(1'b1),
          .CASCADEINB(1'b1),
          .ADDRARDADDR(a_addr_i),
          .ADDRBWRADDR(b_addr_i),
          .CLKARDCLK(clka),
          .CLKBWRCLK(clkb),
          .DIADI(a_din_i),
          .DIBDI(32'h0),
          .DIPADIP(4'h0),
          .DIPBDIP(4'h0),
          .ENARDEN(1'b1),
          .ENBWREN(1'b1),
          .INJECTDBITERR(1'b0),
          .INJECTSBITERR(1'b0),
          .REGCEAREGCE(1'b0),
          .REGCEB(1'b0),
          .RSTRAMARSTRAM(1'b0),
          .RSTRAMB(1'b0),
          .RSTREGARSTREG(1'b0),
          .RSTREGB(1'b0),
          .WEA(a_we_i),
          .WEBWE(8'b0)
        );

    defparam bram.READ_WIDTH_A = 9;
    defparam bram.READ_WIDTH_B = 9;
    defparam bram.WRITE_WIDTH_A = 9;
    defparam bram.WRITE_WIDTH_B = 9;
    defparam bram.RAM_MODE = "TDP";
    defparam bram.DOA_REG = 0;
    defparam bram.DOB_REG = 0;

endmodule
@nelsobe
Copy link
Contributor Author

nelsobe commented Mar 8, 2022

Also, not sure why, but I cannot assign labels to this issue like I can with previous symbiflow repos...

@nelsobe nelsobe changed the title Message: Failed to find matching architecture model for 'RAMB36E1' Symbiflow-classroom: "Message: Failed to find matching architecture model for 'RAMB36E1'" Mar 8, 2022
@nelsobe
Copy link
Contributor Author

nelsobe commented Mar 9, 2022

Is there a way to make a list of the primitives NOT supported by the toolchain?

@mithro
Copy link
Contributor

mithro commented Mar 9, 2022

FYI - The newer interchange stuff has a spreadsheet @ https://docs.google.com/spreadsheets/d/1hF7-PpOpa35uik2C2-RYSpY-oFJS2SpbVJj_SF_IAF0/edit#gid=0 which tracks the status...

@mithro
Copy link
Contributor

mithro commented Mar 9, 2022

I found a spreadsheet at https://docs.google.com/spreadsheets/d/1RlSiC9RJrrswtYfE5DnPGegpP2v7pautYixhwMAKyN0/edit#gid=0 -- Don't know where that comes from.

@nelsobe
Copy link
Contributor Author

nelsobe commented Mar 9, 2022

The interchange spreadsheet shows tests for the RAMB36E1. So, the problem we are seeing might lie elsewhere...?

@mithro
Copy link
Contributor

mithro commented Mar 9, 2022

The newer interchange format stuff is not used here yet.

@nelsobe
Copy link
Contributor Author

nelsobe commented Mar 9, 2022

Thanks! I looked and the first spreadsheet has tests for RAMB36E1, suggesting it is supported. But, it would seem not. So, not sure where the error lies. When I recoded the verilog to be behavioral, Yosys turned it into two RAMB18's and that goes through fine.

@acomodi
Copy link
Contributor

acomodi commented Mar 11, 2022

@nelsobe looking at the techmapping files, the RAMB36E1 primitive should be supported. One thing that comes to my mind is that there is a techmap failure and the RAMB36E1 does not get translated in the "VPR-readable" RAMB36E1_PRIM.

Can you please upload the yosys log file if present? It should be called something like <test_name>_synth.log or similar. It may contain some information on whether techmap failed.

An additional note is that the following inputs are actually missing from the techmap instance interface:

          .INJECTDBITERR(1'b0),
          .INJECTSBITERR(1'b0),

You may try to remove these two (which are also assigned to GND and should be ok to explicitly add them to the BRAM instance.

@nelsobe
Copy link
Contributor Author

nelsobe commented Mar 11, 2022

@mkurc-ant Here is the synthesis log...

multicycle_iosystem_synth.log

@nelsobe
Copy link
Contributor Author

nelsobe commented Mar 25, 2022

@mkurc-ant
I have gone through the techmapper and compared ports in our code with the ports in the techmapper code.

The missing inputs in the techmapper are:
INJECTDBITERR
INJECTSBITERR
CASCADEINA
CASCADEINB

It would seem that, since the techmapper is currently incomplete, it would be helpful to output an error message suggesting to the user what versions of a given primitive the system knows about and the ports it contains along with a suggestion to the user that one fix might be to double check the ports. In our case that would have saved a bunch of time since the error message we did get wasn't that helpful.

Thanks.

@nelsobe
Copy link
Contributor Author

nelsobe commented Mar 25, 2022

@mkurc-ant Even with the above ports commented out in our code, right at the end of the process we get a FASM write error. Any thoughts on what that might be? I am not sure how to tell if our code is bad or it is the tools.

I have attached the synthesis log and the eblif file. Are there other files that might be helpful?

Here is the error:

# Annotating rr_node with routed nets
# Annotating rr_node with routed nets took 0.00 seconds (max_rss 3968.5 MiB, delta_rss +0.0 MiB)
Found 0 mismatches between routing and packing results.
Fixed 0 routing traces due to mismatch between routing and packing results.
Synchronize the packed netlist to routing optimization took 0.00 seconds (max_rss 3968.5 MiB, delta_rss +0.0 MiB)
Writing Implementation FASM: multicycle_iosystem.fasm
Error 1: 
Type: Other
File: /home/runner/work/conda-eda/conda-eda/workdir/conda-env/conda-bld/vtr-optimized_1642201887903/work/utils/fasm/src/parameters.cpp
Line: 21
Message: When emitting FASM for parameter DOB_REG, expected width of 1 got width of 32, value = "00000000000000000000000000000000".
make: *** [/home/nelson/f4pga-examples/common/common.mk:68: /home/nelson/220-nelsobe/Labs/dbg2/build/basys3/multicycle_iosystem.fasm] Error 1

multicycle.zip

@acomodi
Copy link
Contributor

acomodi commented Mar 25, 2022

It would seem that, since the techmapper is currently incomplete, it would be helpful to output an error message suggesting to the user what versions of a given primitive the system knows about and the ports it contains along with a suggestion to the user that one fix might be to double check the ports. In our case that would have saved a bunch of time since the error message we did get wasn't that helpful.

@nelsobe I believe that outputting such an error during synthesis may not be so trivial, but we'll see if something can be done. The fact is that this is not an error per se, but rather it is a situation where there is no techmap available for the given cell because the I/Os do not match, so for yosys, this cell should just be left as it is. No error is generated because there is no real failure during the tachmap pass.

I have attached the synthesis log and the eblif file. Are there other files that might be helpful?
I think also the original verilog design can be helpful to understand what is going on here.

I suppose that the DOB_REG contains 0 instead of 1'b0. In the first case it might get translated into a bit representation of a 32-bit integer, while this parameter should be of width == 1.

@nelsobe
Copy link
Contributor Author

nelsobe commented Mar 25, 2022

@acomodi Thanks for the response. My take is that if the error is on the user's part iin terms of specifying an incorrect port it may not be so necessary to output a very elaborate error message. But when the techmapper is missing ports it is very confusing (and discouraging) to users to have code that goes through Vivado without error refuse to compile...

I have changed the DOB_REG parameter to 1'b0 and it goes through the tools without error. In summary, only by making 5 changes to my simple RAMB36E1 instantiation was I finally able to get it to go through the tools. Yet it went through Vivado without error unchanged. Not a good situation for users.

Should we expect Vivado-compilable code to always work in F4PGA? This is a diffiicult question but this particular case seems like it merits fixing.

@acomodi
Copy link
Contributor

acomodi commented Apr 5, 2022

My take is that if the error is on the user's part iin terms of specifying an incorrect port it may not be so necessary to output a very elaborate error message. But when the techmapper is missing ports it is very confusing (and discouraging) to users to have code that goes through Vivado without error refuse to compile...

@nelsobe We'd probably need to go through the techmap definitions to catch some possible inconsistencies with the top level IOs. The problem is that, if a primitive instance has more IO ports defined w.r.t. the ones defined in the techmap, yosys will not even try to perform the techmap pass for that instance, and skip it completely. This is not something that can be caught easily.

I have changed the DOB_REG parameter to 1'b0 and it goes through the tools without error.

This on the other hand might be something that could potentially be caught during techmapping, but I'd need to double check it. Basically we may emit a _TECHMAP_ERROR_ if the width of the input parameter is exceeded.

Should we expect Vivado-compilable code to always work in F4PGA?

I believe this is one of the goals, to have unmodified code that can flawlessly run in both closed and open source tools.

@acomodi
Copy link
Contributor

acomodi commented Apr 5, 2022

@nelsobe I have opened a PR in f4pga-arch-defs: f4pga/f4pga-arch-defs#2566.
Once it is merged and a new package generated, we can update f4pga-examples.

That PR should fix this issue, basically allowing you not to add those 5 changes to the RAMB36E1 instance

@nelsobe
Copy link
Contributor Author

nelsobe commented Apr 14, 2022

@acomodi I just now did a clean reinstall and am still getting the same error - has the fix been incorporated into f4pga-examples?

@acomodi
Copy link
Contributor

acomodi commented Apr 14, 2022

@nelsobe Yes, it has already been integrated, I'll try to reproduce it as well.

What error of the two are you getting?

@nelsobe
Copy link
Contributor Author

nelsobe commented Apr 14, 2022

@acomodi Still getting this:

Error 1: 
Type: Blif file
File: multicycle_iosystem.eblif
Line: 15585
Message: Failed to find matching architecture model for 'RAMB36E1'

And, this was with the 4 parameters from above and with the DOB_REG being just the constant 0.

@nelsobe
Copy link
Contributor Author

nelsobe commented Apr 14, 2022

@acomodi

The only way to get it to succeed is to comment out the 4 parameters:
INJECTDBITERR
INJECTSBITERR
CASCADEINA
CASCADEINB

and to convert the DOB_REG to 1'b0.

Then it works. So, it seems that nothing has changed yet - did the fixes you made get merged yet?

@acomodi
Copy link
Contributor

acomodi commented Apr 15, 2022

@nelsobe The fix should actually be there already, I have tested locally and everything seems to be working as expected.

Can you confirm that the correct package (https://storage.googleapis.com/symbiflow-arch-defs/artifacts/prod/foss-fpga-tools/symbiflow-arch-defs/continuous/install/20220406-185509/symbiflow-arch-defs-install-3ef4188.tar.xz) has been installed?

The linked package contains the updated cells_map.v file which adds the missing IO signals as well as fixes the DOA and DOB parameters to be one-bit wide.

@nelsobe
Copy link
Contributor Author

nelsobe commented Apr 15, 2022

@acomodi

The docs for f4pga-examples are still pointing to older packages so, no, I was not using that particular package. How would a user know to get newer packages if the docs are out of date? Is there a list somewhere else?

I think the install process needs to be changed somehow - we have spent a lot of time this semester trying to figure out when things have been merged and then trying to figure out what packages to use. Any thoughts on how to make it a bit more robust?

Thanks.

@nelsobe
Copy link
Contributor Author

nelsobe commented Apr 15, 2022

@acomodi I have just finished testing with the package you mention above and it is now working. Thanks!

@umarcor
Copy link
Contributor

umarcor commented May 19, 2022

@nelsobe can we close this issue and discuss the enhancements to the documentation in #292 and #299 ?

@nelsobe
Copy link
Contributor Author

nelsobe commented Jun 6, 2022

Our student testing indicates this issue is fixed as of 6/6/2022 using the latest f4pga-examples tools distribution.

Closing issue.

@nelsobe nelsobe closed this as completed Jun 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants