DRAM fix vendor tools tests #1268

acomodi · 2020-01-17T14:45:33Z

Signed-off-by: Alessandro Comodi acomodi@antmicro.com

litghost

Rather than disabling the dram test, let's just fix the fasm2bels bug?

acomodi · 2020-01-20T17:52:20Z

Rather than disabling the dram test, let's just fix the fasm2bels bug?

Done. I think the issue was that there were orphan sinks due to the absence of routing to the INT tiles of the harness. By changing the designs, the problem should disappear.

litghost · 2020-01-21T06:12:24Z

FAILED: cd /tmpfs/src/github/symbiflow-arch-defs-presubmit-xc7-vendor/build/xc7/tests/dram && /usr/bin/cmake -E env PYTHONPATH=/tmpfs/src/github/symbiflow-arch-defs-presubmit-xc7-vendor/build/env/conda/lib/python3.7/site-packages:/tmpfs/src/github/symbiflow-arch-defs-presubmit-xc7-vendor/third_party/prjxray:/tmpfs/src/github/symbiflow-arch-defs-presubmit-xc7-vendor/third_party/prjxray/third_party/fasm:/tmpfs/src/github/symbiflow-arch-defs-presubmit-xc7-vendor/xc7:/tmpfs/src/github/symbiflow-arch-defs-presubmit-xc7-vendor/utils /tmpfs/src/github/symbiflow-arch-defs-presubmit-xc7-vendor/build/env/conda/bin/python3 -mfasm2bels --db_root /tmpfs/src/github/symbiflow-arch-defs-presubmit-xc7-vendor/third_party/prjxray-db/artix7 --rr_graph /tmpfs/src/github/symbiflow-arch-defs-presubmit-xc7-vendor/build/xc7/archs/artix7/devices/rr_graph_xc7a50t-basys3_test.rr_graph.real.xml --route /tmpfs/src/github/symbiflow-arch-defs-presubmit-xc7-vendor/build/xc7/tests/dram/dram_4_32x1s/artix7-xc7a50t-basys3-roi-virt-xc7a50t-basys3-test/top.route --iostandard_defs /tmpfs/src/github/symbiflow-arch-defs-presubmit-xc7-vendor/build/xc7/tests/dram/dram_4_32x1s/artix7-xc7a50t-basys3-roi-virt-xc7a50t-basys3-test/top.eblif.iostandard.json --bitread /tmpfs/src/github/symbiflow-arch-defs-presubmit-xc7-vendor/build/third_party/prjxray/tools/bitread --bit_file /tmpfs/src/github/symbiflow-arch-defs-presubmit-xc7-vendor/build/xc7/tests/dram/dram_4_32x1s/artix7-xc7a50t-basys3-roi-virt-xc7a50t-basys3-test/top.bit --fasm_file /tmpfs/src/github/symbiflow-arch-defs-presubmit-xc7-vendor/build/xc7/tests/dram/dram_4_32x1s/artix7-xc7a50t-basys3-roi-virt-xc7a50t-basys3-test/top.bit.fasm --pcf /tmpfs/src/github/symbiflow-arch-defs-presubmit-xc7-vendor/build/xc7/tests/common/basys3.pcf --eblif /tmpfs/src/github/symbiflow-arch-defs-presubmit-xc7-vendor/build/xc7/tests/dram/dram_4_32x1s/artix7-xc7a50t-basys3-roi-virt-xc7a50t-basys3-test/top.eblif --top top /tmpfs/src/github/symbiflow-arch-defs-presubmit-xc7-vendor/build/xc7/tests/dram/dram_4_32x1s/artix7-xc7a50t-basys3-roi-virt-xc7a50t-basys3-test/top_bit.v /tmpfs/src/github/symbiflow-arch-defs-presubmit-xc7-vendor/build/xc7/tests/dram/dram_4_32x1s/artix7-xc7a50t-basys3-roi-virt-xc7a50t-basys3-test/top_bit.v.tcl --part xc7a35tcpg236-1 --connection_database /tmpfs/src/github/symbiflow-arch-defs-presubmit-xc7-vendor/build/xc7/archs/artix7/devices/xc7a50t-basys3-roi-virt/channels.db
Traceback (most recent call last):
  File "/tmpfs/src/github/symbiflow-arch-defs-presubmit-xc7-vendor/build/env/conda/lib/python3.7/runpy.py", line 193, in _run_module_as_main
    "__main__", mod_spec)
  File "/tmpfs/src/github/symbiflow-arch-defs-presubmit-xc7-vendor/build/env/conda/lib/python3.7/runpy.py", line 85, in _run_code
    exec(code, run_globals)
  File "/tmpfs/src/github/symbiflow-arch-defs-presubmit-xc7-vendor/xc7/fasm2bels/__main__.py", line 4, in <module>
    main()
  File "/tmpfs/src/github/symbiflow-arch-defs-presubmit-xc7-vendor/xc7/fasm2bels/fasm2bels.py", line 375, in main
    process_tile(top, tile, tile_features)
  File "/tmpfs/src/github/symbiflow-arch-defs-presubmit-xc7-vendor/xc7/fasm2bels/fasm2bels.py", line 98, in process_tile
    PROCESS_TILE[tile_type](top.conn, top, tile, tile_features)
  File "/tmpfs/src/github/symbiflow-arch-defs-presubmit-xc7-vendor/xc7/fasm2bels/clb_models.py", line 1177, in process_clb
    process_slice(top, slices[s])
  File "/tmpfs/src/github/symbiflow-arch-defs-presubmit-xc7-vendor/xc7/fasm2bels/clb_models.py", line 879, in process_slice
    site.add_internal_source(ram32, 'O', lut + "O6")
  File "/tmpfs/src/github/symbiflow-arch-defs-presubmit-xc7-vendor/xc7/fasm2bels/verilog_modeling.py", line 864, in add_internal_source
    bel.connections[bel_pin] = wire_name
AttributeError: 'list' object has no attribute 'connections'

acomodi · 2020-01-21T15:02:22Z

@litghost I have tried to fix the issue with drams, but it seems that the problem is more complex than I first thought.
For instance, there are situations in which is not possible to determine from fasm, which dram needs to be used (dram_2_128x1s and dram_1_128x1d), as they are the same (at least when it comes to the CLBs features).

There are other different situations for which some sources are not being instantiated:

[100%] Generating dram_1_256x1s/artix7-xc7a50t-basys3-roi-virt-xc7a50t-basys3-test/top_bit.v
Traceback (most recent call last):
  File "/home/alessandro/projects/symbiflow/tmp/symbiflow-arch-defs/build/env/conda/lib/python3.7/runpy.py", line 193, in _run_module_as_main
    "__main__", mod_spec)
  File "/home/alessandro/projects/symbiflow/tmp/symbiflow-arch-defs/build/env/conda/lib/python3.7/runpy.py", line 85, in _run_code
    exec(code, run_globals)
  File "/home/alessandro/projects/symbiflow/tmp/symbiflow-arch-defs/xc7/fasm2bels/__main__.py", line 4, in <module>
    main()
  File "/home/alessandro/projects/symbiflow/tmp/symbiflow-arch-defs/xc7/fasm2bels/fasm2bels.py", line 375, in main
    process_tile(top, tile, tile_features)
  File "/home/alessandro/projects/symbiflow/tmp/symbiflow-arch-defs/xc7/fasm2bels/fasm2bels.py", line 98, in process_tile
    PROCESS_TILE[tile_type](top.conn, top, tile, tile_features)
  File "/home/alessandro/projects/symbiflow/tmp/symbiflow-arch-defs/xc7/fasm2bels/clb_models.py", line 1185, in process_clb
    process_slice(top, slices[s])
  File "/home/alessandro/projects/symbiflow/tmp/symbiflow-arch-defs/xc7/fasm2bels/clb_models.py", line 1018, in process_slice
    site.connect_internal(bel, 'S[{}]'.format(idx), source)
  File "/home/alessandro/projects/symbiflow/tmp/symbiflow-arch-defs/xc7/fasm2bels/verilog_modeling.py", line 881, in connect_internal
    assert source in self.internal_sources, source
AssertionError: AO6

IMO this should be solved, but there are other higher priority things to be done, I have temporarily disabled the faulty vendor tool tests and enabled the ones that do pass.

litghost · 2020-01-21T19:02:41Z

dram_2_128x1s and dram_1_128x1d

In both of these cases the output of fasm2bels should actually probably be a RAM64M + 2x F7MUX, which has no ambiguity.

litghost · 2020-01-21T19:06:24Z

xc7/tests/dram/CMakeLists.txt

-dram_tests(1 32m 8 17)
-dram_tests(1 64m 4 17)
+# dram_tests(<dram_num_instances> <dram_mode> <syn_outpad_assert_usage> <syn_inpad_assert_usage> <enable_vivado_target>)
+dram_tests(4 32x1s 4 11 N)


Why are the 32x1s/32x1d failing? Please fix these two.

Regarding 32x1d it is currently enabled and completes correctly (dram_2_32x1d_vivado_diff_fasm).
As for 32x1s, vivado fails with the following error:

ERROR: [Vivado 12-2285] Cannot set LOC property of instance 'CLBLM_R_X3Y116_SLICE_X2Y116_RAM32X1S_C_0', for bel C5LUT Two RAM symbols can not share a LUT site if two RAM symbols are not also sharing the D LUT site. There are fewer than two RAM symbols in the D LUT site, for bel D5LUT Both D6LUT and D5LUT are occupied by a RAM. This prevents LUT xxx to go to site yyy Resolution: When using BEL constraints, ensure the BEL constraints are defined before the LOC constraints to avoid conflicts at a given site.

I have tried to adopt the Resolution suggested, but the same failure is produced.

acomodi · 2020-01-22T12:45:56Z

@litghost I have seen that Vivado implements the 32x1s DRAM mode in a different way than done in Symbiflow. In fact here is the fasm I get:

Vivado:

CLBLM_R_X3Y15.SLICEL_X1.NOCLKINV
CLBLM_R_X3Y15.SLICEL_X1.PRECYINIT.C0
CLBLM_R_X3Y15.SLICEM_X0.ALUT.DI1MUX.BDI1_BMC31
CLBLM_R_X3Y15.SLICEM_X0.ALUT.INIT[33]
CLBLM_R_X3Y15.SLICEM_X0.ALUT.RAM
CLBLM_R_X3Y15.SLICEM_X0.ALUT.SMALL
CLBLM_R_X3Y15.SLICEM_X0.BLUT.DI1MUX.DI_CMC31
CLBLM_R_X3Y15.SLICEM_X0.BLUT.INIT[33]
CLBLM_R_X3Y15.SLICEM_X0.BLUT.RAM
CLBLM_R_X3Y15.SLICEM_X0.BLUT.SMALL
CLBLM_R_X3Y15.SLICEM_X0.CLUT.DI1MUX.DI_DMC31
CLBLM_R_X3Y15.SLICEM_X0.CLUT.INIT[33]
CLBLM_R_X3Y15.SLICEM_X0.CLUT.RAM
CLBLM_R_X3Y15.SLICEM_X0.CLUT.SMALL
CLBLM_R_X3Y15.SLICEM_X0.DLUT.INIT[33]
CLBLM_R_X3Y15.SLICEM_X0.DLUT.RAM
CLBLM_R_X3Y15.SLICEM_X0.DLUT.SMALL
CLBLM_R_X3Y15.SLICEM_X0.NOCLKINV
CLBLM_R_X3Y15.SLICEM_X0.PRECYINIT.C0
CLBLM_R_X3Y15.SLICEM_X0.WEMUX.CE

SymbiFlow:

CLBLM_R_X3Y116.SLICEL_X1.NOCLKINV
CLBLM_R_X3Y116.SLICEL_X1.PRECYINIT.C0
CLBLM_R_X3Y116.SLICEM_X0.ALUT.DI1MUX.BDI1_BMC31
CLBLM_R_X3Y116.SLICEM_X0.BLUT.DI1MUX.DI_CMC31
CLBLM_R_X3Y116.SLICEM_X0.CLUT.DI1MUX.CI
CLBLM_R_X3Y116.SLICEM_X0.CLUT.INIT[33:0] = 34'b1000000000000000000000000000000010
CLBLM_R_X3Y116.SLICEM_X0.CLUT.RAM
CLBLM_R_X3Y116.SLICEM_X0.CLUT.SMALL
CLBLM_R_X3Y116.SLICEM_X0.COUTMUX.O5
CLBLM_R_X3Y116.SLICEM_X0.DLUT.INIT[33:0] = 34'b1000000000000000000000000000000010
CLBLM_R_X3Y116.SLICEM_X0.DLUT.RAM
CLBLM_R_X3Y116.SLICEM_X0.DLUT.SMALL
CLBLM_R_X3Y116.SLICEM_X0.DOUTMUX.O5
CLBLM_R_X3Y116.SLICEM_X0.NOCLKINV
CLBLM_R_X3Y116.SLICEM_X0.PRECYINIT.C0
CLBLM_R_X3Y116.SLICEM_X0.WEMUX.CE

ALUT and BLUT are not used in SymbiFlow, and the stuff is packed as it were a 64x1s. This actually happens because the {n}_dram.pb_type does not have a way to pack a single 32x1s. In fact, if there are two different dram_32x1s to be packed, they will be packed in the same LUT, using both the D05 and D06 outputs, instead of getting in two different LUTs, therefore using D06 outputs only.

I have added an additional pb_type, with an additional model, so to have the 32_SINGLE_PORT packing only a single 32x1s dram at a time, using all the 4 different LUTs for the dram_4_32x1s test

Does this make sense?

Signed-off-by: Alessandro Comodi <acomodi@antmicro.com>

litghost · 2020-01-22T18:17:31Z

So I think the problem is the order that Vivado and Symbiflow pack 32X1S's. I'm guessing (without strong evidence) that Vivado packs:

DLUT.O6
CLUT.O6
BLUT.O6
ALUT.O6
DLUT.O5
CLUT.O5
BLUT.O5
ALUT.O5

And symbiflow was packing:

DLUT.O6
DLUT.O5
CLUT.O6
CLUT.O5
BLUT.O6
BLUT.O5
ALUT.O6
ALUT.O5

We probably need a minitest to verify.

litghost · 2020-01-22T18:19:50Z

I have added an additional pb_type, with an additional model, so to have the 32_SINGLE_PORT packing only a single 32x1s dram at a time, using all the 4 different LUTs for the dram_4_32x1s test

The new structure allows only 4 32x1s per SLICEM, which defeats the purpose of supporting 32-bit LUT-RAMs. If we cannot accomidate 8 32x1s, we should not advertise support for them. I believe RAM32M is working though, which can be used to implement all variants.

acomodi · 2020-01-23T08:37:04Z

I have added an additional pb_type, with an additional model, so to have the 32_SINGLE_PORT packing only a single 32x1s dram at a time, using all the 4 different LUTs for the dram_4_32x1s test

The new structure allows only 4 32x1s per SLICEM, which defeats the purpose of supporting 32-bit LUT-RAMs. If we cannot accomidate 8 32x1s, we should not advertise support for them. I believe RAM32M is working though, which can be used to implement all variants.

So, As far as I have seen, the dram_8_32x1s, if run on Vivado, packs the LUTS in two different CLBs using only O6 outputs (this happens without any kind of user constraints). To verify that I have simply built the verilog design with Vivado and analyze the implemented results.

litghost · 2020-01-23T16:50:45Z

xc7/primitives/slicem/Ndram/d_dram.pb_type.xml

+     <xi:include href="spram32.pb_type.xml" xpointer="xpointer(pb_type/metadata/child::node())" />
+     <meta name="fasm_params">
+      INIT[63:32] = INIT_00
+      INIT[31:0] = INIT_ZERO


Can you add an XML comment explaining that this is known to be wrong? Open a new issue about fixing the RAM32's in the future.

This was referenced Jan 17, 2020

32-bit LUT-RAM are not correctly implemented #1269

Open

Add penalty cost switch to avoid wrong routes #1267

Merged

acomodi requested a review from litghost January 17, 2020 18:10

litghost suggested changes Jan 17, 2020

View reviewed changes

probot-autolabeler bot added the lang-verilog Issue uses (or requires) Verilog language. label Jan 20, 2020

acomodi changed the title ~~dram test: disable vivado target~~ DRAM fix vendor tools tests Jan 20, 2020

acomodi requested a review from litghost January 20, 2020 17:56

acomodi force-pushed the fix-dram-tests-vendor-tools branch from 6f844a4 to 1b9fdc9 Compare January 21, 2020 14:54

probot-autolabeler bot added the lang-python Issue uses (or requires) Python language. label Jan 21, 2020

litghost reviewed Jan 21, 2020

View reviewed changes

probot-autolabeler bot added lang-xml Issues uses (or requires) XML language. type-vpr labels Jan 22, 2020

acomodi added 7 commits January 22, 2020 15:12

dram test: disable vivado target

8686233

Signed-off-by: Alessandro Comodi <acomodi@antmicro.com>

xc7: fix dram vendor tools test

1398932

Signed-off-by: Alessandro Comodi <acomodi@antmicro.com>

xc7: dram: disabled some faulty vivado targets

924449c

Signed-off-by: Alessandro Comodi <acomodi@antmicro.com>

fasm2bels: fixed LUT INIT shift

c57237b

Signed-off-by: Alessandro Comodi <acomodi@antmicro.com>

dram: added 32 single mode dram to have fix 32x1s test

500a81d

Signed-off-by: Alessandro Comodi <acomodi@antmicro.com>

dram: fix dram_8_32x1s test

6ffa298

Signed-off-by: Alessandro Comodi <acomodi@antmicro.com>

dram: fixed INIT values for 32_SINGLE_PORT

1a1f9cd

Signed-off-by: Alessandro Comodi <acomodi@antmicro.com>

acomodi force-pushed the fix-dram-tests-vendor-tools branch from 230d936 to e3d73c0 Compare January 22, 2020 15:20

dram: added pack_patterns to correctly pack drams

0d07454

Signed-off-by: Alessandro Comodi <acomodi@antmicro.com>

acomodi force-pushed the fix-dram-tests-vendor-tools branch from e3d73c0 to 0d07454 Compare January 22, 2020 15:25

acomodi requested a review from litghost January 22, 2020 15:40

litghost reviewed Jan 23, 2020

View reviewed changes

litghost merged commit 83d00eb into f4pga:master Jan 23, 2020

litghost mentioned this pull request Jan 23, 2020

[xc7] 128-bit and 256-bit LUT-RAM vendor tests are disabled #1275

Closed

acomodi deleted the fix-dram-tests-vendor-tools branch January 27, 2020 15:01

lpawelcz mentioned this pull request Dec 29, 2021

tests: fix ASSERT_USAGE tests #2359

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

DRAM fix vendor tools tests #1268

DRAM fix vendor tools tests #1268

acomodi commented Jan 17, 2020

litghost left a comment

acomodi commented Jan 20, 2020

litghost commented Jan 21, 2020

acomodi commented Jan 21, 2020 •

edited

litghost commented Jan 21, 2020

litghost Jan 21, 2020

acomodi Jan 21, 2020

acomodi commented Jan 22, 2020

litghost commented Jan 22, 2020 •

edited

litghost commented Jan 22, 2020

acomodi commented Jan 23, 2020

litghost Jan 23, 2020

DRAM fix vendor tools tests #1268

DRAM fix vendor tools tests #1268

Conversation

acomodi commented Jan 17, 2020

litghost left a comment

Choose a reason for hiding this comment

acomodi commented Jan 20, 2020

litghost commented Jan 21, 2020

acomodi commented Jan 21, 2020 • edited

litghost commented Jan 21, 2020

litghost Jan 21, 2020

Choose a reason for hiding this comment

acomodi Jan 21, 2020

Choose a reason for hiding this comment

acomodi commented Jan 22, 2020

litghost commented Jan 22, 2020 • edited

litghost commented Jan 22, 2020

acomodi commented Jan 23, 2020

litghost Jan 23, 2020

Choose a reason for hiding this comment

acomodi commented Jan 21, 2020 •

edited

litghost commented Jan 22, 2020 •

edited