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

Combinational sensitivity refactor and SSA #344

Merged
merged 32 commits into from
Apr 20, 2023
Merged

Conversation

mkorbel1
Copy link
Contributor

@mkorbel1 mkorbel1 commented Apr 19, 2023

Description & Motivation

This PR is a significant refactor of Combinational to make it safer to use.

  • Removes implementation of deep sensitivity list calculations from Combinational. This also removes FullyCombinational stuff. This exposes previous unit-test failures which were a result of "write after read" implementations.
  • Added strict checking to Combinational against "write after read" behavior.
  • Adds Combinational.ssa which uses "static single-assignment form" (SSA) to "rename" signals to automatically avoid "write after read" scenarios.
  • Refactored Pipeline abstractions to use Combinational.ssa since they encourage and leverage "write after read" coding styles for flexibility.
  • Extensive testing for the new functionality.
  • Added benchmarking code based on bug reports of slow performance which was a result of the old combinational sensitivity list calculations.
  • Adds and updates a variety of ROHD-specific exceptions to make debug and error handling easier.
  • Adds some nice updates to SimCompare: checkIverilogVector so you don't have to compare the bool afterwards, and a mask for known warnings to remove known print messages while doing verilog simulations in the test suite.
  • Provides a cancellable subscription object (SynchronousSubscription) whenever a glitch is listened to. This mirrors capabilities of Streams.
  • Optimized drivers and receivers in conditional logic to use lazy evaluation to avoid recalculation during simulation, which should provide some boost to simulation performance.
  • Merged If and IfBlock, with new If.block API.

Related Issue(s)

Fix #312
Fix #290
Fix #286
Fix #285

Testing

  • Existing tests were refactored or split so that they passed, expected failure, and/or had two versions (one of each).
  • Added many new tests for various scenarios of Combinational.ssa.
  • Expanded on Pipeline tests to cover potential new scenarios.

Backwards-compatibility

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

Yes!

  • Combinational will now throw fatal exceptions in case of "write after read".
  • Logic functions incr, decr, mulAssign, and divAssign now require an SSA-like renaming function for Combinational use since they inherently cause "write after read" behavior without one. The API has changed to use named parameters to support using this for either Combinational.ssa or Sequential.
  • FullyCombinational was removed entirely, along with Module.combinationalPaths and Module.reverseCombinationalPaths.
  • Deprecated APIs for getReceivers, getDrivers, and getConditionals in _Always blocks Combinational and Sequential.
  • Deprecated IfBlock in favor of If.block.

Documentation

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

Documentation was upated in API docs. No new separate README or other docs were written about Combinational.ssa since it's generally a more advanced topic. Eventually a small article would be nice on the topic.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
1 participant