Use Litex generated tests in the soc litex designs for xc7 #1657

tmichalak · 2020-09-07T14:08:40Z

The objective of this PR is to have the netlists for the litex designs used in xc7 soc tests to be auto-generated.
This PR is WIP because so far it has been verified only on the mini_litex design and it depends on other PRs that haven't been merged yet such as the SDC plugin, params plugin integration, SDC plugin integration. It also requires the latest VPR with the never_prune attribute support. Last, but not least there are some SDC commands that have to be handled by Yosys.

litghost

Comments in

xc/xc7/tests/soc/litex/base/CMakeLists.txt

xc/xc7/tests/soc/litex/fixup_xdc.py

xc/xc7/tests/soc/litex/mini/CMakeLists.txt

common/cmake/litex.cmake

litghost · 2020-10-13T18:37:25Z

common/cmake/litex.cmake

+        XDC ${LITEX_GATEWARE}/top.xdc
+    )
+  else()
+    add_fpga_target(


Why do we need to support PCF / SDC? I believe XDC should always work now?

I will add a comment about that. I have seen the following problem. For the minilitex test without DDR and without ethernet, the PLL that is generated by Litex has clocks that will not get used as related to DDR.

This generates issues when creating the SDC, as the final SDC will have clocks that are absent in the VPR netlist.

I believe that this might be yet another fix for the plugin? If a clock does not have any loads, the constraints should not be emitted for that clock.

If the symbiflow flag is provided, LiteX generates the SDC and PCF

I believe that this might be yet another fix for the plugin? If a clock does not have any loads, the constraints should not be emitted for that clock.

I believe that there was talk of VPR demoting a missing clock match to a warning instead of an error. Maybe something to bring up with Vaughn?

Also, I'd argue that it should be accepted without a warning to create a clock on a net with no loads. The key here is have the SDC parser recongize the difference between:

A net that was pruned because it has no loads

A net that doesn't exist at all

The first is not a warning, and the second is likely user error. This seems like something to fix in VPR upstream.

litghost · 2020-10-27T17:40:53Z

xc/xc7/tests/soc/litex/mini/CMakeLists.txt

+  FIXUP_SCRIPT ${FIXUP_XDC}
+  FLAGS
+    --toolchain symbiflow
+  VIVADO_XDC arty_clocks.xdc


Why are we not using the XDC from LiteX? Or does LiteX not supply a clock constraint?

LiteX does supply clock constraints, but the problem was, IIRC, that the output of fasm2bels (Verilog/XDC) does not include certain clock ports present in the LiteX generated XDC, ending up in an error when trying to read it.

I need to double-check this though.

This is still outstanding?

I will open an issue to keep track of this

In general we need an issue tracking the burn down for the remaining "weirdness" around the LiteX support. Examples include this, the fixup_xdc script, the --toolchain symbiflow/SDC stuff from the other converstation, etc.

Oh and the bittool / diff FASM disable too.

litghost

Couple small comments, otherwise LGTM.

Remaining comments:

Signed-off-by: Alessandro Comodi <[email protected]>

Signed-off-by: Tomasz Michalak <[email protected]>

Also added new required submodules Signed-off-by: Alessandro Comodi <[email protected]>

Signed-off-by: Alessandro Comodi <[email protected]>

Also use regexp for the fixup xdc script Signed-off-by: Alessandro Comodi <[email protected]>

Signed-off-by: Alessandro Comodi <[email protected]>

acomodi · 2020-11-04T17:25:42Z

CI went finally green

litghost · 2020-11-04T17:35:40Z

xc/xc7/tests/soc/litex/mini/CMakeLists.txt

+  GENERATE_SCRIPT ${GENERATE_LITEX}
+  FIXUP_SCRIPT ${FIXUP_XDC}
+  FLAGS
+    --toolchain symbiflow


What does --toolchain symbiflow do? I noticed that xc/xc7/tests/soc/litex/mini_ddr/CMakeLists.txt doesn't use --toolchain symbiflow.

This was related to the fact that the PLL generate extra clocks and BUFG which are not used, ending up in clocks generated in the SDC, but they are absent in the cleaned circuit in VPR, therefore the SDC commands for the inexistent generate an error

Please add a row into the burndown to address this issue. If VPR sweeps a netlist, and an SDC that was valid looks invalid, that needs to not be an error. I believe we discussed this a couple times, but it keeps cropping up.

litghost · 2020-11-04T17:36:47Z

The outstanding comments still need to be addressed, but please open follow up PRs.

probot-autolabeler bot added arch-artix7 lang-python Issue uses (or requires) Python language. lang-verilog Issue uses (or requires) Verilog language. lang-xml Issues uses (or requires) XML language. third-party type-utils Issues is related to the scripts inside the repo. type-vpr labels Sep 7, 2020

This was referenced Sep 7, 2020

Add missing SDC commands used in Litex generated XDC chipsalliance/yosys-f4pga-plugins#33

Closed

SDC plugin chipsalliance/yosys-f4pga-plugins#27

Merged

tmichalak force-pushed the use-litex-to-generate-tests branch 2 times, most recently from f6e5903 to 2bb7680 Compare September 16, 2020 10:19

probot-autolabeler bot added the arch-ice40 Issue related to the iCE40 architecture description. label Sep 16, 2020

tmichalak force-pushed the use-litex-to-generate-tests branch 5 times, most recently from 51dfbcd to bfb5564 Compare October 8, 2020 14:05

acomodi force-pushed the use-litex-to-generate-tests branch from bfb5564 to f78acf8 Compare October 12, 2020 18:16

acomodi requested review from mithro and litghost October 12, 2020 18:26

acomodi force-pushed the use-litex-to-generate-tests branch from 72e2264 to d9f0f3b Compare October 12, 2020 18:29

litghost reviewed Oct 12, 2020

View reviewed changes

acomodi requested a review from litghost October 13, 2020 13:49

acomodi force-pushed the use-litex-to-generate-tests branch from 64f2fc1 to 555d81f Compare October 13, 2020 13:51

acomodi changed the title ~~WIP: Use Litex generated tests in the soc litex designs for xc7~~ Use Litex generated tests in the soc litex designs for xc7 Oct 13, 2020

litghost reviewed Oct 13, 2020

View reviewed changes

common/cmake/litex.cmake Show resolved Hide resolved

litghost reviewed Oct 13, 2020

View reviewed changes

common/cmake/litex.cmake Outdated Show resolved Hide resolved

litghost reviewed Oct 13, 2020

View reviewed changes

acomodi added the kokoro:force-run label Oct 13, 2020

symbiflow-kokoro removed the kokoro:force-run label Oct 13, 2020

acomodi force-pushed the use-litex-to-generate-tests branch 2 times, most recently from 4efc05f to 270a0a2 Compare October 26, 2020 09:16

litghost self-requested a review October 27, 2020 17:16

litghost reviewed Oct 27, 2020

View reviewed changes

acomodi mentioned this pull request Oct 27, 2020

Re-enable minilitex_ddr_eth 100T DIFF_FASM test once bittools are fixed #1730

Open

acomodi force-pushed the use-litex-to-generate-tests branch from 137f09a to ac5b5e0 Compare October 30, 2020 08:42

acomodi added the kokoro:force-run label Nov 2, 2020

symbiflow-kokoro removed the kokoro:force-run label Nov 2, 2020

acomodi force-pushed the use-litex-to-generate-tests branch from ac5b5e0 to 832481f Compare November 2, 2020 17:07

acomodi and others added 13 commits November 3, 2020 17:48

xc7: tests: litex: Add necessary submodules

7341959

Signed-off-by: Alessandro Comodi <[email protected]>

submodules: Add requirements for Litex

847632b

Signed-off-by: Alessandro Comodi <[email protected]>

xc7: tests: litex: Have litex netlist generated from Migen

7957c46

Signed-off-by: Alessandro Comodi <[email protected]>

xc7: techmap: Propagate IO_LOC_PAIRS param from OBUFDS to OBUFTDS

d921308

Signed-off-by: Tomasz Michalak <[email protected]>

litex: bump submodules to latest versions

33a63a1

Also added new required submodules Signed-off-by: Alessandro Comodi <[email protected]>

litex: generate all litex designs automatically

63445ce

Signed-off-by: Alessandro Comodi <[email protected]>

run make format and added docstrings

55d4cf2

Signed-off-by: Alessandro Comodi <[email protected]>

fix python linter error

6705a0a

Signed-off-by: Alessandro Comodi <[email protected]>

add litex-hub to have the pre-compiled riscv toolchain from conda

94b7f38

Signed-off-by: Alessandro Comodi <[email protected]>

litex: template for adding litex tests

1cfde11

Also use regexp for the fixup xdc script Signed-off-by: Alessandro Comodi <[email protected]>

address review comments

8c57868

Signed-off-by: Alessandro Comodi <[email protected]>

vivado: have the diff_fasm check performed only by interchange

1908117

Signed-off-by: Alessandro Comodi <[email protected]>

litex: mini_ddr_eth: add link to issue

71ae720

Signed-off-by: Alessandro Comodi <[email protected]>

acomodi force-pushed the use-litex-to-generate-tests branch from 832481f to 71ae720 Compare November 3, 2020 16:48

acomodi requested a review from litghost November 4, 2020 17:25

litghost reviewed Nov 4, 2020

View reviewed changes

litghost merged commit 5f35cd6 into f4pga:master Nov 4, 2020

acomodi deleted the use-litex-to-generate-tests branch March 23, 2021 14:20

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use Litex generated tests in the soc litex designs for xc7 #1657

Use Litex generated tests in the soc litex designs for xc7 #1657

tmichalak commented Sep 7, 2020 •

edited

Loading

litghost left a comment

litghost Oct 13, 2020

acomodi Oct 13, 2020

acomodi Oct 13, 2020

litghost Oct 13, 2020

litghost Oct 13, 2020

litghost Oct 27, 2020 •

edited

Loading

acomodi Oct 27, 2020

litghost Nov 4, 2020

acomodi Nov 4, 2020

litghost Nov 4, 2020 •

edited

Loading

litghost Nov 4, 2020

litghost left a comment •

edited

Loading

acomodi commented Nov 4, 2020

litghost Nov 4, 2020

acomodi Nov 4, 2020

litghost Nov 4, 2020 •

edited

Loading

litghost commented Nov 4, 2020

Use Litex generated tests in the soc litex designs for xc7 #1657

Use Litex generated tests in the soc litex designs for xc7 #1657

Conversation

tmichalak commented Sep 7, 2020 • edited Loading

litghost left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

litghost Oct 27, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

litghost Nov 4, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

litghost left a comment • edited Loading

Choose a reason for hiding this comment

acomodi commented Nov 4, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

litghost Nov 4, 2020 • edited Loading

Choose a reason for hiding this comment

litghost commented Nov 4, 2020

tmichalak commented Sep 7, 2020 •

edited

Loading

litghost Oct 27, 2020 •

edited

Loading

litghost Nov 4, 2020 •

edited

Loading

litghost left a comment •

edited

Loading

litghost Nov 4, 2020 •

edited

Loading