In the ABC pass, avoid scanning the entire module for each ABC run

[rocallahan]

What are the reasons/motivation for this change?

We have some large modules where a single ABC pass runs ABC thousands of times. The pass is very slow because in a couple of places it scans the entire module each time it runs ABC. Without too much work we can avoid this and get significant speedups. For example on one of our real-world examples this PR speeds up the ABC pass by 6x.

Explain how this is achieved.

One problem is rebuilding assign_map from scratch for every ABC call. Instead we can build it once and maintain it incrementally as the module is modified.

Another problem is scanning all module wires and cells to call mark_port(). This is a bit trickier; we need to introduce a reverse map from canonical SigBits to the module wires and cells associated with each bit, and maintain that as the module is modified.

Another problem is that constructing FfInitVals requires iterating over all module wires, and we were constructing one of these in every ABC run. We don't really need to, because the wires don't meaningfully change during this pass. So just construct a single FfInitVals for the module and use it for all ABC runs.

The use of global variables and the very large abc_module() function make things a bit less clear than they need to be, so I started off the PR by cleaning that up a bit.

If applicable, please suggest to reviewers how they can test the change.

This PR is intended to not change results at all, and in my limited testing it doesn't. I have checked that some of our very large circuits are not affected by this PR.

[rocallahan]

I'm considering doing this in a different way.

[rocallahan]

This is a bit trickier; we need to introduce a reverse map from canonical SigBits to the module wires and cells associated with each bit, and maintain that as the module is modified.

So I ended up doing this part in a different way. I introduce a SigValMap type which associated some data (in this case "is it a port") with the canonical SigBits of a SigMap, and make assign_map be a SigValMap. Then once we've partitioned the cells by clock domain for the ABC calls, we set that is_port flag for all signal bits that are connected to more than one partition (including signal bits connected to cells not in any partition). We also set the flag for all bits of wires that are module ports.

Then, when we need to determine which signals are ports for a specific cell partition, we can basically just lookup that flag. We do also need to take into account that some cells in a partition may be rejected and not passed to ABC (I call them the "kept cells") --- connections to those cells need to be treated as ports as well.

(I made this change because I have another PR in the works which runs the ABC runs in parallel. For that to work, we have to be able to extract_cells for multiple clock domains before getting the ABC results for them, which means we need to be able to mark ports for a clock domain in a way that still works when the cells for some other clock domain(s) are missing.)

[phsauter]

Just out of curiosity, do you mean you call Yosys' abc pass multiple times in close proximity to each other, meaning you want to reuse as much of the partitioning as possible from previous passes (while checking if the partitioning is valid and if it is not redo those parts?) or is the problem that one abc pass in Yosys goes through the entire module multiple times and you are speeding it up inside abc.

I think it is the former but then my questions are:

• Can you somehow make sure this produces the same results as if there was no 'cashing' of the partitioning problem? This is not necessary for functionality or correctness but would eliminate any weirdness if for some reason this doesn't apply properly for all cases or people write checkpointed scripts (ie they save the design and then perform different operations on different versions of the saved design, ORFS does this for example).
• How exactly do you keep track of the changes? Is the idea that most of the time the clock domains stay the same and you can check if you currently have a valid partitioning. If you encounter an invalid one you then solve the problem again for that clock domain. So you just check for correctness of the previous solution on the new design and update if it is false?

[rocallahan]

is the problem that one abc pass in Yosys goes through the entire module multiple times and you are speeding it up inside abc.

This.

[rocallahan]

I discovered another "scan the whole module per ABC run", constructing FfInitVals, so I've fixed that here too.

[QuantamHD]

@widlarizer This one provides quite a large speedup on some of our workloads could we prioritize getting this reviewed and merged?

[widlarizer]

The assign_map change looks good (assign_map needs to be updated when and only when adding module connections)

the wires don't meaningfully change

Could you be more specific?

[widlarizer]

Also, the refactor is very welcome, thanks

[rocallahan]

Could you be more specific?

FfInitVals only cares about wires with the ID::init attribute, and AbcPass only adds wires, and the wires it adds don't have the ID::init attribute. I'll add a comment explaining this.

[widlarizer]

constructing FfInitVals requires iterating over all module wires, and we were constructing one of these in every ABC run

Looking at main, I don't think this is the case. This happens per module and no other initvals.set happens:

assign_map.set(mod);
initvals.set(&assign_map, mod);

As far as I can tell, the unnecessary initvals rebuild per abc_module call snuck in while refactoring, which is fine, it just threw me for a bit of a loop reviewing this

[rocallahan]

As far as I can tell, the unnecessary initvals rebuild per abc_module call snuck in while refactoring, which is fine, it just threw me for a bit of a loop reviewing this

Oops, sorry. Thanks for clarifying that.