Skip to content

♻️ Unify how register initialization is represented - #701

Open
marcelwa wants to merge 2 commits into
lsils:masterfrom
marcelwa:upstream-register-init
Open

♻️ Unify how register initialization is represented#701
marcelwa wants to merge 2 commits into
lsils:masterfrom
marcelwa:upstream-register-init

Conversation

@marcelwa

@marcelwa marcelwa commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Part of a three-PR chain, please review in this order:
#700 (AIGER reader fixes) → #701 (this PR)#699 (sequential write_aiger).
Once #700 lands this diff reduces to the register-initialization change alone.

The dependency on #700 is real rather than incidental: on master the ASCII reader
compares the wrong token, so an explicit latch reset of 1 is never recognised and the
mapping this PR introduces cannot be observed correctly.

Description

register_t::init carries three different encodings of "undefined" at the same time:

Surface "undefined" is
register_t default 3
blif_reader 2 (nondeterministic) / 3 (unspecified)
aiger_reader 255

The 255 is not a choice. The reader computes int8_t r = ... ? -1 : ... and assigns it
into a uint8_t field, so -1 narrows to 255.

This corrupts output

write_blif emits the initialization verbatim, and the BLIF .latch statement accepts
only 0, 1, 2 and 3. So a sequential AIGER file with an undefined latch reset, read
back and written as BLIF, produces:

.latch li0 new_n2   255

which no BLIF consumer accepts — ABC included. This is reproducible on master today.

Approach

Introduce register_init with the four documented values, following the BLIF .latch
field since it is the most expressive of the supported formats, and use it consistently
across aiger_reader, blif_reader and write_blif. AIGER has no counterpart for
unknown, so a latch without a defined reset maps to dont_care.

Two helpers come with it:

  • register_init::is_defined( value ) expresses the test callers actually want — whether
    a reset is 0 or 1 — so code stays correct if a format ever introduces further
    undefined states.
  • register_init::sanitize( value ) keeps write_blif from emitting a value the format
    cannot represent, whatever a caller may have stored.

write_aiger is updated in #699 rather than here, since the latch-writing code it touches
is introduced by that PR.

Compatibility

The only value that changes is the one AIGER produced for a nondeterministic latch,
2552. Comparisons of the form init > 1 or init != 0 && init != 1 are
unaffected; only code testing against 255 would notice, and that value was never
intentional API.

Verification

The AIGER file that previously produced 255 now yields .latch li0 new_n2 2, which
ABC reads back as Total latches = 1. Init0 = 0. Init1 = 0. InitDC = 1.

New test case covers explicit 0/1 and self-literal resets, asserting each stays within
the representable range and that is_defined agrees.

🤖 Generated with Claude Code

https://claude.ai/code/session_01GbUqcnZayPooFgekEXVSjz

A sequential network written by ABC could not be read back correctly. Three
separate defects, all in the AIGER latch and output handling.

**Omitted latch reset values were read as undefined.** Both the binary and the
ASCII reader defaulted to NONDETERMINISTIC when a latch line carried no reset
field. The format specifies the opposite: the original AIGER format initialized
every latch to zero, and the reset field added in 1.9 is optional, so its absence
means 0. ABC relies on this and omits the field for zero-initialized latches, so
every zero-initialized register came back undefined.

**The ASCII reader compared the wrong token.** The branch recognizing a reset
value of 1 tested `tokens[1u]`, the next-state literal, instead of `tokens[2u]`.
An explicit reset of 1 was therefore never recognized, and a latch whose
next-state literal happened to be 1 was wrongly reported as one-initialized.

**Bad state properties were dropped.** A writer emitting the AIGER 1.9 extended
header moves the primary outputs into the bad-state section; ABC does this for
any design with a non-zero latch initialization. `on_bad_state` was not
overridden, so those outputs disappeared and the network came back with none.
They are now kept as primary outputs, which is what they were, along with their
names from the symbol table.

Also fix an adjacent defect in the destructor: the output index only advanced for
outputs that carried a name, so with a sparse symbol table every name after the
first unnamed output landed on the wrong one.

Verified end to end against ABC. A network with three registers initialized to
0, 1, and undefined previously returned with no outputs and a corrupted first
register; it now round-trips through `resyn2` with its inputs, outputs,
registers, and reset values intact.

Note that AIGER constraints are still ignored. They are assumptions rather than
outputs, so mapping them onto primary outputs would change the meaning of the
network.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GbUqcnZayPooFgekEXVSjz
@codecov

codecov Bot commented Jul 28, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 88.88889% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 84.06%. Comparing base (25beb0e) to head (2b7d54a).

Files with missing lines Patch % Lines
include/mockturtle/io/aiger_reader.hpp 75.00% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #701      +/-   ##
==========================================
- Coverage   84.06%   84.06%   -0.01%     
==========================================
  Files         190      190              
  Lines       29468    29478      +10     
==========================================
+ Hits        24773    24781       +8     
- Misses       4695     4697       +2     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

`register_t::init` carried three different encodings of "undefined" at once.
`register_t` itself defaulted to 3, `blif_reader` used 2 for a nondeterministic
latch and 3 for an unspecified one, and `aiger_reader` produced 255 -- the
result of narrowing an `int8_t` of -1 into a `uint8_t` field rather than a
deliberate choice.

That last one corrupted output. `write_blif` emits the initialization verbatim
and the BLIF `.latch` statement accepts only 0, 1, 2, and 3, so a sequential
AIGER file with an undefined latch reset read back and written as BLIF produced

    .latch li0 new_n2   255

which no BLIF consumer accepts, ABC included.

Introduce `register_init` with the four documented values, following the BLIF
`.latch` field since it is the most expressive of the supported formats, and use
it consistently across the readers and writers. AIGER has no counterpart for
`unknown`, so a latch without a defined reset maps to `dont_care`.

`register_init::is_defined` expresses the test that callers actually want,
namely whether a reset value is 0 or 1, so code stays correct if a format ever
introduces further undefined states. `register_init::sanitize` keeps `write_blif`
from emitting a value the format cannot represent.

The only value that changes is the one AIGER produced for a nondeterministic
latch, from 255 to 2. Comparisons of the form `init > 1` are unaffected; only
code testing against 255 would notice, and that value was never intentional.

Verified end to end: the AIGER file above now yields `.latch li0 new_n2   2`,
which ABC reads back as one don't-care-initialized latch.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GbUqcnZayPooFgekEXVSjz
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant