Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix combinational sensitivity excessive pessimism, fix #233 #240

Merged
merged 3 commits into from
Jan 5, 2023

Conversation

mkorbel1
Copy link
Contributor

Description & Motivation

Issue #233 finds that searches for Combinational sensitivity lists, it is excessively pessimistic in that sometimes there aren't really any combinational paths, but it assumes that there may be. This can cause an issue where a false combinational loop is detected, causing incorrect X-generation.

This PR fixes the bug by calculating more detailed combinational paths for Modules at build time. This information can be leveraged by Combinational to understand more accurately where combinational paths may exist. Results are cached to avoid repeated calculations when multiple Combinationals search through similar paths of potential sensitivities. All Modules with custom functionality must explicitly define any combinational paths from all inputs to applicable outputs or else Combinational will be unable to properly determine an accurate sensitivity list. A new FullyCombinational mixin is included in this PR to automatically add all input/output combinations as potential combinational paths, since that's a common case for many small custom gates.

Additionally,

  • Added RohdException base type for all ROHD exceptions, making it easier to handle ROHD-related exceptions
  • Added new ModuleNotBuiltException, used in multiple places in Module

Related Issue(s)

Fix #233

Testing

  • Added a new test that had reproduced the original issue without this fix.
  • Existing tests cover scenarios where non-trivial sensitivities exist.

Backwards-compatibility

Is this a breaking change that will not be backwards-compatible? If yes, how so?

Other custom functionality modules defined outside of the ROHD framework which had previously benefited from assumption of the behavior of FullyCombinational will now be treated as the complete opposite (no combinational paths). This could mean Combinationals would now be missing sensitivities that previously existed. If those sensitivities were true, then existing simulations in ROHD might now have incorrect behavior. This is likely rare, and since this is really a bug fix it doesn't quite feel like it's breaking backwards-compatibility.

Documentation

Does the change require any updates to documentation? If so, where? Are they included?

Doc comments updated/included

lib/src/module.dart Outdated Show resolved Hide resolved
lib/src/module.dart Show resolved Hide resolved
@mkorbel1 mkorbel1 merged commit 27ee267 into intel:main Jan 5, 2023
@mkorbel1 mkorbel1 deleted the combpaths branch January 5, 2023 22:40
quekyj pushed a commit to quekyj/rohd that referenced this pull request Jan 9, 2023
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.

Combinational sensitivity detection is excessively pessimistic
1 participant