Universal Yosys module #581

kboronski-ant · 2022-07-05T21:05:18Z

The synth module is quite troublesome, because its outputs are dependent on the contents of a Tcl script used by Yosys. This PR is an attempt to solve this issue by replacing this module with an universal Yosys module.

Content description

The approach is to create a binding mechanism for variables used in Tcl script that allows f4pga to inspect the I/O of the script. This is achieved by replacing direct references to environmental variables with calls to f4pga command in Tcl script. f4pga provides its implementation of Tcl f4pga command that gathers metadata from the arguments and creates dummy bindings for those variables. The Tcl script is then executed by a Tcl interpreter within f4pga using that implementation when instantiating the module. This allows the module to inspect I/O of the script and present stage dependencies and values. Then when the script gets passed to yosys, it gets prefixed with an alternative implementation of f4pga command, one that reads environmental variables. Those variables are provided by the yosys module during execution stage, or by legacy wrappers*.

* Legacy wrappers require changes in names of environmental variables in order to work with this.

Features

Inspect Tcl scripts for I/O
Can be used with legacy scripts (after modifying them a little bit)
Manage temporary files used by Tcl scripts (creates bindings to unique file names in a temporary directory and performs cleanup after script execution)
Run yosys in unpolluted environment
I/O can be determined by logic within Tcl script
Supports optional takes and product descriptions
Produce Yosys log on demand

Caveats

Yosys scripts (non-tcl) are not supported
Running the modified Tcl scripts in yosys requires running the f4pga_exec.tcl script first. If anybody is using those scripts in a standalone manner, this could be a confusing change.
Scripts need to behave in a deterministic manner and flow control should be independent from file paths.

Work being done

f4pga/common_modules/yosys.py

umarcor · 2022-08-04T19:06:13Z

Changing this to a draft, to be rebased after #604 is merged.

umarcor · 2022-08-06T11:32:52Z

To be complemented with f4pga/f4pga-arch-defs#2851.

kboronski-ant · 2022-09-06T17:41:40Z

Marking as ready for review, as for now the most effective approach is probably to keep a separate copy of Tcl scripts to use with legacy tooling (arch-defs), until we figure out how to fully transition to using f4pga build.

f4pga/flows/platforms.yml

umarcor · 2022-09-06T19:24:07Z

f4pga/flows/platforms.yml

+        yosys_plugins:
+          - ql-iob
+          - ql-qlf
+    split_inouts:


See comment above.

umarcor · 2022-09-07T12:01:02Z

f4pga/flows/common.py

@@ -30,6 +30,7 @@

 bin_dir_path = str(Path(sys_argv[0]).resolve().parent.parent)
 share_dir_path = str(F4PGA_SHARE_DIR)
+aux_dir_path = str(Path(__file__).resolve().parent.parent / "aux")


aux is a reserved keyword on some systems. Please, use a different name/location for the aux_dir_path. I would suggest to put the TCL scripts added in this PR into f4pga/flows/tcl.

Aux stands for "auxilary". The directory is mean to hold all sorts of auxilary scripts/data. I think it's more convinient to havve one, well-structured directory for such things callend "aux", rather than having one for tcl, one for python, etc and creating a separate variable for each such directory.

Could you give me an example of how this "aux" would interfere with some systems?

Could you give me an example of how this "aux" would interfere with some systems?

Cloning this branch on an NTFS drive on Fedora produces the following error:

$ git clone https://github.com/antmicro/f4pga Cloning into 'f4pga'... remote: Enumerating objects: 5008, done. remote: Counting objects: 100% (5008/5008), done. remote: Compressing objects: 100% (1855/1855), done. remote: Total 5008 (delta 3007), reused 4971 (delta 2995), pack-reused 0 Receiving objects: 100% (5008/5008), 241.55 MiB | 16.21 MiB/s, done. Resolving deltas: 100% (3007/3007), done. $ cd f4pga/ $ git checkout -b test-branch origin/yosys_module fatal: cannot create directory at 'f4pga/aux': Invalid argument

See also https://www.google.com/search?q=aux+reserved+filename.

I think it's more convinient to havve one, well-structured directory for such things callend "aux", rather than having one for tcl, one for python, etc and creating a separate variable for each such directory.

So far, we only have TCL assets (which will very probably be removed soon) and a single YAML file. Python code is installed as part of the package and it does not need any separated variable.

Let's not overengineer the implementations. Better wait until the need arises. Please, focus on the core functionality that this PR is trying to bring.

How are you going to remove Tcl scripts? The old ones might be removed at some point, but I don't think that the new ones are going to be purged as well.

I renamed aux to auxiliary. That should fix the problem with nonsensical filesystems.

I'm not entirely sure what data is going to be used in the future. Whether it's auxiliary directory or tcl doesn't really change the complexity of my current solution.

Signed-off-by: Krzysztof Boronski <kboronski@antmicro.com>

Extracted common Tcl utility commands into another Tcl sctipt Signed-off-by: Krzysztof Boronski <kboronski@antmicro.com>

Signed-off-by: Krzysztof Boronski <kboronski@antmicro.com>

* use ttop value instead of eblif path for bitstream file name * io_rename fixes, eos-s3 flow: sdc-in -> sdc Signed-off-by: Krzysztof Boronski <kboronski@antmicro.com>

Signed-off-by: Krzysztof Boronski <kboronski@antmicro.com>

Signed-off-by: Unai Martinez-Corral <umartinezcorral@antmicro.com>

Signed-off-by: Krzysztof Boronski <kboronski@antmicro.com>

… comments about optional outputs Signed-off-by: Krzysztof Boronski <kboronski@antmicro.com>

Signed-off-by: Krzysztof Boronski <kboronski@antmicro.com>

umarcor · 2022-09-07T16:45:53Z

f4pga/auxiliary/tool_data/yosys/scripts/vendor/xilinx/xc7/utils.tcl

@@ -0,0 +1,61 @@
+# Copyright (C) 2022 F4PGA Authors


This seems to be sourced in synth.tcl only. Is there any reason not to copy the content there? See https://github.com/chipsalliance/f4pga/blob/main/f4pga/wrappers/tcl/xc7.f4pga.tcl#L14-L58.

umarcor · 2022-09-07T16:47:11Z

f4pga/wrappers/sh/xc7/synth.f4pga.sh

@@ -18,7 +18,16 @@

 set -e

-SYNTH_TCL_PATH="$(python3 -m f4pga.wrappers.tcl synth)"
+MYPATH=`realpath $0`


Since the current proposal is to duplicate the TCL pipelines, please keep f4pga.wrappers.sh untouched in this PR. Move these changes to a follow-up PR.

umarcor · 2022-09-07T16:48:11Z

f4pga/flows/common_modules/pack.py

@@ -58,8 +58,8 @@ def execute(self, ctx: ModuleContext):
        if ctx.outputs.util_rpt:
            (build_dir / DEFAULT_UTIL_RPT).rename(ctx.outputs.util_rpt)

-    def __init__(self, _):
-        self.name = "pack"
+    def __init__(self, params, r_env, instance_name):


What is the motivation for this change? Can it be handled as an standalone PR?

The motivation is to grab be able to access stage's name. When yosys module gets called twice in a flow, it should output different log targets. Names of these targets are based on the name of a stage.

I think this change could be handled in a separate PR.

This change can also be used to remove stage_name field from generic_script_wrapper.

Please, split it to a separate PR, so we can analyse that feature on its own. If most of the stages receive params, r_env and instance_name, it might be worth wrapping those in an object/class and use it as the payload from stage to stage.

What about r_env?

umarcor · 2022-09-07T16:55:46Z

f4pga/auxiliary/tool_data/yosys/scripts/vendor/quicklogic/pp3/synth.tcl

+
+# Write output JSON, fixup cell names using an external Python script
+write_json $f4pga_json_org
+exec $f4pga_python3 ${f4pga_shareDir}/scripts/yosys_fixup_cell_names.py $f4pga_json_org $f4pga_json_presplit


After rebasing, use -m f4pga.utils.quicklogic.yosys_fixup_cell_names and -m f4pga.utils.yosys_split_inouts as in https://github.com/chipsalliance/f4pga/blob/main/f4pga/wrappers/tcl/eos-s3.f4pga.tcl#L176-L194.

Signed-off-by: Krzysztof Boronski <kboronski@antmicro.com>

kboronski-ant force-pushed the yosys_module branch from 99ba21a to c73e210 Compare July 6, 2022 16:48

umarcor added Enhancement New feature or request f4pga (python) Python package labels Jul 11, 2022

kboronski-ant marked this pull request as ready for review July 11, 2022 17:16

kboronski-ant force-pushed the yosys_module branch 3 times, most recently from 587d188 to 0c9fd3f Compare July 12, 2022 13:46

kboronski-ant force-pushed the yosys_module branch from f5ada6f to 6c11391 Compare July 20, 2022 10:41

This was referenced Jul 20, 2022

Add ice40 flow #586

Draft

"spectrum" example for FOMU (ice40) chipsalliance/f4pga-examples#337

Draft

kboronski-ant force-pushed the yosys_module branch 5 times, most recently from 78ff0d1 to eebe733 Compare August 2, 2022 11:13

umarcor reviewed Aug 2, 2022

View reviewed changes

f4pga/common_modules/yosys.py Outdated Show resolved Hide resolved

kboronski-ant force-pushed the yosys_module branch 3 times, most recently from 9c2c779 to 978589c Compare August 4, 2022 12:37

umarcor marked this pull request as draft August 4, 2022 19:06

umarcor mentioned this pull request Aug 6, 2022

TEST use branch yosys_module f4pga/f4pga-arch-defs#2851

Closed

umarcor force-pushed the yosys_module branch 6 times, most recently from a58e880 to 0cb8079 Compare August 25, 2022 04:48

kboronski-ant force-pushed the yosys_module branch from 0cb8079 to 0813ba9 Compare August 25, 2022 13:22

kboronski-ant force-pushed the yosys_module branch 2 times, most recently from 45da1d9 to 0cb8079 Compare August 26, 2022 13:53

umarcor force-pushed the yosys_module branch 4 times, most recently from c533d72 to 94b8b09 Compare September 2, 2022 13:21

kboronski-ant mentioned this pull request Sep 2, 2022

Refactor Yosys for f4pga/symbiflow usage olofk/edalize#335

Closed

kboronski-ant marked this pull request as ready for review September 6, 2022 17:37

umarcor suggested changes Sep 6, 2022

View reviewed changes

umarcor reviewed Sep 7, 2022

View reviewed changes

kboronski-ant and others added 10 commits September 7, 2022 18:44

Yosys module

4598071

Signed-off-by: Krzysztof Boronski <kboronski@antmicro.com>

yosys module: save log

251c959

Signed-off-by: Krzysztof Boronski <kboronski@antmicro.com>

Added aux directory

ff58f9f

Extracted common Tcl utility commands into another Tcl sctipt Signed-off-by: Krzysztof Boronski <kboronski@antmicro.com>

f4pga/common_modules/yosys: add is_dry subcommand

03500ac

Signed-off-by: Krzysztof Boronski <kboronski@antmicro.com>

f4pga/eos-s3: use yosys module instead of the removed synth module

7dc5c14

* use ttop value instead of eblif path for bitstream file name * io_rename fixes, eos-s3 flow: sdc-in -> sdc Signed-off-by: Krzysztof Boronski <kboronski@antmicro.com>

Update test flow configurations

0e55c33

Signed-off-by: Krzysztof Boronski <kboronski@antmicro.com>

f4pga/setup: use glob pattern instead of a custom func

5cce550

Signed-off-by: Unai Martinez-Corral <umartinezcorral@antmicro.com>

Update tcl scripts to have inputs that are more friendly to arch-defs

e82a4e6

Signed-off-by: Krzysztof Boronski <kboronski@antmicro.com>

Unified synthesis and json->eblif tcl scripts for yosys module. Added…

693312f

… comments about optional outputs Signed-off-by: Krzysztof Boronski <kboronski@antmicro.com>

Rename aux -> auxiliary

3be9135

Signed-off-by: Krzysztof Boronski <kboronski@antmicro.com>

umarcor suggested changes Sep 7, 2022

View reviewed changes

umarcor reviewed Sep 7, 2022

View reviewed changes

kboronski-ant force-pushed the yosys_module branch from 45d5d7e to 3be9135 Compare September 7, 2022 17:00

umarcor marked this pull request as draft September 8, 2022 11:02

Use python command to call scripts in tcl files

9d1490d

Signed-off-by: Krzysztof Boronski <kboronski@antmicro.com>

umarcor added Backlog Discussion labels Sep 28, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Universal Yosys module #581

Universal Yosys module #581

kboronski-ant commented Jul 5, 2022 •

edited

umarcor commented Aug 4, 2022

umarcor commented Aug 6, 2022

kboronski-ant commented Sep 6, 2022

umarcor Sep 6, 2022

umarcor Sep 7, 2022

kboronski-ant Sep 7, 2022

umarcor Sep 7, 2022

umarcor Sep 7, 2022

kboronski-ant Sep 7, 2022 •

edited

kboronski-ant Sep 7, 2022 •

edited

umarcor Sep 7, 2022

umarcor Sep 7, 2022

umarcor Sep 7, 2022

kboronski-ant Sep 8, 2022

umarcor Sep 8, 2022

umarcor Sep 7, 2022

Universal Yosys module #581

Are you sure you want to change the base?

Universal Yosys module #581

Conversation

kboronski-ant commented Jul 5, 2022 • edited

Content description

Features

Caveats

Work being done

umarcor commented Aug 4, 2022

umarcor commented Aug 6, 2022

kboronski-ant commented Sep 6, 2022

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

kboronski-ant Sep 7, 2022 • edited

Choose a reason for hiding this comment

kboronski-ant Sep 7, 2022 • edited

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

Choose a reason for hiding this comment

kboronski-ant commented Jul 5, 2022 •

edited

kboronski-ant Sep 7, 2022 •

edited

kboronski-ant Sep 7, 2022 •

edited