Skip to content

Upgrade the Yosys+Odin-II Front-end #2148

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

Open
wants to merge 34 commits into
base: master
Choose a base branch
from

Conversation

sdamghan
Copy link
Member

@sdamghan sdamghan commented Aug 29, 2022

Description

In this PR, the Yosys script for the Yosys+Odin-II is updated to perform the synthesis for basic operations by Yosys. Then Yosys generates the BLIF file with unmapped and coarse-grained memories, arithmetic operations, simple DFF types with set/reset, and latches so that the Odin-II partial mapper will take care of them.

Related Issue

Motivation and Context

Fixing issues raised by the Koios 2.0 development team

How Has This Been Tested?

Types of changes

  • 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
  • All new and existing tests passed

@sdamghan sdamghan force-pushed the enhance_yosys_odin_koios++ branch from 147fb37 to 88e04a0 Compare August 29, 2022 11:31
@github-actions github-actions bot added the Odin Odin II Logic Synthesis Tool: Unsorted item label Aug 29, 2022
@sdamghan
Copy link
Member Author

sdamghan commented Aug 29, 2022

@aman26kbm - this branch improves the Yosys+Odin-II front-end and should address some of the issues we discussed for Koios 2.0. Would you please give it a run and let me know your thoughts on the results?

@aman26kbm
Copy link
Contributor

Sure, lemme try it out.

@sdamghan
Copy link
Member Author

Thanks @aman26kbm - JFYI, no worries about the last commits as they only include updates in expected results.

@sdamghan sdamghan force-pushed the enhance_yosys_odin_koios++ branch from 97bfd3c to 4e809c8 Compare August 29, 2022 20:18
@aman26kbm
Copy link
Contributor

Hi Seyed, this PR seems to have some issue.

I ran these designs:

attention_layer
clstm_like.small
conv_layer
dla_like.small
eltwise_layer
lstm
robot_rl
softmax
spmv
stereovision2
tiny_darknet_like.small

I ran them in 4 modes:

  1. Odin-only, with automatic inference of dsps and brams (i call this "soft")
  2. Yosys+ODIN, with automatic inference of dsps and brams
  3. Odin-only, with hard instantiations of dsps and brams (i call this "hard")
  4. Yosys+ODIN, with hard instantiations of dsps and brams

The results are tabulated in this spreadsheet: https://utexas.sharepoint.com/:x:/s/LCATeam3-VTR-ML-Benchmarks/EWQwBjnUJLxPjnq5QqTwP-wB50gk4hfGXz9YwkroBCB4lg?e=LtxlGI
See tabs: odin_vs_yosodin_aug29_hard and odin_vs_yosodin_aug29_soft

In most of the Yosys+ODIN runs, you'll see the results are blank. That's because I see this error in the log files:

 87 /export/aman/vtr_aman/vtr-verilog-to-routing/ODIN_II/SRC/BlockMemories.cpp:1081: create_nrmw_dual_port_ram: Assertion num_rd_ports > 2 failed
 88   1079 |
 89   1080 |     /* should have been resovled before this function */
 90   1081 |     oassert(num_rd_ports > 2);
 91         ^~~~
 92   1082 |     oassert(num_wr_ports > 2);
 93   1083 |
 94 Command terminated by signal 6

In the "hard" runs, I see the error mentioned above only in 1 design - tiny_darknet_like.small.
But in the "soft" runs, I see the error mentioned above in 7 out of 11 designs.

Also, for the designs that do pass, I still see some differences because of packing (attention_layer, eltwise_layer, lstm, robot_rl, softmax).

@sdamghan
Copy link
Member Author

Thanks @aman26kbm - let me go through it one more time. Meanwhile, can you provide me with the Koios++ scripts you use to run the VTR flow?

@aman26kbm
Copy link
Contributor

Sure. The scripts are located here:
https://github.com/aman26kbm/vtr-verilog-to-routing/tree/master/vtr_flow/tasks/koios

There are scripts here for generating various folders for experiments, generating task configs, parsing results, etc. But don't worry about all that.

I name experiments as exp1a, exp1b, exp4c, etc. They correspond to the different experiments we conduct for the paper.
For example, exp1a is the normal run for all designs, but exp4c is the run with the "densest" architecture.

For the current case, the relevant folders that you need are:
exp1a_odin_hard
exp1a_odin_soft
exp1a_yosodin_hard
exp1a_yosodin_soft

You can go inside each folder and you'll see the run file. You'll see 3 commands pointing to 3 different task lists (see files called task_list_exp*). I have categorized the designs into 3 lists - short, normal, large - based on how long each design takes.

You can just execute the run file to run all designs, but you can delete some of the designs in the task lists and then execute the run file to run a few designs. (In the current case, I deleted all but the 11 designs I listed above)

@sdamghan sdamghan added the Yosys+Odin-II The Yosys+Odin-II synthesizer: the Yosys coarse-grained Tcl script and Odin-II partial mapping flow label Sep 1, 2022
@sdamghan
Copy link
Member Author

sdamghan commented Sep 1, 2022

Moving an email thread here:

Hi Aman,

Thanks for the results. I have outlined my results in the QoR_results sheet. I do see some minor inconsistency in the number of CLBs, however, I am not sure where are they coming from.

Based on my observations, the new upgrade works better than the previous version as we survive the SPMV benchmark in terms of num of DSPs and Memories. Also, if you look at the DSP column for the comparison tables, you would notice that overall, the new YosysOdin has results closer to Odin-II than the previous version. Concerning the number of memory blocks with hard blocks enabled, they are both equal except for the SPMV benchmarks, which are fixed and increased by the new updates to the synthesizer.

There are a lot of improvements within the new upgrade. However, we lost the tiny_darknet_benchmark as it failed in the ABC stage. I have not had the chance to run all koios++ benchmarks with the new synthesizer, but we can decide better when we see its behaviour for other benchmarks as well.

Regarding the new upgrade, I should say the synthesis load of basic logic (such as AND ,OR reduction) and FFs has transferred to Yosys (whatever the simplemap command is for in Yosys). Moreover, additional memory optimization passes are added to the Yosys synthesis script. This should provide more assurance about the connection as partially fine-grained synthesis is performed for these basic logics.


Hi Seyed,

Thanks for the detailed analysis. I also agree that the new results are much closer to ODIN. So, this upgrade certainly looks better. I will run all benchmarks with this new upgrade and tabulate the results.

Btw another thing came to my mind about the differences that are because of packing.. I think this is a non-issue because the Yosys+ODIN generated netlist is such that VPR is able to pack the blocks better. We can mention this in the paper. I wasn't thinking about it this way earlier.

Regarding the other benchmarks where the difference is not because of packing (clstm, dla and stereovision2), we had said that we will use the results from ODIN for these designs. We can do the same for tiny darknet. I think we have to decide and start writing the paper.

The QoR Comparison for all front-ends, including the comparison of the previous version of Yosys+Odin-II to the upgraded one, is in the following links.

Upgraded Yosys+Odin-II QoR comparisons for a small set of Koios2.0 benchmarks

Upgraded Yosys+Odin-II QoR comparisons for VTR benchmarks

@sdamghan sdamghan force-pushed the enhance_yosys_odin_koios++ branch from 370886a to a928eb1 Compare September 4, 2022 16:03
…oration by utilizing Yosys built-in coarse-grained synthesis

		 steps in addition to calling simplemap for basic operations. At this point Odin-II mainly performs the partial
		 technology mapping on arithmetic operations, memories (soft and hard), and HARD IPs

Signed-off-by: Seyed Alireza Damghani <[email protected]>
…they are introduced by recent changes

		 on the Yosys+Odin-II coarse-grained synthesis script

Signed-off-by: Seyed Alireza Damghani <[email protected]>
…sensitivity and the initial value of

		 latches in the BLIF files read by Odin-II BLIFReader
[Odin-II]: enhance the Odin-II bit map reader for finding logic types in the BLIF files for LOGICAL OR

Signed-off-by: Seyed Alireza Damghani <[email protected]>
…y satisfied the partial mapper requirement

		 - padding signals with from gnd net instead of unconn for equalizing port sizes

Signed-off-by: Seyed Alireza Damghani <[email protected]>
…ers have the same name as top output signals

Signed-off-by: Seyed Alireza Damghani <[email protected]>
…pass for basic operations.

		 Memory, arithmetic, latches and simple dffs with set/reset will remain for the
		 Odin-II partial mapper. The purpose of adding this pass was to avoid losing
		 some intermediatory connections between submodules and the top module when a
		 pure coarse-grained BLIF file was generated by Yosys.

Signed-off-by: Seyed Alireza Damghani <[email protected]>
…ingle bit width node in the partial mapper

		 - treat single bit width and single port logical OR as BUF nodes to eliminate them later in
		   the partial mapping phase

Signed-off-by: Seyed Alireza Damghani <[email protected]>
…LIF file by BLIFReader

		 - move reduce_input_port after port size checking while equalize input port
		   routine is called

Signed-off-by: Seyed Alireza Damghani <[email protected]>
…nchmarks located in regression_tests/benchmaks/common

		 - fix output port size of div and mod benchmarks to avoid additional warnings in JSON results

Signed-off-by: Seyed Alireza Damghani <[email protected]>
		 - update simulation output vector for bm_dag3_mod and bm_expr_all_mod

Signed-off-by: Seyed Alireza Damghani <[email protected]>
                 - update simulation output vectors

Signed-off-by: Seyed Alireza Damghani <[email protected]>
sdamghan and others added 13 commits September 4, 2022 13:50
Signed-off-by: Seyed Alireza Damghani <[email protected]>
…rch, bm_match4_str_arch, and bm_expr_all_mod

Signed-off-by: Seyed Alireza Damghani <[email protected]>
… collecting memory instances

Signed-off-by: Seyed Alireza Damghani <[email protected]>
…or multiport memory (regardless of the number of ports)

		 - fix a memory leak in split_cascade_port

Signed-off-by: Seyed Alireza Damghani <<[email protected]>
Signed-off-by: Seyed Alireza Damghani <[email protected]>
Signed-off-by: Seyed Alireza Damghani <[email protected]>
… Yosys plugins installed

		 - report the frontend elaborator used by Odin-II

Signed-off-by: Seyed Alireza Damghani <[email protected]>
Signed-off-by: Seyed Alireza Damghani <[email protected]>
@sdamghan sdamghan force-pushed the enhance_yosys_odin_koios++ branch from dc0474d to a26d280 Compare September 4, 2022 16:51
@sdamghan
Copy link
Member Author

sdamghan commented Sep 5, 2022

@aman26kbm - Would you please share a short summary of the results and your thoughts here? If there is no more roadblock or issue, we can move forward and merge this PR.

@aman26kbm
Copy link
Contributor

Here's the update from the runs from yesterday (only ran these 5 designs because these are the ones that have been giving us trouble lately):

Yosys+ODIN hard
Tiny darknet small -> Fail at abc stage
Lenet -> Fail at abc stage
Deepfreeze style1 -> ODIN crash
Deepfreeze style2 -> ODIN crash
Deepfreeze style3 -> ODIN crash

ODIN-only hard
Tiny darknet small -> Pass
Others fail, but that's expected. These designs have never passed ODIN-only flow, coz they have some SV syntax in them that's not supported by ODIN.

Yosys+ODIN soft
Tiny darknet small -> Fail at abc stage
Lenet -> Fail at abc stage
Deepfreeze style1 -> Pass
Deepfreeze style2 -> ODIN crash
Deepfreeze style3 -> Pass

ODIN-only soft
Tiny darknet small -> Pass
Others fail, but that's expected. These designs have never passed ODIN-only flow, coz they have some SV syntax in them that's not supported by ODIN.

@sdamghan
Copy link
Member Author

sdamghan commented Sep 6, 2022

We temporarily suspend this PR to provide appropriate solutions for the bugs mentioned above.

@sdamghan sdamghan closed this Sep 6, 2022
@sdamghan
Copy link
Member Author

Hey @alirezazd and @poname , this PR is about upgrading the Yosys+Odin-II front-end by using more Yosys side by performing some simple mapping and postponing technology mapping of the memory hard blocks and DSPs to Odin-II partial mapper. So going forward, please keep @aman26kbm posted about your updates/enhancements, specifically about this PR.

@sdamghan sdamghan reopened this Sep 10, 2022
@sdamghan sdamghan assigned sdamghan and unassigned sdamghan Sep 10, 2022
@vaughnbetz
Copy link
Contributor

CI is green and the code looks OK to me -- should this be merged at this point? @aman26kbm @sdamghan please let me know.

@aman26kbm
Copy link
Contributor

aman26kbm commented Oct 5, 2022

These changes were leading to some degradation in results for some designs (in addition to the improvement in many designs). So, we decided to hold off merging this PR until they are debugged.

@poname poname mentioned this pull request Dec 20, 2023
7 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Odin Odin II Logic Synthesis Tool: Unsorted item Yosys+Odin-II The Yosys+Odin-II synthesizer: the Yosys coarse-grained Tcl script and Odin-II partial mapping flow
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants