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

Support negative edge-triggered Sequentials #483

Draft
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

mkorbel1
Copy link
Contributor

Description & Motivation

Adds support for negative edge-triggered Sequentials. In some cases, downstream tools may treat an inverted clock fed into a sequential differently than a negative edge triggered sequential. Functionally, in simulation, the behavior is the same for either option. Generators may choose to generate these differently (e.g. using negedge vs. posedge in SystemVerilog generation).

Additionally fixed a bug where a multi-triggered Sequential may not generate X's if one trigger is valid and another (later specified) trigger is invalid.

The changes also motivated some refactoring of trigger handling in Sequential, as well as renaming of the first positional argument in Sequential.multi.

Related Issue(s)

This is related to the discussion on #42, where previously it was decided not to include negedge due to lack of motivation. Downstream tool differences now motivate adding the feature.

Testing

Added new tests, existing tests pass.

Backwards-compatibility

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

No, but the signature of Sequential.multi has been slightly expanded and modified:

  • first positional argument changed name from clks to posedgeTriggers
  • added negedgeTriggers as a named optional argument

Documentation

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

This is a relatively uncommon feature, so documentation added onto the API docs should be sufficient (extra example in user guide not really needed).

@mkorbel1 mkorbel1 marked this pull request as draft June 4, 2024 18:19
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