Address some &mfs crashes
#30
Conversation
The former implementation of `Sfm_NtkDfs` was trying to serialize the network while ordering all box inputs ahead of the box outputs. This is sometimes impossible, leaving the result unordered, which led to crashes in the `&mfs` code when it was reintegrating the result into the GIA structure: ABC: Assertion failed: iLitNew >= 0 (src/aig/gia/giaMfs.c: Gia_ManInsertMfs: 388) With a small change to `Gia_ManInsertMfs` which does the reintegration we don't really need the ordering to see through boxes, ordering on the paths between boxes is sufficient. Relaxing the ordering requirement, we make `Sfm_NtkDfs` robust.
When the network is being handed over to the "sfm" core, all blackboxes are modeled by inserting new PIs, POs, and those being connected by buffers to the nodes representing the CIs, COs. Make two changes: * Robustly deny the fake PIs from being considered when shopping for LUT fanin substitutions. Such reconnection occurring would trip up the code reintegrating the result. * Make sure the buffer connecting the fake-PO to the CO doesn't get rewritten as part of the `mfs` transformation, and extend this protection to any whitebox models.
There was a problem hiding this comment.
Tiny nitpick, but thanks to @yrabbit generously testing this on the Project Apicula test suite, I am reasonably confident this doesn't break code in the wild, and even helps in some cases.
|
Should this be upstreamed to abc? |
|
@QuantamHD Yes, but because of discouraging stories about how others tried to upstream ABC fixes, and because |
| @@ -170,14 +171,33 @@ Vec_Int_t * Sfm_NtkDfs( Sfm_Ntk_t * p, Vec_Wec_t * vGroups, Vec_Int_t * vGroupMa | |||
| Vec_IntClear( vBoxesLeft ); | |||
| vNodes = Vec_IntAlloc( p->nObjs ); | |||
| Sfm_NtkIncrementTravId( p ); | |||
| if ( fAllBoxes ) | |||
|
|
|||
| assert(!fAllBoxes); // TODO | |||
There was a problem hiding this comment.
See here; this assertion fails for &mfs -b
We have a good relationship with Alan. I think it's worth opening a PR if you can, and I can signal boost it with him. |
Yosys calls
&mfsas part of the default abc9 flow but every time it does it's prepared for the worst, and is ready to recover when the command crashes (see YosysHQ/yosys#1974). In this PR we try to make it crash less, see the individual commits for details.