formal: add SymbiYosys proof for cc_ecc_encode/cc_ecc_decode - #355
Open
repowazdogz-droid wants to merge 1 commit into
Open
formal: add SymbiYosys proof for cc_ecc_encode/cc_ecc_decode#355repowazdogz-droid wants to merge 1 commit into
repowazdogz-droid wants to merge 1 commit into
Conversation
Prove the four cases documented in the cc_ecc_decode header for the encoder/decoder pair: clean round-trip, single-bit correction inside the Hamming codeword, a flipped extended parity bit, and double error detection. One encoder feeds four decoders so the cases cannot interfere. DataWidth 1, 2, 4, 5, 11 and 12 are proved by default, bracketing each parity-width transition on both sides. Wider parametrizations up to the module default of 64 run as a separate sweep task, not part of `make all`. The properties claim nothing about data_o under a double error, nothing beyond two flipped bits, and nothing about interoperability with another implementation of the same code. The parity width is checked against a count of the power-of-two codeword positions rather than by recomputing the cc_pkg search, so a defect there is not reproduced by the check. `make ecc.check` runs a cover task alongside the proof, so a proof resting on an unsatisfiable assumption cannot pass unnoticed.
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.
Adds a SymbiYosys proof for the
cc_ecc_encode/cc_ecc_decodepair, as a subtask of #288.Following your note on #288, this does not wait on #354. I used #354 only as a rough reference and kept the layout to what current
masteralready uses forlzcandheaviside, so it should rebase cleanly onto whatever #354 settles on.cc_ecc_decodesuits formal work because its header already carries the specification. The syndrome and parity truth table gives four cases over a combinational pair with no protocol and no state, so the properties are a transcription of documented behaviour rather than a new model.What is proved
One encoder feeds four decoders, each seeing a different corruption of the encoded word, so no case can weaken another.
data_o == data_i, all three flags lowdata_o == data_i,single_error_ohigh, other flags lowdata_o == data_i,parity_error_ohigh, other flags lowdouble_error_ohigh, other flags lowP2 excludes the extended parity bit deliberately, since a flip there is reported through
parity_error_oand stating P2 over the whole encoded word would contradict the truth table.What is not claimed
data_ounder a double error. The decoder does not promise a value there, and the syndrome can address a position outside the codeword. That decoder instance has itsdata_oleft unconnected so no property can read it.One check is not matched-pair.
cc_ecc_properties.svcounts the power-of-two positions in the codeword and compares that count againstcc_pkg::ecc_get_parity_width, rather than recomputing the same search, which would agree with the package for the same wrong reason. Both operands are elaboration-time constants, so it is an elaboration check and not a proof obligation, and the comment says so.Widths and engine
make ecc.checkprovesDataWidth1, 2, 4, 5, 11 and 12.cc_pkg::ecc_get_parity_widthadds a parity bit going from 1 to 2, from 4 to 5 and from 11 to 12, so each transition is proved on both sides. 1, 4 and 11 are also the widths whose encoded word is exactly a power of two bits wide, where an index constraint is easiest to get wrong.Engine is
abc pdrrather than thesmtbmcthe other proofs use. The pair is purely combinational, so depth 1 is already exhaustive, and PDR is chosen only for runtime: on the default width set the proof takes about 10 s with PDR against about 3 min with smtbmc. I did not run smtbmc to completion on the full sweep, so I am not putting a number on that.Wider parametrizations up to the module default of 64 are a separate task, not part of
make all. It proved all 128 properties in 46 min 47 s here:Vacuity
make ecc.checkruns acovertask alongside the proof. The properties constrain where the injected errors may land, so the cover task is what shows those positions are reachable;abc pdrreports no PREUNSAT, so a proof resting on an unsatisfiable assumption would otherwise pass silently.That is not hypothetical. An earlier version of this harness shared the error-position inputs across widths, which let one width's constraint narrow another's reachable positions while the proof stayed green. The cover task is what caught it, which is why it runs by default rather than on request.