siliconcompiler / lambdapdk

Library of open source Process Design Kits (PDKs)
Apache License 2.0
15 stars 3 forks source link

Importing STD cells libraries #34

Closed sebinho closed 1 week ago

sebinho commented 2 weeks ago

Hi,

I am currently in the process of importing a proprietary PDK into lambadapdk and SiliconCompiler. The vendor provides many libraries that are split across different liberty files. To make a long story short, all the cells I need are split into 3 different liberty files:

I am importing those various files using the following command:

lib.add('output', 'typical', 'nldm', 'core.lib.gz')
lib.add('output', 'typical', 'nldm', 'clk.lib.gz')
lib.add('output', 'typical', 'nldm', 'PR.lib.gz')

As yosys (and its abc call) should only use the core library, I add the following command: lib.set('option', 'file', 'yosys_dff_liberty', 'core.lib.gz')

All my files are respecting the correct folder structure like the Sky130 and other examples. I then call the python script generate_lambdalib.py to create all the lambda files for my new PDK.

If I remove the PR library, it works well and it can generate all the verilog wrappers for my PDK (in the lambda folder of my PDK). But if I keep included the PR liberty file, I get the following errors:

| ERROR   | siliconcompiler.targets.xxx040_demo-LIBNAME | syn       | 0 | ABC: Scl_LibertyReadGenlib() skipped cell "FILLERCELL1" without logic function.
| ERROR   | siliconcompiler.targets.xxx040_demo-LIBNAME | syn       | 0 | ABC: Scl_LibertyReadGenlib() skipped cell "FILLERCELL2" without logic function.
| ERROR   | siliconcompiler.targets.xxx040_demo-LIBNAME | syn       | 0 | ABC: Scl_LibertyReadGenlib() skipped cell "FILLERCELL3" without logic function.

And there are more errors like this. These cells are typically required for OpenRoad when doing P&R.

Does someone know what is going wrong and how to get around this? I suppose that if I add those cells to the dontuse list they will not be available for P&R.... Do I need to manually edit the liberty files?

Thanks for your help!

sebinho commented 2 weeks ago

My issues seem to be related to filler cells as they do not contain any description. Maybe those cells have to be removed from the PR liberty file to avoid those errors?

I do see in Sky130 that those filler cells are not included in the liberty file. I am not very familiar with backend design, just trying to get things running.

gadfort commented 2 weeks ago

@sebinho fill cells and decap cells are fine to have in the liberty files, usually just there to provide power modeling. We have several PDKs this has run on with fill and decap cells without issue. Without being able to see the files it would be hard to provide better feedback on why ABC is failing, my initial guess would be that some of the power and ground pins in the liberty file has been marked as pin() instead of pg_pin().

sebinho commented 2 weeks ago

@sebinho fill cells and decap cells are fine to have in the liberty files, usually just there to provide power modeling. We have several PDKs this has run on with fill and decap cells without issue. Without being able to see the files it would be hard to provide better feedback on why ABC is failing, my initial guess would be that some of the power and ground pins in the liberty file has been marked as pin() instead of pg_pin().

@gadfort thanks for your reply. Here below is an example of a cell that is creating an error (snippet from the liberty file)

  cell(FILLERCELL1){
   area : 0.3528;
   cell_footprint : AF58C13EF43B357793A1A26D1BB87822;
   cell_leakage_power : 3.1387e-14;
   is_filler_cell : true;
   threshold_voltage_group : "vt=LL,tr=14,h=1G";
   pg_pin(gnd){
    pg_type : primary_ground;
    related_bias_pin : gnds;
    voltage_name : gnd;
   }
   pg_pin(gnds){
    pg_type : pwell;
    physical_connection : device_layer;
    voltage_name : gnds;
   }
   pg_pin(vdd){
    pg_type : primary_power;
    related_bias_pin : vdds;
    voltage_name : vdd;
   }
   pg_pin(vdds){
    pg_type : nwell;
    physical_connection : device_layer;
    voltage_name : vdds;
   }
   leakage_power(){
    related_pg_pin : vdds;
    value : 3.1387e-14;
   }
   leakage_power(){
    related_pg_pin : vdd;
    value : 1e-20;
   }
  }

Any ideas? It seems that it is already using pg_pin()

sebinho commented 2 weeks ago

I am able to remove those errors, but I have to manually (I could for sure script this) remove the cells from the liberty file.

As a side note, my liberty files have a lot of cells that contain the dont_use : true parameter, and those cells create errors with ABC:

| ERROR   | siliconcompiler.targets.stcmos040_demo-LIBNAME | syn       | 0 | ABC: Scl_LibertyReadGenlib() skipped cell "CELL1" due to dont_use attribute.
| ERROR   | siliconcompiler.targets.stcmos040_demo-LIBNAME | syn       | 0 | ABC: Scl_LibertyReadGenlib() skipped cell "CELL2" due to dont_use attribute.

Do I have to manually add all those cells to the dont_use python list of my asic?

Thanks

gadfort commented 2 weeks ago

Why are these getting logged as errors? I looked at one of our runs and there these are just messages. Your file looks correct. What version of SC are you using and what version of yosys are you using?

sebinho commented 2 weeks ago

Why are these getting logged as errors? I looked at one of our runs and there these are just messages. Your file looks correct. What version of SC are you using and what version of yosys are you using?

I am using SiliconCompiler v0.21.7 and Yosys Yosys 0.39+124 (git sha1 d73f71e81, g++ 13.2.1 -fPIC -Os)

sebinho commented 2 weeks ago

I did an upgrade of Yosys to the most recent version Yosys 0.40 (git sha1 a1bb0255d, g++ 13.2.1 -fPIC -Os) and I get the same errors. I did the same with SiliconCompiler for its latest version, same issue

gadfort commented 2 weeks ago

I'm unable to replicate the issue so it's likely something in the liberty file that is at issue, without access there isn't much I can suggest. if dont_use works you can start with that. You might want to look through the rest of the log to make sure there aren't other problems earlier in the log

sebinho commented 2 weeks ago

Thanks for the help. I will investigate further and comment any findings…

sebinho commented 2 weeks ago

It seems that I now get slightly different results. When I look at the syn.log file, I can see a warning preceding an error. So it does not seem that the filler cells or the don't use attributes are the problem. See log here below. It says warning: Templates are not defined.

ABC: Parsing finished successfully. Parsing time = 0.00 sec ABC: Warning: Templates are not defined. ABC: Scl_LibertyReadGenlib() skipped cell "DECAP1" without logic function. ABC: Scl_LibertyReadGenlib() skipped cell "DECAP2" without logic function. ABC: Scl_LibertyReadGenlib() skipped cell "DECAP3" without logic function. ABC: Scl_LibertyReadGenlib() skipped cell "DECAP4" without logic function. ABC: Scl_LibertyReadGenlib() skipped cell "ANTPROT2" due to dont_use attribute. ABC: Scl_LibertyReadGenlib() skipped cell "FILLERCELL1" without logic function. ABC: Scl_LibertyReadGenlib() skipped cell "FILLERCELL2" without logic function. ABC: Scl_LibertyReadGenlib() skipped cell "FILLERCELL3" without logic function. ABC: Scl_LibertyReadGenlib() skipped cell "FILLERCELL4" without logic function. ABC: Scl_LibertyReadGenlib() skipped cell "FILLERCELL8" without logic function. ABC: Scl_LibertyReadGenlib() skipped cell "FILLERNPW3" without logic function. ABC: Scl_LibertyReadGenlib() skipped cell "FILLERNPW4" without logic function. ABC: Scl_LibertyReadGenlib() skipped cell "FILLERPFOP1" without logic function. ABC: Scl_LibertyReadGenlib() skipped cell "FILLERPFOP10" without logic function. ABC: Scl_LibertyReadGenlib() skipped cell "FILLERPFOP12" without logic function. ABC: Scl_LibertyReadGenlib() skipped cell "FILLERPFOP16" without logic function. ABC: Scl_LibertyReadGenlib() skipped cell "FILLERPFOP2" without logic function. ABC: Scl_LibertyReadGenlib() skipped cell "FILLERPFOP32" without logic function. ABC: Scl_LibertyReadGenlib() skipped cell "FILLERPFOP4" without logic function. ABC: Scl_LibertyReadGenlib() skipped cell "FILLERPFOP64" without logic function. ABC: Scl_LibertyReadGenlib() skipped cell "FILLERPFOP8" without logic function. ABC: Scl_LibertyReadGenlib() skipped cell "FILLERSNPW3" without logic function. ABC: Scl_LibertyReadGenlib() skipped cell "FILLERSNPW4" without logic function. ABC: Scl_LibertyReadGenlib() skipped cell "ANTPROT2" due to dont_use attribute. ABC: Scl_LibertyReadGenlib() skipped cell "DECAPXT10" without logic function. ABC: Scl_LibertyReadGenlib() skipped cell "DECAPXT12" without logic function. ABC: Scl_LibertyReadGenlib() skipped cell "DECAPXT16" without logic function. ABC: Scl_LibertyReadGenlib() skipped cell "DECAPXT32" without logic function. ABC: Scl_LibertyReadGenlib() skipped cell "DECAPXT64" without logic function. ABC: Scl_LibertyReadGenlib() skipped cell "DECAPXT7" without logic function. ABC: Scl_LibertyReadGenlib() skipped cell "DECAPXT8" without logic function. ABC: Scl_LibertyReadGenlib() skipped cell "FILLERCELL1" without logic function. ABC: Scl_LibertyReadGenlib() skipped cell "FILLERCELL2" without logic function. ABC: Scl_LibertyReadGenlib() skipped cell "FILLERCELL3" without logic function. ABC: Scl_LibertyReadGenlib() skipped cell "FILLERCELL4" without logic function. ABC: Scl_LibertyReadGenlib() skipped cell "FILLERCELL8" without logic function. ABC: Scl_LibertyReadGenlib() skipped cell "FILLERNPW3" without logic function. ABC: Scl_LibertyReadGenlib() skipped cell "FILLERNPW4" without logic function. ABC: Scl_LibertyReadGenlib() skipped cell "FILLERPFOP1" without logic function. ABC: Scl_LibertyReadGenlib() skipped cell "FILLERPFOP10" without logic function. ABC: Scl_LibertyReadGenlib() skipped cell "FILLERPFOP12" without logic function. ABC: Scl_LibertyRERROR: Can't open ABC output file '/tmp/yosys-abc-ds8BwW/output.blif'.

@gadfort Do you know what this warning/error means?

I found a similar error on Yosys issues board, seems to come from a liberty file that is not complete enough. But my liberty file is coming straight from the vendor and is a proven library. Strange. I don't get this problem with the other libraries I have (like core and clock cells libraries). https://github.com/YosysHQ/yosys/issues/815

thx

sebinho commented 2 weeks ago

Apologies if I am dropping too much information here. My real usecase is not the one mentioned in my previous message. The usecase is when I use all the different libraries at the same time. When I do that, and that I look at the log, I see the following error:

ABC: Scl_LibertyReadGenlib()ERROR: ABC: execution of command ""/opt/yosys/bin/yosys-abc" -s -f /tmp/yosys-abc-Epxflz/abc.script 2>&1" failed: return code 134.

If I then try to execute this command in a terminal I get the following:

> /opt/yosys/bin/yosys-abc -s -f /tmp/yosys-abc-Epxflz/abc.script
...
yosys-abc: src/map/scl/sclLiberty.c:1040: void abc::Scl_LibertyDumpTables(Vec_Str_t*, Vec_Flt_t*, Vec_Flt_t*, Vec_Flt_t*): Assertion `Vec_FltSize(vInd1) * Vec_FltSize(vInd2) == Vec_FltSize(vValues)' failed.
Aborted (core dumped)

Seems to be something deep down yosys. I might have to open an issue on Yosys. No clue at all what is happening

sebinho commented 2 weeks ago

From what I could gather so far, it seems to be yosys-abc crashing due to the size of the liberty file (my file is 400MB++). If I merge my liberty files excluding some to get it smaller (360MB), it does not crash....

ubfx commented 2 weeks ago

Without having seen the liberty and without having looked deeper into the ABC source code, this looks like it's checking for consistency of timing / power tables in the liberty. The tables have two indices, relating to rows and columns of the table, and the table itself. For the table to be complete, the number of values in the table has to be the product of the number of row and column indices.

Here is an example from sky130:

 normalized_driver_waveform ("driver_waveform_template") {
        index_1("0.0100000000, 0.0230506000, 0.0531329000, 0.1224745000, 0.2823108000, 0.5000000000, 0.6507428000, 1.5000000000");
        index_2("0.0000000000, 0.5000000000, 1.0000000000");
        driver_waveform_name : "ramp";
        values("0.0000000000, 0.0083333333, 0.0166666670", \
            "0.0000000000, 0.0192088180, 0.0384176350", \
            "0.0000000000, 0.0442774400, 0.0885548810", \
            "0.0000000000, 0.1020620700, 0.2041241500", \
            "0.0000000000, 0.2352590100, 0.4705180100", \
            "0.0000000000, 0.4166666700, 0.8333333300", \
            "0.0000000000, 0.5422856800, 1.0845714000", \
            "0.0000000000, 1.2500000000, 2.5000000000");
    }

If I merge my liberty files excluding some to get it smaller (360MB), it does not crash....

Maybe there's a cell somewhere in the liberties that you excluded, which has a malformed table, i.e. too few or too many values to match the indices?

sebinho commented 2 weeks ago

Without having seen the liberty and without having looked deeper into the ABC source code, this looks like it's checking for consistency of timing / power tables in the liberty. The tables have two indices, relating to rows and columns of the table, and the table itself. For the table to be complete, the number of values in the table has to be the product of the number of row and column indices.

Here is an example from sky130:

 normalized_driver_waveform ("driver_waveform_template") {
        index_1("0.0100000000, 0.0230506000, 0.0531329000, 0.1224745000, 0.2823108000, 0.5000000000, 0.6507428000, 1.5000000000");
        index_2("0.0000000000, 0.5000000000, 1.0000000000");
        driver_waveform_name : "ramp";
        values("0.0000000000, 0.0083333333, 0.0166666670", \
            "0.0000000000, 0.0192088180, 0.0384176350", \
            "0.0000000000, 0.0442774400, 0.0885548810", \
            "0.0000000000, 0.1020620700, 0.2041241500", \
            "0.0000000000, 0.2352590100, 0.4705180100", \
            "0.0000000000, 0.4166666700, 0.8333333300", \
            "0.0000000000, 0.5422856800, 1.0845714000", \
            "0.0000000000, 1.2500000000, 2.5000000000");
    }

If I merge my liberty files excluding some to get it smaller (360MB), it does not crash....

Maybe there's a cell somewhere in the liberties that you excluded, which has a malformed table, i.e. too few or too many values to match the indices?

thanks a lot for your insights. I have a similar table as in Sky130 and the table has the right format.

For the time being, I reverted back to not excluding any cells from the liberty files. And I get the error mentioned above.

So basically I have 3 liberty files, let's call the A,B and C. A is the core lib, B is the clock lib, C is the P&R lib. When I include A and B (410MB), it does not work. When I include A and C (360MB), it works. When I include A,B and C (412MB) it does not work. When I include A alone, it works. When I include B alone, it works. When I include C alone, it does not work. But this is the P&R library and most of the cells are empty. In this case I also get the warning warning: Templates are not defined.

So I am a bit clueless for the time being, just trying things here and there. My only idea for the moment is to shrink the size of my core library as it includes lots of information I don't need right now like:

intrinsic_parasitic()
leakage_current()
dynamic_current()

These infos take up most of the size for the library.

ubfx commented 2 weeks ago

Did you have a look at the .lib files in the build/la_.../[target]/syn/0/inputs folder? Maybe they get mangled somehow?

sebinho commented 2 weeks ago

Did you have a look at the .lib files in the build/la_.../[target]/syn/0/inputs folder? Maybe they get mangled somehow?

@ubfx it does seem to be indeed something related to those Liberty tables.

What I noticed, is that each of my liberty file, contains at the beginning a bunch of templates and some of those templates have the same name for different libraries. When the tool does merge the libraries, it seems to only keep the templates of the first liberty file added. I suppose this is creating the issue and creating inconsistencies when using different liberty files....

I attached an example of the templates for both my CORE and CLK liberty files to exhibit the issue.

Is there a way to deal with such a setup?

test_core.txt test_clk.txt

ubfx commented 2 weeks ago

I think the automatic liberty merging will never work well enough to catch these advanced use cases which require a more in-depth parsing of the liberty. As far as I understand it, the merging is a necessary evil because ABC doesn't support reading in multiple .lib files. So the "right" solution would probably not be improving the merging script but teaching ABC to handle multiple liberty files.

As for a practical solution for your case, I would probably pre-process the libraries by hand/script, replacing the names of all the lookup table templates with table_n_core, table_n_clk, table_n_pr etc. in each of the different liberty files and then merge them by hand. Then I would just feed them as a single .lib.gz to the flow and it should hopefully work.

gadfort commented 2 weeks ago

@sebinho @ubfx yes the library merging is an issue. It's a limitation of abc which is known, but at present no one is actively working to fix that. I think there is a relatively easy fix for this. If you set : 'library', lib, 'option', 'file', 'yosys_synthesis_libraries', these are the files yosys will use and we will merge. Just add all but the PR one and you should do good.

sebinho commented 2 weeks ago

@gadfort @ubfx I did write a script to replace the names of the templates in my liberty files. The last remaining issue is the one I can see in the code form SiliconCompiler here below: https://github.com/siliconcompiler/siliconcompiler/blob/d4aa1418e4ea612c74604427463fcbd9d1a5ac70/siliconcompiler/tools/yosys/mergeLib.py

When merging libraries, it only copies the header of the 1st library that is added. Then it will copy all the cells from the different libraries. Ideally, in my case, it should allow to also copy all the templates that exist in the different libraries, not just the first one. As a consequence I have to write a script to also merge all my liberty files upfront.....

Not sure that is something relevant to update in the future for SiliconCompiler. But in this case it would then also need to rename any duplicate in the templates. I use a PDK from one of the big names, so I suppose this could happen to someone else in the future when using proprietary PDKs.

What I might end up doing is forking SiliconCompiler to add any custom changes I need.

sebinho commented 2 weeks ago

@sebinho @ubfx yes the library merging is an issue. It's a limitation of abc which is known, but at present no one is actively working to fix that. I think there is a relatively easy fix for this. If you set : 'library', lib, 'option', 'file', 'yosys_synthesis_libraries', these are the files yosys will use and we will merge. Just add all but the PR one and you should do good.

thanks for your proposal. The issue is that my CLK liberty file also has templates that have duplicates...

gadfort commented 2 weeks ago

@sebinho my preference would be to get the fix into abc so we don't have to worry about this merging step since it's going to be an issue. You can always open a pull request with your changes if they are useful to others.

sebinho commented 2 weeks ago

@sebinho my preference would be to get the fix into abc so we don't have to worry about this merging step since it's going to be an issue. You can always open a pull request with your changes if they are useful to others.

So do you mean for ABC to add support for distinct libraries? This might be tricky for me to implement as I don't have any C/C++ experience and I am not sure people will be interested in supporting this new feature. I can always ask.

In the meantime, I need a fix asap, so I will probably perform this in the python script of SC. This cannot harm as there is already a merge step in SC and my update would not break anything but will make sure my issue does not happen to someone else.

gadfort commented 2 weeks ago

@sebinho yes, technically ABC has some support, but it doesn't work right. I'll check with someone to see what is needed to get a fix into ABC and yosys/ABC so we don't have to worry about this merging step (you will need to rely on your script for a while until the limitations in the tools are corrected).

sebinho commented 2 weeks ago

@gadfort thanks for doing that. It would great to eventually get support into abc. I can use my script for now. I will let you know a bit later today if it completely solves the issue.

sebinho commented 2 weeks ago

@gadfort, I was able to fix my problem by integrating changes in tools/yosys/mergeLib.py. I can now use different liberty files without any errors.

Are you interested in a pull request? Just let me know. My changes can be found here: https://github.com/siliconcompiler/siliconcompiler/commit/3d6c8fe3397002f2affe03f36f68c60c726d5e97

gadfort commented 2 weeks ago

@sebinho if you have the changes done, sure, please open a PR in SC, for this kind of change please also include a test case.

gadfort commented 2 weeks ago

@sebinho FYI: https://github.com/berkeley-abc/abc/pull/285 Once this is merged, an additional PR will be needed in Yosys to expose this flag, but that should allow us to completely remove the mergeLib.py script.

sebinho commented 2 weeks ago

@sebinho FYI: berkeley-abc/abc#285 Once this is merged, an additional PR will be needed in Yosys to expose this flag, but that should allow us to completely remove the mergeLib.py script.

Will this new flag allow proper merging including taking care of potential template conflicts like I had? Thx

sebinho commented 2 weeks ago

Ok. I will make a PR on Monday with a small test case.

gadfort commented 2 weeks ago

@sebinho FYI: berkeley-abc/abc#285 Once this is merged, an additional PR will be needed in Yosys to expose this flag, but that should allow us to completely remove the mergeLib.py script.

Will this new flag allow proper merging including taking care of potential template conflicts like I had? Thx

@sebinho I think so, I mocked up a test where I renamed all the templates in one file and merged them and ABC was fine with that.

gadfort commented 1 week ago

Resolved with: https://github.com/siliconcompiler/siliconcompiler/pull/2359