Upgrading to Yosys v0.55 and Updating Slang

[loglav03]

Upgraded both yosys and yosys-slang to more recent versions, and provided explicit top module selection for HDL designs.

Description

Upgraded Yosys to v0.55 and upgraded yosys-slang to more recent version (55e3a2a). Modified synthesis flow to explicitly select the top modules for the given HDL instead of solely relying on the yosys hierarchy -auto-top command to automatically select top modules. Also added some common koios benchmarks to the top_map array for top module selection.

Related Issue

Supports: #3195

Motivation and Context

Upgrading yosys and slang to more recent versions could potentially help solve some of the issues with latch inference occurring in some HDL designs. And providing explicit top module selection is required by slang to avoid errors, and is better practice than relying on automatic selection of top modules.

How Has This Been Tested?

Ran tests locally: These were my own test clones of vtr_reg_nightly_test3/vtr_reg_qor and vtr_reg_nightly_test4/koios_medium
Ran and passed CI tests.

Local tests still have failures because still waiting on benchmark modifications in #3195

Types of changes

• [ x] Bug fix (change which fixes an issue)
• New feature (change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

• My change requires a change to the documentation
• I have updated the documentation accordingly
• I have added tests to cover my changes
• [ x] All new and existing tests passed

Summary of QoR

Tests Used: vtr_reg_qor, vtr_reg_qor_chain, koios_medium, koios_other
Tables Used:
QoR_Tables.zip
Summary_of_Tables.xlsx

Most metrics have minimal differences. At most they will differ by 1% - 2%

[AlexandreSinger]

Thanks @loglav03 , I just have a couple of comments.

The self-hosted CI runner is currently down right now and I am working on getting it up and running again. Once its back up, I want to run the weekly tests on this branch before we merge it. I do not expect much change to the golden results based on the changes in this PR, but its always good to check.

[loglav03]

@AlexandreSinger - Before we merge, I'm looking into replacing the -auto-top option with -top <module(s)> in the yosys hierarchy command in synthesis.tcl. This for some reason is giving me a syntax error when using -top, so this may take a bit to get working.

[loglav03]

@AlexandreSinger - Before we merge, I'm looking into replacing the -auto-top option with -top <module(s)> in the yosys hierarchy command in synthesis.tcl. This for some reason is giving me a syntax error when using -top, so this may take a bit to get working.

@AlexandreSinger - I got this working now. My concern with this is that in order to discover what top modules go with their respective verilog files, it needs to check an array map that I set up in the slang_filelist.tcl helper file for synthesis.tcl. This array list contains key value pairs for verilog files (key) and top modules (value) and this needs to be updated manually if it can't find the verilog file in the map. I think this can prove to be tedious for vtr users if they attempt to use a benchmark that I didn't set up in the map. I'm already dealing with this when testing with the vtr_reg_strong regression test.

I'm wondering if the set up I have is okay or if I should move the map to some place more intuitive. Maybe this is something we should discuss more tomorrow at the vtr sync up meeting.

[AlexandreSinger]

@AlexandreSinger - Before we merge, I'm looking into replacing the -auto-top option with -top <module(s)> in the yosys hierarchy command in synthesis.tcl. This for some reason is giving me a syntax error when using -top, so this may take a bit to get working.

@AlexandreSinger - I got this working now. My concern with this is that in order to discover what top modules go with their respective verilog files, it needs to check an array map that I set up in the slang_filelist.tcl helper file for synthesis.tcl. This array list contains key value pairs for verilog files (key) and top modules (value) and this needs to be updated manually if it can't find the verilog file in the map. I think this can prove to be tedious for vtr users if they attempt to use a benchmark that I didn't set up in the map. I'm already dealing with this when testing with the vtr_reg_strong regression test.

I'm wondering if the set up I have is okay or if I should move the map to some place more intuitive. Maybe this is something we should discuss more tomorrow at the vtr sync up meeting.

@loglav03 Sounds good! Lets discuss this during the meeting tomorrow. I think the most intuitive place to put this information is within the benchmark directory itself instead of a large monolithic file. For example, I imagine the following structure:

/benchmark_dir
  benchmark1.v
  benchmark2.v
  ...
  top_modules.txt

And within the `top_modules.txt we expect a mapping between the benchmark and its top level module. Something like:

benchmark1.v: top
benchmark2.v: or1200
...

This does not have to be a text file, it can be whatever file format would be the most convenient for use in your tcl script. I prefer a solution like this since it allows each benchmark to own its mapping.

In my opinion though, I do not think this should block this PR. I believe that we should upgrade Yosys and Slang in one PR (this one) and then move to this new top-level module scheme in another PR. What do you think?

To merge this PR, I would need to run the weekly tests, which are currently running master to ensure that they are working correctly. After that concludes I plan to run your branch.

[loglav03]

@AlexandreSinger - That plan sounds good to me! Good news is I have most of the benchmarks already mapped to their top modules for slang usage so it shouldn't be too difficult to get it set up the way you described.

[AlexandreSinger]

I have started the weekly tests now:
https://github.com/verilog-to-routing/vtr-verilog-to-routing/actions/runs/16807032024

I have updated the golden results for the weekly tests and ran it yesterday, so all tests should be green.

[loglav03]

@AlexandreSinger - I've completed the QoR comparisons for this. Should vtr_reg_qor, vtr_reg_qor_chain, and koios_medium be enough tests for this?

[AlexandreSinger]

@AlexandreSinger - I've completed the QoR comparisons for this. Should vtr_reg_qor, vtr_reg_qor_chain, and koios_medium be enough tests for this?

Hi @loglav03 , those are pretty good. Koios_medium does not do all of the koios testcases though, and they are all a bit small. To be extra safe, could you please also do the koios_other tests? These include a few of the larger Koios benchmarks which I am interested how they changed.

These can be found in vtr_reg_nightly_test6/koios_other/.

This should give us a good breadth of how the change to the versions changed the Koios benchmark suite.

[AlexandreSinger]

The quality of the three benchmarks you sent seem to be good to me! They are practically just noise due to the number of CLBs changing slightly (0.1% increase in num_clb). The CPD and WL improved slightly by 1%, but I am interested to see how the koios_other results look since these circuits are a bit small.

Thank you very much for collecting these! If koios_other looks good too, we should merge this!

@vaughnbetz FYI.

[loglav03]

@AlexandreSinger - I've completed the QoR comparisons for this. Should vtr_reg_qor, vtr_reg_qor_chain, and koios_medium be enough tests for this?

Hi @loglav03 , those are pretty good. Koios_medium does not do all of the koios testcases though, and they are all a bit small. To be extra safe, could you please also do the koios_other tests? These include a few of the larger Koios benchmarks which I am interested how they changed.

These can be found in vtr_reg_nightly_test6/koios_other/.

This should give us a good breadth of how the change to the versions changed the Koios benchmark suite.

Koios_other has now been added!

[AlexandreSinger]

Thanks @loglav03 , the koios_other results looks great! If anything this is improving the quality quite substantially. WL improved by around 1% and CPD improved by around 4% (however, CPD tends to be noisy for these circuits). Overall it looks like this change is consistent across these benchmarks. I am ok with merging. I will update the branch and merge when it passes the CI again (good to be safe and the CI is quick now).

[AlexandreSinger]

LGTM

[vaughnbetz]

Thanks @loglav03 and @AlexandreSinger ! Great to be up to date.