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

verification: guard statements with module reset #1891

Merged
merged 1 commit into from
Apr 29, 2021

Conversation

ekiwi
Copy link
Contributor

@ekiwi ekiwi commented Apr 29, 2021

Contributor Checklist

  • x Did you add Scaladoc to every public function/method?
  • Did you add at least one test demonstrating the PR?
  • Did you delete any extraneous printlns/debugging code?
  • Did you specify the type of improvement?
  • Did you add appropriate documentation in docs/src?
  • Did you state the API impact?
  • Did you specify the code generation impact?
  • Did you request a desired merge strategy?
  • Did you add text to be included in the Release Notes for this change?

Type of Improvement

  • behavior change for consistency

API Impact

  • guards the new assert, assume and cover statements with the current default reset

Backend Code Generation Impact

  • this will change the Verilog to disable verification statements when reset is active

Desired Merge Strategy

  • squash

Release Notes

Assert, assume and cover statements are now disabled during reset by default. If you want your assertion to be active at all times, you can create a reset scope with reset=false.

Reviewer Checklist (only modified by reviewer)

  • Did you add the appropriate labels?
  • Did you mark the proper milestone (3.2.x, 3.3.x, 3.4.x, 3.5.0) ?
  • Did you review?
  • Did you check whether all relevant Contributor checkboxes have been checked?
  • Did you mark as Please Merge?

@ekiwi ekiwi requested a review from jackkoenig April 29, 2021 16:46
@ekiwi ekiwi force-pushed the verification-reset-guard branch from ef13ffe to 74a7959 Compare April 29, 2021 18:28
@ekiwi
Copy link
Contributor Author

ekiwi commented Apr 29, 2021

Do we treat this as a bug and backport to 3.4.x or do we treat this as a behavioral change (which it is) and only add it to the pre-release branch?

@ekiwi ekiwi added this to the 3.4.x milestone Apr 29, 2021
@ekiwi ekiwi added the Please Merge Accepted PRs that are ready to be merged. Useful when waiting on CI. label Apr 29, 2021
@jackkoenig
Copy link
Contributor

Discussed offline but this is a bug, so we're backporting

@mergify mergify bot merged commit 4d8fed0 into chipsalliance:master Apr 29, 2021
mergify bot pushed a commit that referenced this pull request Apr 29, 2021
@mergify mergify bot added the Backported This PR has been backported label Apr 29, 2021
mergify bot added a commit that referenced this pull request Apr 29, 2021
(cherry picked from commit 4d8fed0)

Co-authored-by: Kevin Laeufer <[email protected]>
jackkoenig added a commit that referenced this pull request Feb 28, 2023
These options are generally specific to a stage and thus should not be
propagating across serialization
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Backported This PR has been backported Please Merge Accepted PRs that are ready to be merged. Useful when waiting on CI.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants