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

New unsafeSimSynchronizer, use in outputVerifier #1931

Merged
merged 1 commit into from
Sep 24, 2021

Conversation

DigitalBrains1
Copy link
Member

Warn when Clash.Explicit.Testbench.outputVerifier is used in HDL which
is not marked as being a test bench and is given different clock domains
as testDom and circuitDom. If testDom and circuitDom refer to
the same domain, the warning is not emitted.

To this purpose, a new primitive unsafeSimSynchronizer is added to
Clash.Explicit.Testbench, and it is used if a clock domain crossing is
needed in outputVerifier.

The code for outputVerifier is also cleaned up. It now declares it
needs a non-empty list of samples to verify, since it never worked with
an empty list. Common code is split off and shared rather than
duplicated, and it now uses Bounded and SaturatingNum to work with
the Index iterating the samples instead of writing out the same
functionality.

Rationale: outputVerifier does clock domain crossing in a manner that
is not suitable for actual hardware but is efficient for HDL simulation.
The code was not intended to be used outside of a test bench context,
but nonetheless it could be synthesized as part of hardware. If a
user does this, we should warn them that the resulting hardware is
improper.

The user can then add their own clock domain crossing in the circuit if
they would need this functionality. An imaginable use of
outputVerifier in a circuit is in combination with an ILA standing in
for the assert behavior.

Still TODO:

  • Write a changelog entry (see changelog/README.md)
  • Check copyright notices are up to date in edited files

@DigitalBrains1
Copy link
Member Author

I purposely did not write a Changelog entry for unsafeSimSynchronizer.

It is purely something I needed for implementing the desired behaviour in outputVerifier. But I didn't see a reason to keep it hidden, so I exposed it in the module. On the other hand, I found it so unsignificant I didn't want to enshrine it in the Changelog.

Warn when `Clash.Explicit.Testbench.outputVerifier` is used in HDL which
is not marked as being a test bench and is given different clock domains
as `testDom` and `circuitDom`. If `testDom` and `circuitDom` refer to
the same domain, the warning is not emitted.

To this purpose, a new primitive `unsafeSimSynchronizer` is added to
`Clash.Explicit.Testbench`, and it is used if a clock domain crossing is
needed in `outputVerifier`.

The code for `outputVerifier` is also cleaned up. It now declares it
needs a non-empty list of samples to verify, since it never worked with
an empty list. Common code is split off and shared rather than
duplicated, and it now uses `Bounded` and `SaturatingNum` to work with
the `Index` iterating the samples instead of writing out the same
functionality.

Documentation for `biTbClockGen` is adjusted after discussion about
unclear parenthetical in original doc.

Rationale: `outputVerifier` does clock domain crossing in a manner that
is not suitable for actual hardware but is efficient for HDL simulation.
The code was not intended to be used outside of a test bench context,
but nonetheless it could be synthesized as part of hardware. If a
user does this, we should warn them that the resulting hardware is
improper.

The user can then add their own clock domain crossing in the circuit if
they would need this functionality. An imaginable use of
`outputVerifier` in a circuit is in combination with an ILA standing in
for the `assert` behavior.
@DigitalBrains1
Copy link
Member Author

The PR now includes the adjusted documentation for biTbClockGen which we discussed on Slack on 1 Sep, the discussion which lead to the rest of this PR in the first place.

@DigitalBrains1 DigitalBrains1 merged commit b4ddd62 into master Sep 24, 2021
@DigitalBrains1 DigitalBrains1 deleted the warn_outputverifier branch September 24, 2021 14:52
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.

None yet

2 participants