Same as eb32507cb, for two `t:*` counts it missed. They only differ once
a design can still be in signorm mode when the assertion runs; the
subtraction is a no-op otherwise.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MQU5XCqDYY8MbivGkNRrDo
Every cell needs an AIG model or the pass errors out, and the signorm
marker cells are bookkeeping with no timing behaviour and no model:
`Unsupported cell '$input_port' in module 'top2'` (various/timeest).
Joining the ~20 passes that already opt out is cheaper than teaching the
estimator which cell types are not logic.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MQU5XCqDYY8MbivGkNRrDo
Two wires whose init bits map to the same representative are legal --
they are connected -- and each extra alias makes it likelier. Today the
first one recorded wins and any later disagreement is a hard error, even
when one of the two is 1'x, which says nothing at all
(arch/ice40/bug1644: `\DAC1_i2s.left_1 [0] = 1'0 != 1'x`).
An x now loses to a defined value and never reports a conflict; two
defined values that disagree still do.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MQU5XCqDYY8MbivGkNRrDo
A port's direction is read off the cell type, so assigning
type_impl = $lut while the $__ICE40_CARRY_WRAPPER ports are still
connected makes every following unsetPort consult the wrong type: \B was
an input under the wrapper and is not a port of $lut at all, so the
teardown takes the output branch and trips
`log_assert(w->driverCell_ == this)` in signorm mode (arch/ice40/counter,
arch/ice40/bug1598).
Disconnect everything under the old type, switch, then connect \A and \Y
under the new one. Retiring \O first also leaves that wire undriven, so
it comes back as \Y on the same wire rather than behind a freshly
interposed alias.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MQU5XCqDYY8MbivGkNRrDo
The worklist already followed $connect in both directions -- they are the
one place liveness flows backwards through a cell input -- but the
port-output/keep roots only dereferenced driverCell_. A module output
whose net reaches its driver *through* a $connect therefore marked
nothing at all, and the sweep took the whole module: arch/intel_alm/adffs
and arch/intel_alm/counter end with zero cells and `\q is used but has no
driver`.
Both paths now go through one mark_bit, so a root is followed exactly the
way a live cell's input is.
Unreachable until now only because opt_dff denormalized immediately
before opt_clean in the opt loop, so the signorm path was never taken
there.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MQU5XCqDYY8MbivGkNRrDo
BROKEN: 12 tests fail (list below). Committed to keep the work resumable,
not because it is ready.
opt_clean stops calling sigNormalize(false) and, for modules carrying an
index, runs a tracing GC against it instead of rebuilding its own view of
the netlist: mark from keep/output/$input_port roots through
wire->driverCell_ (one dereference per input bit, no wire2driver
hashtable, no assign_map), sweep cells, sweep wires on fanout emptiness,
then compact the sigmap over the survivors before the wires are freed.
Per the design decision, it does not rename or rewire: where the old pass
collapsed a public wire and its driver's wire into one, this keeps both
plus the alias in the sigmap, which is fine to traverse.
Kernel support: SigNormIndex::compact() (mfp cannot erase, so the sigmap
only ever grows -- opt_clean is the only pass that knows what is garbage,
hence the only one that can shrink it), an index-aware
Module::remove(pool<Wire*>), and signorm_sigmap() normalizing fully
rather than only flushing, since a caller reading the *index* rather than
just the map has to see it settled.
What this is worth on its own: 4.75s -> 4.54s on a 2000-stage pipeline
(Debug), against main-dbg's 4.56s. The real win needs opt_dff to stop
tearing the index down too -- together they are 3.39s -- but opt_dff
regresses optimization quality under signorm and is left denormalizing.
Neither change pays off without the other, which is the point of keeping
this around. See g/signorm-opt_clean-analysis.md.
Failing: arch/fabulous/fsm, arch/gatemate/fsm,
arch/quicklogic/qlf_k6n10f/t_mem{1,5,6},
asicworld/code_verilog_tutorial_explicit, memories/wide_all, sat/share,
various/{bug3515,bug4909,muxcover,port_sign_extend}.
port_sign_extend is diagnosed and is not this pass's fault: Design::add()
indexes a module while it is still empty, so setup_module_inputs() finds
no ports and the index stays incomplete for wires added afterwards. Same
defect as submod. muxcover is another marker-cell count. sat/share is the
one still worth suspecting the GC over.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MQU5XCqDYY8MbivGkNRrDo
$input_port, $output_port and $public are signorm bookkeeping rather than
mapped logic, so assertions counting "every cell" or "every $ cell" have
to subtract them -- as tests/opt/opt_expr_*.ys already do with
`t:$*_port %d t:$public %d`.
The memlib assertions are generated, so the fix goes in generate_mk.py
rather than the generated scripts.
These only differ once a design can still be in signorm mode when the
assertion runs; the subtraction is a no-op otherwise.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MQU5XCqDYY8MbivGkNRrDo
Four passes that mutate designs in ways the signorm index does not
survive, joining the ~20 that already opt out:
- fsm_extract rewires control outputs by writing cell connections_
directly, leaving the old wire still recorded as driven by the port it
left and the new one recorded as driven by nothing.
- autoname renames cells, and PortBit hashes through cell->name, so every
fanout entry of a renamed cell becomes unfindable. This is structural,
not incidental -- abc9_ops notes the same hazard for swap_names, and
rename already opts out for it.
- blackbox razes modules: makeblackbox() drops every cell and every
non-port wire regardless of what the index believes drives what.
- submod creates modules and populates them afterwards, so their ports
appear after setup and never get the $input_port cells signorm expects.
All four corrupt the index today; `check` reports it. Harmless only for
as long as the index is guaranteed to be thrown away before anything
consults it again.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MQU5XCqDYY8MbivGkNRrDo
The library file is the only description of these cell types, so unless
the user has separately read in a model, memory_libmap left the design
instantiating a module that does not exist. `hierarchy -check` rejects
that, and Cell::port_dir() has nothing to answer with, which matters
beyond tidiness: setPort treats a port of unknown direction as an output,
so on a normalized design every input port gets a fresh driven wire plus
a $connect cell tying it back to its real source. Those reach the backend
-- write_verilog emits them as `tran` -- and simulate as x.
Declare each port on a generated blackbox before connecting it, following
memory_bram. Directions come from the cell interface in memlib.md, where
PORT_<name>_RD_DATA is the sole output; widths come from the signal being
connected, so they match the chosen configuration. A model the user
supplied is left alone rather than reshaped.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MQU5XCqDYY8MbivGkNRrDo
design->flagSigNormalized says the design is in signorm mode, not that a
given module has an index: one created after the mode was entered (submod
splitting out a new module, say) has none. check_bufnorm_cell and
check_signorm_fanout went to the index on the strength of the design flag
alone and tripped Module::fanout()'s assertion.
Unreachable until now only because opt_clean tore the index down before
`check` ever ran on such a module.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MQU5XCqDYY8MbivGkNRrDo
load_blackbox() created the module and its port wires with
twines.add(Twine{...}), which interns a private twine, while
load_blackboxes() checked for an existing module with
twines.add(std::string{...}), which interns a public one. The two never
matched, so the generated modules were unreachable under the name the
emitted cells use for their type: write_rtlil showed the module as
$priv@... against a $pub@... cell type.
The blackboxes were therefore inert since 25e7118e4 introduced them.
Cell::port_dir() kept falling back to PD_UNKNOWN for every BRAM cell,
which is the thing that commit set out to fix, and `hierarchy -check`
still had no module to find.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MQU5XCqDYY8MbivGkNRrDo
While in signorm mode the module's index already holds a complete SigMap
of its alias connectivity; connections_ is only a scratch buffer that
restore_connections() materializes that map into on demand. So every
`SigMap sigmap(module)` was a round trip: restore_connections() walked
the entire union-find emitting (bit, sigmap(bit)) pairs, and SigMap::set()
hashed all of them straight back into an identical union-find. Worse,
set() iterates connections() twice, so each construction paid for two
full materializations.
Since restore_connections() emits the representative as the right-hand
side and SigMap::add() leaves the right-hand side as the root, the
rebuild reproduces exactly the classes and representatives it started
from -- so copy the mfp instead. Module::signorm_sigmap() hands it out
after merging any pending connections, and returns nullptr outside
signorm mode, where the old path stands.
The copy also carries entries for bits that map to themselves, which the
connection-derived rebuild would omit. That is only observable through
SigMap::allbits() or by iterating .database, neither of which has a
caller outside sigtools.h.
Measured on a 2000-stage pipeline with dead logic (Debug build, `opt`):
signorm_restore drops from 0.76s over 22 calls to 0.08s over 12, and the
whole run from 7.24s to 6.40s. The sigmap being round-tripped there peaks
at 1.29M entries, which is the alias structure of the post-proc netlist.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MQU5XCqDYY8MbivGkNRrDo
run_constbits() built a ModWalker (a full driver/consumer index over every
cell port of the module) and a QuickConeSat unconditionally, but both are
only ever reached through prove_const_with_sat(), which is gated on
opt.sat. Without -sat the whole index was built and thrown away.
run_eqbits() already had the `if (!opt.sat) return false;` guard ahead of
its own ModWalker; this gives run_constbits the same treatment by making
both objects optional.
Measured on a 2000-stage pipeline with dead logic (Debug build, `opt`):
opt_dff drops from 2.08s to 0.45s, whole-run peak RSS from 640MB to 548MB.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MQU5XCqDYY8MbivGkNRrDo
Route the (2^k-1)-x -> ~x rewrite through OptExprPatcher like its
sibling transforms in this file, instead of raw module->Not +
module->connect + module->remove. Loosen the postopt assertion to
tolerate the $input_port shim cell bufnorm adds for the (still
signorm-active) input port, matching the t:$*_port %d idiom already
used by opt_expr_alu.ys and other sibling tests.
CC_LUT2 1 (was 2), CC_L2T4 max 15 (was 14) reflect the cell mapping
produced after the proc src-location changes.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
derived_name and the template's marker wire are interned in map->twines;
decoding them with design->twines read an out-of-range/foreign pool slot
(garbage, or a crash once pool sizes diverge) in the _TECHMAP_FAIL_ log.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
add(std::string) tags bare strings as public names, so a single-source
concat (which returns the lone ref unchanged) decoded with a spurious
'\' prefix, splitting one source file into two in linecoverage output.
add(Twine{}) stores the location string verbatim like set_src_attr.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
kernel/compressor_tree.cc, passes/tests/test_kogge_stone.cc,
passes/opt/peepopt_shiftpow2.pmg, and techlibs/lattice/lattice_dsp_nexus.pmg
(plus the 5 new constids it needs: A0, B0, PIPELINED, SIGNED, SUBTRACT)
were all added to main after the emil/twines-proc-srcloc fork point, so no
commit in this phase's range ever touches them. They still use the old
IdString-based addWire/addCell/setPort/getParam APIs (NEW_ID, plain \X
port names) that the twine migration replaces, so they fail to build
against current HEAD. Not attributable to any single commit in this
phase per task.md's carve-out for genuinely phase-spanning fixes -
bundled here as its own commit rather than folded into an unrelated
pick.
