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

yosys/synth.tcl: migrate to newer supported drw & drf commands #2051

Merged
merged 9 commits into from
Nov 22, 2023

Conversation

dlmiles
Copy link
Contributor

@dlmiles dlmiles commented Nov 19, 2023

  • Add tt05-i2c-bert (https://github.com/dlmiles/tt05-i2c-bert) to CI
    ~ Replace instances of ABC command rewrite with drw -l with SYNTH_ABC_LEGACY_REWRITE being set to 1 restoring the older functionality (0 by default)
    ~ Replace instances of ABC command refactor with drf -l with SYNTH_ABC_LEGACY_REFACTOR being set to 1 restoring the older functionality (0 by default)
    ~ Added delete t:\$print to synth.tcl to fix designs such as PPU with synthesized prints (as in Update Yosys, Add EQY/SBY efabless/openlane2#189)

Original PR body follows:

Re Yosys commands: rewrite & refactor

These commands are considered obsolete and unmaintained by Yosys

Replacement commands exist in the form of: drw & drf

See also:
YosysHQ/yosys#4039
#1523


[ci ets]

Resolves #2052

Depends on efabless/openlane-ci-designs#3

Re Yosys commands: rewrite & refactor

These commands are considered obsolete and unmaintained by Yosys

Replacement commands exist in the form of: drw & drf

See also:
  YosysHQ/yosys#4039
  The-OpenROAD-Project#1523
@dlmiles
Copy link
Contributor Author

dlmiles commented Nov 19, 2023

This patch may require significant QA and testing

This patch is known to resolve linked issues and removes the need to request a downgrade of yosys version within recent OpenLane tags at #2050

Thanks to @jix at YoSysHQ for the quick diagnosis, support and details of a corrective action to take

@donn
Copy link
Collaborator

donn commented Nov 20, 2023

FWIW PPU also appears to be failing in main for the same reason, so it will not be treated as a regression for the purposes of this patch.

@jix
Copy link

jix commented Nov 20, 2023

I just noticed that while drf supports all the toggles of refactor it has a different default for the -l toggle (preserve number of levels) so drf -l should be closer to refactor and drf closer to refactor -l. This might help with minimizing the effects replacing the older commands has.

@donn donn requested a review from kareefardi November 20, 2023 13:43
@donn
Copy link
Collaborator

donn commented Nov 20, 2023

@dlmiles Would you happen to have a test design on hand? We'd like to add it to our test sets if possible.

@dlmiles
Copy link
Contributor Author

dlmiles commented Nov 20, 2023

Do you have a link to the format you require the design to be in ? Maybe you just want a ZIP with *.tcl and *.def and *.v and *.vh ?

What are you testing, just getting to the "[SUCCESS] Flow Complete" ? any post-implementation testing ?

The design is actively maintained here https://github.com/dlmiles/tt05-i2c-bert and Apache2 and has a GDS version included in efabless 2311 shuttle in TT05 and the design is at prototype status (the github version has a number of open-source github CI automations including post-implementation gate-level testing, but no open-source SDF/back-annotation testing yet)

Does the project need to work on gf180, currently it does not. The project uses a few directly instantiated sky130 cells MAJ3/LATCHES.

Note the design (dlmiles/tt05-i2c-bert@08a437f) in question has the following results on these OPENLANE_TAG:

  • 2bd7085~2023.09.25 SUCCESS Flow complete STEP39 [LAST KNOWN WORKING TAG]
  • 5771836~2023.10.02 LVS ERROR STEP40-lvs-lef [FIRST LVS ERROR TAG]
  • 903a86a~2023.10.10 LVS ERROR STEP40-lvs-lef
  • 747bdbb~2023.10.20 LVS ERROR STEP40-lvs-lef
  • dc5af98~2023.10.21 LVS ERROR STEP40-lvs-lef [Yosys 0.30+48 (git sha1 14d50a176d5, gcc 8.3.1 -fPIC -Os)]
  • 6ab36bf~2023.10.27 Yosys ERROR STEP1-1-synthesis [Yosys 0.34 (git sha1 4a1b5599258, gcc 8.3.1 -fPIC -Os)]
    yosys-abc: src/base/abc/abcAig.c:1134: void abc::Abc_AigUpdateLevelR_int(abc::Abc_Aig_t*): Assertion `Abc_ObjIsNode(pNode)' failed.
  • 1153701~2023.11.11 Yosys ERROR STEP1-1-synthesis
  • 1e9efe9~2023.11.17 Yosys ERROR STEP1-1-synthesis

The recent period of LVS errors is this something OpenLane is aware of? it looks like the design had 3 steps added during that period, it looks like those steps are not included in the current flow, it is not clear when they were reverted/removed as STEP1 failures hide any STEP39 failures.

Is there any merit to request efabless build a hub.docker.com for Yosys 0.33+6 (git sha1 41b34a193) even if it is not used by any OpenLane tag. This would be very near the last known working version of Yosys before the ABC update it received. The purpose of this is for it to be available at hub.docker.com for performing any comparisons concerning both the ABC assertion/crash and changes in design output resulting in the command changes. efabless already has 0.34 (currently being used) at hub.docker.com so it maybe useful during the next 12 months to provide an easy option to manually downgrade Yosys and rebuild OpenLane to the point immediately before the ABC update during diagnostics.

@jix Can you confirm the patch in the current form make sense for drw -l usage, I read your comment and only see comments on the refactor/drf command. The question is should drw also use the -l option ? I would not have assumed it should from your advice in the comment.

@dlmiles
Copy link
Contributor Author

dlmiles commented Nov 21, 2023

tt05-i2c-bert-testcase.zip

Minimal Project.

OPENLANE_TAG=2023.09.25 [Last known success]
OPENLANE_TAG=2023.11.17 [Most recent tested failure, due to yosys ABC assertion]

$ unzip -lv tt05-i2c-bert-testcase.zip
Archive:  tt05-i2c-bert-testcase.zip
 Length   Method    Size  Cmpr    Date    Time   CRC-32   Name
--------  ------  ------- ---- ---------- ----- --------  ----
       0  Stored        0   0% 11-20-2023 22:42 00000000  tt05-i2c-bert-testcase/
    1165  Defl:N      533  54% 11-20-2023 22:42 d6d9adb4  tt05-i2c-bert-testcase/sky130_fd_sc_hd__dlrtp.v
    4472  Defl:N     1373  69% 11-20-2023 22:42 f48a507a  tt05-i2c-bert-testcase/top_tt_um_dlmiles_tt05_i2c_bert.v
    1102  Defl:N      518  53% 11-20-2023 22:42 861acd38  tt05-i2c-bert-testcase/sky130_fd_sc_hd__maj3.v
    2226  Defl:N      867  61% 11-20-2023 22:42 67c9ae2a  tt05-i2c-bert-testcase/glitch_free_clock_mux_async_reset.v
     636  Defl:N      304  52% 11-20-2023 22:42 4a0b5ffd  tt05-i2c-bert-testcase/user_config.tcl
     795  Defl:N      426  46% 11-20-2023 22:42 b86d2d74  tt05-i2c-bert-testcase/dff_async_reset.v
     833  Defl:N      437  48% 11-20-2023 22:42 a6520e22  tt05-i2c-bert-testcase/dffqn_negedge_async_reset.v
    2795  Defl:N     1316  53% 11-20-2023 22:42 6ed04ba4  tt05-i2c-bert-testcase/config.tcl
   11357  Defl:N     3948  65% 11-20-2023 22:42 7b5d04bc  tt05-i2c-bert-testcase/LICENSE
   11360  Defl:N     1395  88% 11-20-2023 22:42 f9d93b87  tt05-i2c-bert-testcase/tt_block_1x1.def
    1080  Defl:N      452  58% 11-20-2023 22:42 f43be74d  tt05-i2c-bert-testcase/README.txt
   67386  Defl:N     7898  88% 11-20-2023 22:42 f15d1db6  tt05-i2c-bert-testcase/TT05I2CBertTop.v
--------          -------  ---                            -------
  105207            19467  82%                            13 files

@mattvenn
Copy link
Collaborator

would it be an idea to add a lot more of the tinytapeout designs to the openlane regression? It seems like a great resource that would easily improve the tests.

@donn
Copy link
Collaborator

donn commented Nov 22, 2023

@mattvenn Agreed. Can you get in touch with @kareefardi over Slack? He's currently gathering designs we can add to the CI.

@donn donn merged commit f691c8c into The-OpenROAD-Project:master Nov 22, 2023
49 of 52 checks passed
@donn
Copy link
Collaborator

donn commented Nov 22, 2023

Thank you so much for looking into this @dlmiles. Intensely appreciated.

donn added a commit to efabless/openlane2 that referenced this pull request Nov 22, 2023
* Updated `Yosys.Synthesis`, `Yosys.VHDLSynthesis` per comments from Yosys team [(1)](YosysHQ/yosys#4039 (comment)) [(2)](The-OpenROAD-Project/OpenLane#2051 (comment))  
    * Replaced instances of ABC command `rewrite` with `drw -l` with new variable `SYNTH_ABC_LEGACY_REWRITE` being set to `true` restoring the older functionality (`false` by default)
    * Replaced instances of ABC command `refactor` with `drf -l` with new variable `SYNTH_ABC_LEGACY_REFACTOR` being set to `true` restoring the older functionality (`false` by default)
urish added a commit to TinyTapeout/tt-gds-action that referenced this pull request Nov 25, 2023
Also update the PDK to the respective version. This solves the OpenLane ABC issue: The-OpenROAD-Project/OpenLane#2051
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 this pull request may close these issues.

Replace outdated commands in synthesis scripts
5 participants