&cec -x/-y: is the AND-free check meant to be sufficient for equivalence? - #543
Open
marcelwa wants to merge 1 commit into
Open
&cec -x/-y: is the AND-free check meant to be sufficient for equivalence?#543marcelwa wants to merge 1 commit into
marcelwa wants to merge 1 commit into
Conversation
The equivalence verdict in these three branches is taken from Gia_ManAndNum(pNew) == 0 after Cec4_/Cec5_ManSimulateTest3. An AND-free GIA can still have outputs that are constant 1 or CI literals, which are satisfiable, so this reports "Networks are equivalent" for some non-equivalent pairs -- for example `a & b` against `~(a & b)`, where the miter sweeps to constant 1. Check the outputs as well: AND nodes remaining -> UNDECIDED as before; AND-free with all outputs constant 0 -> equivalent as before; AND-free otherwise -> NOT equivalent, which is decidable by inspection since such a miter is satisfiable.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Apologies in advance if I have misread this — I would rather ask than assume.
While auditing my own results I noticed that
&cec -yreported a pair of networks as equivalent that I was fairly sure were not, and following it back led me to the verdict logic inAbc_CommandAbc9Cec. I may well be missing an invariant that makes the current form correct, in which case please just close this and, if you have a moment, I would be grateful to know what I overlooked.What I think I am seeing
In the
-xand-ybranches of&cec(and the corresponding branch of&icec), the verdict is taken from the swept miter having no AND nodes:My reading is that
Gia_ManAndNum(pNew) == 0is necessary but not sufficient: an AND-free GIA can still have an output that is constant 1, or a CI literal, and both of those are satisfiable miters. If that is right, the check reports equivalence for some non-equivalent pairs, and the branch has noNOT_EQUIVALENToutcome at all.Smallest case I could reduce it to
Two 2-input AIGs,
f = a & bandg = ~(a & b)— they differ on every input:The default engine gets it right, which is why this went unnoticed on my side for a while — it only surfaced because one of my checks happened to pin
-y. On a real 36 500-LUT netlist I also saw-yaccept a mutant whose first output is the exact complement of the reference's, at 302 s, wherececrefutes it in 1.2 s.The change proposed here
If the reading above is correct, the smallest fix I could find is to check the outputs as well as the AND count:
UNDECIDED, exactly as before;NOT equivalent, which seems safe to decide, since such a miter is satisfiable by inspection.Gia_ManPoIsConst0()already exists, so the helper is four lines. Applied at all three sites, since the same idiom appears in&cec -x,&cec -yand&icec.Behaviour on the small cases above, built from this branch:
and2vsand2and2vsnand2and2vsor2The default
&cecandcecpaths go throughCec_ManVerifyand are untouched.Caveats I am aware of
I have only exercised this on small hand-built cases plus my own benchmark netlists, not against your regression suite, so please treat the change as a suggestion rather than a validated patch. I am also not certain whether the
-x/-ybranches are intended as full equivalence checks or as fast pre-filters where a caller is expected to confirm a positive result some other way — if it is the latter, then this is a documentation question rather than a bug, and I would be happy to see it closed on those grounds.Thanks very much for ABC, and for taking a look.