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

Disabling non-linear integer arithmetics #783

Merged
merged 8 commits into from
Dec 5, 2023
Merged

Conversation

Dspil
Copy link
Contributor

@Dspil Dspil commented Dec 1, 2023

Adds a flag in silicon to disable non-linear integer arithmetics when Z3 via STDIO is used.

@Dspil Dspil requested a review from jcp19 December 1, 2023 14:43
Copy link
Contributor

@jcp19 jcp19 left a comment

Choose a reason for hiding this comment

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

I have a few minor comments that we should address before merging. Could we also add a test that typically passes, but which should fail with the new flag to make sure that the implementation has the expected behaviour?

src/main/scala/Config.scala Outdated Show resolved Hide resolved
src/main/scala/Config.scala Outdated Show resolved Hide resolved
src/main/scala/decider/Z3ProverStdIO.scala Outdated Show resolved Hide resolved
Dspil and others added 4 commits December 1, 2023 22:27
Co-authored-by: João Pereira <joaopereira.19@gmail.com>
Co-authored-by: João Pereira <joaopereira.19@gmail.com>
Co-authored-by: João Pereira <joaopereira.19@gmail.com>
@Dspil Dspil requested a review from jcp19 December 1, 2023 21:40
Copy link
Contributor

@jcp19 jcp19 left a comment

Choose a reason for hiding this comment

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

LGTM. I added a comment to address if possible though

@marcoeilers
Copy link
Contributor

We already have a flag to pass options to Z3, right? Is this just tomake it more convenient?

@Dspil
Copy link
Contributor Author

Dspil commented Dec 2, 2023

Yes, it seems useful to have it as a separate option since we use it more and more (especially in scion)

@marcoeilers
Copy link
Contributor

Okay, makes sense. Would be nice if you could make it work for Z3 API as well.

Copy link
Contributor

@jcp19 jcp19 left a comment

Choose a reason for hiding this comment

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

lgtm

@Dspil Dspil merged commit cef593b into master Dec 5, 2023
4 checks passed
@Dspil Dspil deleted the dspil_disable_nlri branch December 5, 2023 10:51
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.

3 participants