Turns out this is not strictly necessary for this PR but it's
still a good thing to do and makes it clearer that the stats
are not modified in a possibly racy way.
We will want to query `keep_cache` from parallel threads. If we compute
the results on-demand, that means we need synchronization for cache
access in those queries, which adds complexity and overhead. Instead, prefill
the cache with the status of all relevant modules. Note that this doesn't
actually do more work --- we always consult `keep_cache` for all cells of
all selected modules, so scanning all those cells and determining the kept
status of all dependency modules is always required.
Later in this PR we're going to parallelize `scan_module` itself, and that's also
much easier to do when no other parallel threads are running.
Previously, setundef would rewrite sigspecs in all cells, processes,
and connections regardless of the active selection. Only modules and
memories were correctly filtered by selection.
Fix by using module->selected_cells() for cells, adding a
module->selected() check for processes, and checking wire selection
on the lhs of each connection before rewriting.
Fixes#5624
A real-world ABC9 flow hit residual combinational loops after SCC breaking, tripping the prep_xaiger loop assertion.
Keep the existing topological assertions in place (prep_xaiger and reintegrate still assert no_loops).
To handle residual non-box loops, add a targeted fallback in prep_xaiger: when loops remain after normal SCC breaking, insert additional $__ABC9_SCC_BREAKER cuts on non-box loop cells, rebuild toposort, and then re-check the existing assertion.
Also keep pre-ABC9 SCC tagging on all cell types (scc -all_cell_types) and add a regression test (tests/techmap/abc9-nonbox-loop-with-box.ys).
These updates should not be necessary. In fact, if they were necessary, this code
would be buggy, because the results would depend on the order in which wires are traversed:
If wire A is retained, which causes an update to `used_signals`, which then causes wire B
to be retained when it otherwise wouldn't be, then we would get different results depending
on whether A is visited before B.
These updates will also make it difficult to process these wires in parallel.
The negation here is confusing. The intent of the code is "if `s1` is preferred
over `s2` as the canonical `SigBit` for this signal, make `s1` the canonical `SigBit`
in `assign_map`", so write the code that way instead of "if `s2` is not preferred
over `s1` ...".
This doesn't change any behavior now that `compare_signals()` is a total order,
i.e. `s1` is preferred over `s2`, `s2` is preferred over `s1`, or `s1` and `s2` are equal.
Now, when `s1` and `s2` are equal, we don't call `assign_map.add(s1)`, but that's
already a noop in that case.
Currently when `s1` and `s2` are different bits of the same wire,
it is possible for both `compare_signals(s1, s2)` and `compare_signals(s2, s1)` to
return false. This means the calling code will call `assign_map.add()` for
both `s1` and `s2`, which doesn't make much sense --- one of `s1` or `s2`
should be consistently preferred.
So fix that by preferring the `SigBit` with the smaller bit offset.
Change from equality check to >= to allow cells where output
is wider than natural width (zero-extended). Only reject cells
with Y_WIDTH < natural width (truncated).
This fixes test failures while still preventing the truncation
issue identified in widlarizer's feedback.
Only allow rebalancing of cells with "natural" output widths (no truncation).
This prevents equivalence failures when moving operands between adders
with different intermediate truncation points.
For each operation type, the natural width is:
- Addition: max(A_WIDTH, B_WIDTH) + 1 (for carry bit)
- Multiplication: A_WIDTH + B_WIDTH
- Logic ops: max(A_WIDTH, B_WIDTH)
Fixes widlarizer's counterexample in YosysHQ/yosys#5605 where an 8-bit
intermediate wire was intentionally truncating adder results, and
rebalancing would change where that truncation occurred.
This pass converts cascaded chains of arithmetic and logic cells ($add,
$mul, $and, $or, $xor) into balanced binary trees to improve timing
performance in hardware synthesis.
The optimization uses a breadth-first search approach to identify chains
of compatible cells, then recursively constructs balanced trees that
reduce the critical path depth.
Features:
- Supports arithmetic cells: $add, $mul
- Supports logic cells: $and, $or, $xor
- Command-line options: -arith (arithmetic only), -logic (logic only)
- Preserves signed/unsigned semantics
- Comprehensive test suite with 30 test cases
Original implementation by Akash Levy <akash@silimate.com> for Silimate.
Upstreamed from https://github.com/Silimate/yosys
I am getting weird crashes on `main` in `tests/sdc/alu_sub.ys` which I traced to a null `Wire*`
in `SdcObjects::constrained_ports`. The null `Wire*` is being set in the `SdcObjects`
constructor. I don't understand what's going on here, so I added this check to detect the
missing wire early ... and that made the crash go away. Compiler bug maybe? I have
`Debian clang version 19.1.7 (3+build5)`, default build configuration.
Anyway this code seems fine to have.
I'm not sure why but this is actually faster than existing `opt_merge` even with
YOSYS_MAX_THREADS=1, for the jpeg synthesis test. 16.0s before, 15.5s after for
end-to-end synthesis.
This avoids changing `assign_map` and `initvals`, which are inputs to the hash function for `known_cells`,
while `known_cells` exists. Changing the hash function for a hashtable while it exists leads to
confusing behavior. That also means the exact behavior of `opt_merge` cannot be reproduced by a
parallel implementation.
When IdString refcounting was expensive, it made sense to pass it by const reference
instead of by value, to avoid refcount churn. Now that IdString is not refcounted,
it's slightly more efficient to pass it by value.
Some cells (e.g. $macc_v2) are marked evaluable, but will raise an abort if called with `CellTypes::eval()`.
Instead of falling through to the abort, we can pass a pointer to a boolean to check for errors.
Use said check to catch `CellTypes::eval()` errors and treat them as unevaluable but otherwise continue.
Reflows the series of if checks into `if ... else if ... else` so that we can check for errors and set state in one place.
Fixes these build errors I'm getting locally with `tcl-devel-9.0.0-7.fc42.x86_64`.
I guess Tcl 9 broke this.
```
passes/cmds/sdc/sdc.cc:438:6: error: no matching function for call to 'Tcl_ListObjLength'
438 | if (Tcl_ListObjLength(interp, listObj, &listLength) == TCL_OK) {
| ^~~~~~~~~~~~~~~~~
/usr/include/tclDecls.h:1788:13: note: candidate function not viable: no known conversion from 'int *' to 'Tcl_Size *' (aka 'long *') for 3rd argument
1788 | EXTERN int Tcl_ListObjLength(Tcl_Interp *interp,
| ^
1789 | Tcl_Obj *listPtr, Tcl_Size *lengthPtr);
| ~~~~~~~~~~~~~~~~~~~
passes/cmds/sdc/sdc.cc:446:8: error: no matching function for call to 'Tcl_ListObjLength'
446 | if (Tcl_ListObjLength(interp, subListObj, &subListLength) == TCL_OK) {
| ^~~~~~~~~~~~~~~~~
/usr/include/tclDecls.h:1788:13: note: candidate function not viable: no known conversion from 'int *' to 'Tcl_Size *' (aka 'long *') for 3rd argument
1788 | EXTERN int Tcl_ListObjLength(Tcl_Interp *interp,
| ^
1789 | Tcl_Obj *listPtr, Tcl_Size *lengthPtr);
| ~~~~~~~~~~~~~~~~~~~
```
abhinavputhran
Miodrag Milanovic
Robert O'Callahan
Emil J. Tywoniak
Emil J
nella
Tianji Liu
KrystalDelusion
Andrew Pullin
Gus Smith
nella
Natalia
Lofty
Martin Povišer
nataliakokoromyti
Roland Coeurjoly
N. Engelhardt
Robert O'Callahan
Mike Inouye