counter_test example hits yosys ABC9 assert during synth #120

Xiretza · 2021-01-25T22:23:54Z

As discussed on IRC, trying to build the xc7/counter_test example results in a failed assertion in yosys during synth with my build configuration. Below are the exact steps to (hopefully) reproduce this issue using vanilla/unpatched yosys, yosys-symbiflow-plugins, symbiflow-arch-defs and prjxray-db.

$ mkdir -p /tmp/reproduce/install/bin
$ export PATH=/tmp/reproduce/install/bin:$PATH

$ cd /tmp/reproduce

$ git clone https://github.com/YosysHQ/yosys
$ cd yosys
$ git checkout 2116c5858
$ make config-gcc
$ make -j4 PREFIX=/tmp/reproduce/install/
$ make PREFIX=/tmp/reproduce/install/ install
$ which yosys
/tmp/reproduce/install/bin/yosys
$ yosys -V
Yosys 0.9+3710 (git sha1 2116c585, gcc 10.2.0 -fPIC -Os)
$ cd ..

$ BASEURL=https://storage.googleapis.com/symbiflow-arch-defs/artifacts/prod/foss-fpga-tools/symbiflow-arch-defs/continuous/install/125/20210122-000052
$ curl "$BASEURL/symbiflow-arch-defs-install-bff52005.tar.xz" | tar -C /tmp/reproduce/install/ -Jxv
$ curl "$BASEURL/symbiflow-arch-defs-xc7a50t_test-bff52005.tar.xz" | tar -C /tmp/reproduce/install/ -Jxv
$ which symbiflow_synth
/tmp/reproduce/install/bin/symbiflow_synth

$ git clone https://github.com/SymbiFlow/prjxray-db
$ git -C prjxray-db checkout e7663ba6e

$ git clone https://github.com/SymbiFlow/yosys-symbiflow-plugins
$ cd yosys-symbiflow-plugins
$ git checkout 0a5b28faf
$ make -j4
$ make install
$ cd ..

$ git clone https://github.com/SymbiFlow/symbiflow-examples
$ cd symbiflow-examples/
$ git checkout dce2d6859
$ make -C xc7/counter_test DATABASE_DIR=/tmp/reproduce/prjxray-db/ TARGET=arty_35
make: Entering directory '/tmp/reproduce/symbiflow-examples/xc7/counter_test'
cd build/arty_35 && symbiflow_synth -t top -v /tmp/reproduce/symbiflow-examples/xc7/counter_test/counter.v -d artix7 -p xc7a35tcsg324-1 -x /tmp/reproduce/symbiflow-examples/xc7/counter_test/arty.xdc 2>&1 > /dev/null
ERROR: Assert `cell->parameters.empty()' failed in passes/techmap/abc9_ops.cc:781.
make: *** [Makefile:42: build/arty_35/top.eblif] Error 1
make: Leaving directory '/tmp/reproduce/symbiflow-examples/xc7/counter_test'

The failing step is the second ABC9 run, line 158 in /tmp/reproduce/install/share/symbiflow/scripts/xc7/synth.tcl.

I have previously dug into this crash using gdb and found the erroring cell to be $auto$alumacc.cc:485:replace_alu$1385.slice[0].carry4, line 65970 in /tmp/reproduce/symbiflow-examples/xc7/counter_test/build/arty_35/top.json.pre_abc9.ilang, though that was on my possibly dirty and patched-up system.

The text was updated successfully, but these errors were encountered:

Xiretza · 2021-01-25T22:28:57Z

Here are the checksums of my build files, I have also attached them in case the build isn't reproducible:

$ sha256sum /tmp/reproduce/symbiflow-examples/xc7/counter_test/build/arty_35/*
56ac092a000482c52ed3e5a3f5bb3a066be56b95176202bf0ed99d861a495b04  /tmp/reproduce/symbiflow-examples/xc7/counter_test/build/arty_35/top.json.carry_fixup.json
fe6eff717c2d8803d3d6d35bb0c6578efd7189d9f0fa1c6e7afffa2c3fa758f8  /tmp/reproduce/symbiflow-examples/xc7/counter_test/build/arty_35/top.json.carry_fixup_out.json
01ecd2c0e57ac657dcdfde150106dcbf3524e41a7643870f7d11198885b80dc4  /tmp/reproduce/symbiflow-examples/xc7/counter_test/build/arty_35/top.json.pre_abc9.ilang
e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855  /tmp/reproduce/symbiflow-examples/xc7/counter_test/build/arty_35/top.sdc
e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855  /tmp/reproduce/symbiflow-examples/xc7/counter_test/build/arty_35/top_fasm_extra.fasm
a8cfde35f0b6ff56b40425ac8e0d6362bfaae420c001d702a6c4d366e853eacb  /tmp/reproduce/symbiflow-examples/xc7/counter_test/build/arty_35/top_synth.log
71642684727d56d69c85f4b520ecc1262160cd87e627a27044b0fdac7159314e  /tmp/reproduce/symbiflow-examples/xc7/counter_test/build/arty_35/top_synth.v.premap.v

artifacts.tar.gz

kgugala · 2021-02-27T20:12:20Z

@Xiretza I know what is going on here. Yosys built with gcc used in packages builder CI does not call asserts at all - they are optimised out.

Here is the part of the code where you hit assert:

 776                 RTLIL::Module* box_module = design->module(cell->type);
 777                 if (!box_module)
 778                         continue;
 779                 if (!box_module->get_bool_attribute(ID::abc9_box))
 780                         continue;
 781                 log_assert(cell->parameters.empty());
 782                 log_assert(box_module->get_blackbox_attribute());
 783 
 784                 cell->attributes[ID::abc9_box_seq] = box_count++;

When built with the GCC used to build the package it gives the following result:

1220635                 RTLIL::Module* box_module = design->module(cell->type);                                        
1220636   4e13d9:       48 8b 44 24 30          mov    0x30(%rsp),%rax                                                 
1220637   4e13de:       48 89 df                mov    %rbx,%rdi                                                       
1220638   4e13e1:       4c 8d 70 4c             lea    0x4c(%rax),%r14                                                 
1220639   4e13e5:       4c 89 f6                mov    %r14,%rsi                                                       
1220640   4e13e8:       e8 a1 d0 c2 ff          callq  10e48e <_ZN5Yosys5RTLIL8IdStringC1ERKS1_>                       
1220641   4e13ed:       48 89 de                mov    %rbx,%rsi                                                       
1220642   4e13f0:       48 8b 7c 24 50          mov    0x50(%rsp),%rdi                                                 
1220643   4e13f5:       67 e8 f5 26 c4 ff       addr32 callq 123af0 <_ZN5Yosys5RTLIL6Design6moduleENS0_8IdStringE>     
1220644   4e13fb:       48 89 df                mov    %rbx,%rdi                                                       
1220645   4e13fe:       48 89 44 24 08          mov    %rax,0x8(%rsp)                                                  
1220646   4e1403:       e8 16 54 c3 ff          callq  11681e <_ZN5Yosys5RTLIL8IdStringD1Ev>                           
1220647                 if (!box_module)                                                                               
1220648   4e1408:       48 83 7c 24 08 00       cmpq   $0x0,0x8(%rsp)                                                  
1220649   4e140e:       0f 84 e7 06 00 00       je     4e1afb <_ZN12_GLOBAL__N_111prep_xaigerEPN5Yosys5RTLIL6ModuleEb+0x1357>                        
1220650                 if (!box_module->get_bool_attribute(ID::abc9_box))                                             
1220651   4e1414:       48 8b 44 24 08          mov    0x8(%rsp),%rax
1220652   4e1419:       48 8d 35 74 02 34 00    lea    0x340274(%rip),%rsi        # 821694 <_ZN5Yosys5RTLIL2ID8abc9_boxE>
1220653   4e1420:       48 89 df                mov    %rbx,%rdi
1220654   4e1423:       4c 8d 60 08             lea    0x8(%rax),%r12                                                  
1220655   4e1427:       e8 62 d0 c2 ff          callq  10e48e <_ZN5Yosys5RTLIL8IdStringC1ERKS1_>                       
1220656   4e142c:       48 89 de                mov    %rbx,%rsi
1220657   4e142f:       4c 89 e7                mov    %r12,%rdi
1220658   4e1432:       67 e8 7c 05 c4 ff       addr32 callq 1219b4 <_ZNK5Yosys5RTLIL10AttrObject18get_bool_attributeENS0_8IdStringE>        
1220659   4e1438:       41 88 c5                mov    %al,%r13b                                                       
1220660   4e143b:       48 89 df                mov    %rbx,%rdi
1220661   4e143e:       e8 db 53 c3 ff          callq  11681e <_ZN5Yosys5RTLIL8IdStringD1Ev>                           
1220662   4e1443:       45 84 ed                test   %r13b,%r13b                                                     
1220663   4e1446:       0f 84 af 06 00 00       je     4e1afb <_ZN12_GLOBAL__N_111prep_xaigerEPN5Yosys5RTLIL6ModuleEb+0x1357>                                
1220664                 cell->attributes[ID::abc9_box_seq] = box_count++;

All the log_assert calls have been optimised out.
The same dump from gcc on my machine gives the following output:

1284095                 if (!box_module)
1284096   52caba:       48 83 7c 24 20 00       cmpq   $0x0,0x20(%rsp)
1284097   52cac0:       0f 84 92 07 00 00       je     52d258 <_ZN12_GLOBAL__N_111prep_xaigerEPN5Yosys5RTLIL6ModuleEb+0x1669>
1284098                 if (!box_module->get_bool_attribute(ID::abc9_box))
1284099   52cac6:       48 8b 44 24 20          mov    0x20(%rsp),%rax
1284100   52cacb:       48 8d 35 a2 b6 34 00    lea    0x34b6a2(%rip),%rsi        # 878174 <_ZN5Yosys5RTLIL2ID8abc9_boxE>
1284101   52cad2:       48 89 ef                mov    %rbp,%rdi
1284102   52cad5:       4c 8d 68 08             lea    0x8(%rax),%r13
1284103   52cad9:       e8 20 5e c1 ff          callq  1428fe <_ZN5Yosys5RTLIL8IdStringC1ERKS1_>
1284104   52cade:       48 89 ee                mov    %rbp,%rsi
1284105   52cae1:       4c 89 ef                mov    %r13,%rdi
1284106   52cae4:       e8 6b cc c2 ff          callq  159754 <_ZNK5Yosys5RTLIL10AttrObject18get_bool_attributeENS0_8IdStringE>
1284107   52cae9:       89 c3                   mov    %eax,%ebx
1284108   52caeb:       48 89 ef                mov    %rbp,%rdi
1284109   52caee:       e8 03 06 c2 ff          callq  14d0f6 <_ZN5Yosys5RTLIL8IdStringD1Ev>
1284110   52caf3:       84 db                   test   %bl,%bl
1284111   52caf5:       0f 84 5d 07 00 00       je     52d258 <_ZN12_GLOBAL__N_111prep_xaigerEPN5Yosys5RTLIL6ModuleEb+0x1669>
1284112     { return __lhs.base() == __rhs.base(); }
1284113   52cafb:       48 8b 44 24 08          mov    0x8(%rsp),%rax
1284114   52cb00:       31 ff                   xor    %edi,%edi
1284115                 log_assert(cell->parameters.empty());
1284116   52cb02:       48 8d 15 06 c5 1f 00    lea    0x1fc506(%rip),%rdx        # 72900f <_ZTSSt5_BindIFMN12_GLOBAL__N_118abc9_output_filterEFvRKNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEEES1_St12_PlaceholderILi1EEEE+0xe0f>
1284117   52cb09:       48 8d 35 97 cb 1f 00    lea    0x1fcb97(%rip),%rsi        # 7296a7 <_ZTSSt5_BindIFMN12_GLOBAL__N_118abc9_output_filterEFvRKNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEEES1_St12_PlaceholderILi1EEEE+0x14a7>
1284118   52cb10:       48 8b 88 a0 00 00 00    mov    0xa0(%rax),%rcx
1284119   52cb17:       48 39 88 a8 00 00 00    cmp    %rcx,0xa8(%rax)
1284120   52cb1e:       b9 0d 03 00 00          mov    $0x30d,%ecx
1284121   52cb23:       40 0f 94 c7             sete   %dil
1284122   52cb27:       e8 18 6a ff ff          callq  523544 <_ZN5YosysL17log_assert_workerEbPKcS1_i>
1284123                 log_assert(box_module->get_blackbox_attribute());
1284124   52cb2c:       31 f6                   xor    %esi,%esi
1284125   52cb2e:       4c 89 ef                mov    %r13,%rdi
1284126   52cb31:       e8 be 5f c6 ff          callq  192af4 <_ZNK5Yosys5RTLIL10AttrObject22get_blackbox_attributeEb>
1284127   52cb36:       0f b6 f8                movzbl %al,%edi
1284128   52cb39:       b9 0e 03 00 00          mov    $0x30e,%ecx
1284129   52cb3e:       48 8d 15 ca c4 1f 00    lea    0x1fc4ca(%rip),%rdx        # 72900f <_ZTSSt5_BindIFMN12_GLOBAL__N_118abc9_output_filterEFvRKNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEEES1_St12_PlaceholderILi1EEEE+0xe0f>
1284130   52cb45:       48 8d 35 65 ca 1f 00    lea    0x1fca65(%rip),%rsi        # 7295b1 <_ZTSSt5_BindIFMN12_GLOBAL__N_118abc9_output_filterEFvRKNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEEES1_St12_PlaceholderILi1EEEE+0x13b1>
1284131   52cb4c:       e8 f3 69 ff ff          callq  523544 <_ZN5YosysL17log_assert_workerEbPKcS1_i>
1284132                 cell->attributes[ID::abc9_box_seq] = box_count++;

Here, asserts calls are present. I got similar results with clang.

I don't know yet, why the asserts are optimised out. However they seem to be too strict - we can get a working netlist without them afterwards.

@Xiretza for now you can simply comment out all the parameters.empty() asserts - there is a few of them in backends/aiger/xaiger.cc and passes/techmap/abc9_ops.cc

Xiretza · 2021-02-27T20:24:09Z

What's the yosys -V of the CI build? Maybe it's built with ENABLE_NDEBUG?

kgugala · 2021-02-27T20:28:10Z

Yosys 0.9+3710 (git sha1 2116c585, x86_64-conda_cos6-linux-gnu-gcc 1.23.0.449-a04d0 -fvisibility-inlines-hidden -fmessage-length=0 -march=nocona -mtune=haswell -ftree-vectorize -fPIC -fstack-protector-strong -fno-plt -O2 -ffunction-sections -fPIC -Os -fno-merge-constants)

Arvind-Srinivasan · 2021-02-27T22:31:50Z

I can confirm the the issue is resolved on my system after commenting out those asserts when I compiled the latest Yosys version Yosys 0.9+3683 (git sha1 40d9e120, gcc 10.2.0-13ubuntu1 -fPIC -Os) from the Symbiflow Yosys Repo and using the pinned commit 40efa51 of the Yosys Symbiflow Plugins

Ravenslofty · 2021-03-01T18:56:05Z

Alternative fix:

--- /tmp/reproduce/install/share/symbiflow/techmaps/xc7_vpr/techmap/cells_sim.v.orig    2021-03-01 18:50:07.403552300 +0000
+++ /tmp/reproduce/install/share/symbiflow/techmaps/xc7_vpr/techmap/cells_sim.v 2021-03-01 18:45:35.903552300 +0000
@@ -85,7 +85,7 @@

 endmodule

-(* abc9_box, blackbox *)
+(* abc9_box, lib_whitebox *)
 module CARRY4_VPR(O0, O1, O2, O3, CO0, CO1, CO2, CO3, CYINIT, CIN, DI0, DI1, DI2, DI3, S0, S1, S2, S3);
   parameter CYINIT_AX = 1'b0;
   parameter CYINIT_C0 = 1'b0;

I think this is a poor error message on the part of ABC9, but I think the assert is sound and correct.

litghost · 2021-03-01T18:58:51Z

Will lib_whitebox allow ABC9 to optimize through the CARRY4? Because we specifically need ABC9 to leave the CARRY4 alone at this part of the flow. I want ABC9 to be able to do timing analysis of the CARRY4, but only want it handle the LUT's on the outputs, not try to optimize the carry logic itself.

Ravenslofty · 2021-03-01T19:17:11Z

Will lib_whitebox allow ABC9 to optimize through the CARRY4? Because we specifically need ABC9 to leave the CARRY4 alone at this part of the flow.

Yes, though I believe it only removes completely-undriven carry logic. I think the solution is for the Python script to mark all CARRY4_VPR cells as (* keep *), which turns carry logic pins into primary inputs/outputs when passed to ABC.

litghost · 2021-03-01T19:20:56Z

Will lib_whitebox allow ABC9 to optimize through the CARRY4? Because we specifically need ABC9 to leave the CARRY4 alone at this part of the flow.

Yes, though I believe it only removes completely-undriven carry logic. I think the solution is for the Python script to mark all CARRY4_VPR cells as (* keep *), which turns carry logic pins into primary inputs/outputs when passed to ABC.

In the past, ABC9 was smart enough to completely swap pins and merge apparently redundant logic (from ABC's perspective), so we need something to suppress that behavior. If (* keep *) will do that, that should work.

Ravenslofty · 2021-03-01T19:23:36Z

The Yosys pass itself uses this behaviour to preserve logic loops by hiding them from ABC.

https://github.com/YosysHQ/yosys/blob/5aa35b8992fab8b55c1c1fae793b4ad845fd4c4c/backends/aiger/xaiger.cc#L157-L161

GitHub
YosysHQ/yosys
Yosys Open SYnthesis Suite. Contribute to YosysHQ/yosys development by creating an account on GitHub.

litghost · 2021-03-01T19:28:03Z

The Yosys pass itself uses this behaviour to preserve logic loops by hiding them from ABC.

https://github.com/YosysHQ/yosys/blob/5aa35b8992fab8b55c1c1fae793b4ad845fd4c4c/backends/aiger/xaiger.cc#L157-L161

If I'm reading that right, we need the (*keep*) on the relevant wires? Or is there similar logic to handle a (*keep*) on the cell?

Ravenslofty · 2021-03-01T19:44:50Z

Hmm. It seems like keep might be incompatible with marking modules as (* abc9_box *), judging by this assert? I suppose whether white box or black box, marking a module as (* abc9_box *) means you're permitting that module to be optimised and/or swept away.

I might have to talk to Eddie about this.

litghost · 2021-03-01T19:50:58Z

Hmm. It seems like keep might be incompatible with marking modules as (* abc9_box *), judging by this assert? I suppose whether white box or black box, marking a module as (* abc9_box *) means you're permitting that module to be optimised and/or swept away.

I might have to talk to Eddie about this.

So for clarity the goal is:

Continue to have ABC9 be able to compute combinatorial delays through the CARRY4 for delay path analysis. I believe ABC9 needs to have a abc9_box for this?
Prevent ABC9 from optimizing the CARRY4 away or collapsing logic into the CARRY4. This was the reason that blackbox was used over lib_whitebox
Allow ABC9 to re-arrange other LUT's or logic elements that have been changed now that additional logic elements are in use.

Ravenslofty · 2021-03-01T20:04:07Z

Here's another approach I'm going to throw out: I know that ABC9 handles boxes marked blackbox fine, the problem is the module also being parametric (which implies potential incompleteness). A hacky idea is to map the cell into a temporary box that contains timing and path information but stores the parameter information via input wires connected to constants, and then map the boxes back to the cell with the right parameters. It's awful, but it works.

litghost · 2021-03-01T21:34:05Z

The easiest short term solution would be to remove both abc9_box and blackbox and see if the QoR changes significantly. It is possible that ABC having the combinatoric delays through the CARRY4 might be not that important for this part.

Ravenslofty · 2021-03-01T21:46:06Z

Do you want me to modify the PR so you can run CI, or are you going to create your own?

litghost · 2021-03-01T22:11:02Z

Do you want me to modify the PR so you can run CI, or are you going to create your own?

Always appreciate the help.

Ravenslofty · 2021-03-05T21:52:10Z

Since a workaround got merged, is this going to be closed, or will that wait for the "proper" fix?

litghost · 2021-03-05T21:54:35Z

I believe the latest files have the update. @Xiretza are you willing to re-test to see if this is fixed for you?

Arvind-Srinivasan · 2021-03-05T22:04:06Z

I can confirm the issue is now fixed with the removal of the blackbox line. I commented back in the asserts in the two files and they no longer trigger.

Xiretza · 2021-03-05T22:40:53Z

This particular issue seems to be resolved, but I'm hitting the equivalent of #127 in my own project.

Arvind-Srinivasan mentioned this issue Feb 28, 2021

picosoc_demo example hits yosys ABC9 assert during synth #127

Open

CajuM mentioned this issue Mar 1, 2021

yosys should not be built with debug flags NixOS/nixpkgs#114719

Closed

litghost mentioned this issue Mar 1, 2021

Mark CARRY4_VPR as lib_whitebox f4pga/f4pga-arch-defs#2061

Merged

Xiretza closed this as completed Mar 5, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

counter_test example hits yosys ABC9 assert during synth #120

counter_test example hits yosys ABC9 assert during synth #120

Xiretza commented Jan 25, 2021

Xiretza commented Jan 25, 2021

kgugala commented Feb 27, 2021 •

edited

Xiretza commented Feb 27, 2021

kgugala commented Feb 27, 2021

Arvind-Srinivasan commented Feb 27, 2021 •

edited

Ravenslofty commented Mar 1, 2021

litghost commented Mar 1, 2021

Ravenslofty commented Mar 1, 2021

litghost commented Mar 1, 2021 •

edited

Ravenslofty commented Mar 1, 2021 •

edited by unfurl-links bot

litghost commented Mar 1, 2021 •

edited

Ravenslofty commented Mar 1, 2021

litghost commented Mar 1, 2021 •

edited

Ravenslofty commented Mar 1, 2021 •

edited

litghost commented Mar 1, 2021

Ravenslofty commented Mar 1, 2021

litghost commented Mar 1, 2021

Ravenslofty commented Mar 5, 2021

litghost commented Mar 5, 2021 •

edited

Arvind-Srinivasan commented Mar 5, 2021

Xiretza commented Mar 5, 2021 •

edited

counter_test example hits yosys ABC9 assert during synth #120

counter_test example hits yosys ABC9 assert during synth #120

Comments

Xiretza commented Jan 25, 2021

Xiretza commented Jan 25, 2021

kgugala commented Feb 27, 2021 • edited

Xiretza commented Feb 27, 2021

kgugala commented Feb 27, 2021

Arvind-Srinivasan commented Feb 27, 2021 • edited

Ravenslofty commented Mar 1, 2021

litghost commented Mar 1, 2021

Ravenslofty commented Mar 1, 2021

litghost commented Mar 1, 2021 • edited

Ravenslofty commented Mar 1, 2021 • edited by unfurl-links bot

litghost commented Mar 1, 2021 • edited

Ravenslofty commented Mar 1, 2021

litghost commented Mar 1, 2021 • edited

Ravenslofty commented Mar 1, 2021 • edited

litghost commented Mar 1, 2021

Ravenslofty commented Mar 1, 2021

litghost commented Mar 1, 2021

Ravenslofty commented Mar 5, 2021

litghost commented Mar 5, 2021 • edited

Arvind-Srinivasan commented Mar 5, 2021

Xiretza commented Mar 5, 2021 • edited

kgugala commented Feb 27, 2021 •

edited

Arvind-Srinivasan commented Feb 27, 2021 •

edited

litghost commented Mar 1, 2021 •

edited

Ravenslofty commented Mar 1, 2021 •

edited by unfurl-links bot

litghost commented Mar 1, 2021 •

edited

litghost commented Mar 1, 2021 •

edited

Ravenslofty commented Mar 1, 2021 •

edited

litghost commented Mar 5, 2021 •

edited

Xiretza commented Mar 5, 2021 •

edited