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

[Verif] Make verif.formal parameters an ODS property #7867

Merged
merged 1 commit into from
Nov 21, 2024

Conversation

fabianschuiki
Copy link
Contributor

Instead of using the discardable attributes dictionary of verif.formal as vessel for user-defined test parameters, add a dedicated parameters field to the op itself. This makes the parameters non-discardable and a proper part of the test.

Instead of using the discardable attributes dictionary of `verif.formal`
as vessel for user-defined test parameters, add a dedicated `parameters`
field to the op itself. This makes the parameters non-discardable and
a proper part of the test.
Copy link
Member

@maerhart maerhart left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Slightly more documentation would be nice 😁

@@ -295,7 +295,7 @@ def FormalOp : VerifOp<"formal", [

### Example
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A short paragraph in the documentation describing what parameters mean for frontends and for backends would be great. E.g., what kind of parameters can a frontend add and what can it expect/what guarantees does it have? What do backends need to do with the parameters? Can they just ignore them at will?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

😁 Yeah we'll definitely need more details in the documentation. The problem right now is that we don't know yet which parameters are relevant or used. I expect us to figure this out as we build out the runners and the circt-test tool. For example, it looks like a "mode" and "bound" parameter makes sense for formal tests. But maybe also not really. My reflex here was to leave this out until we actually know which parameters are guaranteed to be understood by the runners, instead of promising something up front that then doesn't happen. 😬

@fabianschuiki fabianschuiki merged commit 454923f into main Nov 21, 2024
4 checks passed
@fabianschuiki fabianschuiki deleted the fschuiki/verif-formal-ods-params branch November 21, 2024 19:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants