setup_driven_wires() gave a zero-width port connection -- an output left
unattached, `.q_bar()` -- a helper wire and recorded the cell as its driver.
Nothing ever retracts that: unsetPort skips zero-width connections, as does
signorm_index_add when adding them. So removing the cell left the helper wire
pointing at freed memory, and the next `check` dereferenced it.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MQU5XCqDYY8MbivGkNRrDo
addCell(name, other) assigned connections_ wholesale, so a clone made while
the module carries an index was invisible to it: its inputs were absent from
the fanout and its outputs claimed no driver. The first setPort on the clone
then tried to retract entries that had never been made and tripped the
`erased' assertion. Go through setPort instead, which also interposes a wire
wherever the original is still driving the net.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MQU5XCqDYY8MbivGkNRrDo
Until `hierarchy` runs, a connection made by position is named `$<port_id>`
rather than after the port it reaches, so looking it up by name in the
instantiated module finds nothing and port_dir() reports PD_UNKNOWN. The
signorm index reads that as "not an input", which makes the cell claim a
driver on a net it only reads.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MQU5XCqDYY8MbivGkNRrDo
Leaving signorm dropped the $input_port markers but left the $connect
cells standing. A $connect is not decoration -- it is how the index
spells "these two already-driven nets are the same net" -- so once the
index is gone it is an opaque two-input cell, and the net that reached
its driver through it now has no driver at all. Outside the index that
fact is a module connection, so emit one.
Found via techmap, which is where it does real damage: templates are
copied cell by cell, so a $connect surviving in a map module reappears in
every module the template is instantiated into. `techmap.v`'s `_80_mul`
picks up four of them (plus their setup_driven_wires helper wires) while
the map design is briefly normalized by a _TECHMAP_DO_ command, and
techmap then splices them into the parent. opt_expr -undriven finds the
helper wires driverless, ties them to x, and the multiplier cone
evaporates: 103 cells -> 2 on
read_verilog <<EOT
module top(input signed [24:0] A, input signed [17:0] B, output [47:0] P);
assign P = A * B;
endmodule
EOT
synth_analogdevices
techmap -autoproc -wb -map +/analogdevices/cells_sim.v
opt_expr -undriven
That is arch/{analogdevices/dsp_abc9,intel_alm/mul,nexus/mul,
nexus/fuse_mac} -- every failure that keeping the index across opt_dff
introduced -- and sat/share, which predates it. `make -C tests` goes from
12 failures to 7; the benchmark is unchanged at 3.14s.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MQU5XCqDYY8MbivGkNRrDo
This is the other half of the signorm-native opt_clean: the two are
strictly complementary, since whichever of them still calls
sigNormalize(false) destroys the index and opt_expr rebuilds it on the
next iteration of the opt loop. Together they take `opt` on a 2000-stage
pipeline from 4.54s to 3.14s (Debug) against main-dbg's 4.28s, with peak
RSS 540MB against main's 683MB.
The obstacle was never bit2mux/bitusers, as the TODO removed here
guessed -- that guard passes in both modes, and it is not even consulted
in one pass: try_merge_srst absorbs the reset mux, emits an $sdff, pushes
it back onto dff_cells, and the enable is found on the next iteration.
What breaks is the SigMap the worker snapshots in its constructor.
FfData::emit() builds the replacement cell before the caller removes the
old one, so at setPort(Q, \q) the wire still has a driver and the kernel
does what I1 requires: interpose a fresh wire and alias \q to it. (Same
unconditionally for a sliced Q -- a port may never hold a partial wire.)
The snapshot does not relate the two, so try_merge_ce compares
a = sigmap(\q) against ff.sig_q and sees no feedback, yielding $sdff
where $sdffe was expected (tests/sim/sim_sdffe.ys).
So stop snapshotting. In signorm mode the module already maintains this
map and keeps it live across our edits; borrow it and settle the index
once per FF rewrite instead. FfDataSigMapped and BitSim take a
const SigMapView & so the borrow is read-only -- only the kernel may
write a module's own map.
Where resync() goes is the subtlety: only between whole FF rewrites.
Mid-rewrite, after try_merge_srst has emitted cells for some bit groups
but before the original cell is removed, a \q slice transiently has two
drivers, and flushing there would emit a multi-driver $connect instead of
merging the alias.
bitusers/bit2mux stay constructor snapshots. Keyed by canonical bits, so
a bit whose representative later moves becomes unreachable and its
lookups miss -- every guard reading them then fails closed, never open.
Test suite: 12 failures before, 12 after, four traded. Fixed:
arch/fabulous/fsm, arch/quicklogic/qlf_k6n10f/t_mem{5,6},
memories/wide_all. New: arch/{analogdevices/dsp_abc9,intel_alm/mul,
nexus/mul,nexus/fuse_mac}, all DSP/whitebox flows failing the same way --
techmap copies signorm helper wires out of a template that was in signorm
mode, and opt_expr -undriven then ties them to x. Diagnosed but not
fixed here; see g/signorm-opt_clean.md.
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
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
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
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
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.
