Skip to content

Sound MoreCompleteExhale on demand#682

Merged
marcoeilers merged 16 commits into
masterfrom
meilers_mce_on_demand
Mar 10, 2023
Merged

Sound MoreCompleteExhale on demand#682
marcoeilers merged 16 commits into
masterfrom
meilers_mce_on_demand

Conversation

@marcoeilers

@marcoeilers marcoeilers commented Feb 13, 2023

Copy link
Copy Markdown
Contributor

This PR does two things:

First, it adds a mode where Silicon's MoreCompleteExhale mode is used on retry only. That is, Silicon works in its normal greedy mode, and whenever it cannot prove an assertion and retries the proof, MoreCompleteExhale is temporarily turned on (for this purpose, a moreCompleteExhale flag is introduced in the State). Then, the assertion is proved in MCE mode, and the branch continues verifying in greedy mode.
The idea is that MCE is often needed to prove a few assertions in large examples, but is not needed for the vast majority. Since MCE also comes with a performance penalty, we try to do as much as possible in the faster greedy mode, and only switch to MCE when needed.
To do this, the PR adds a new command line flag exhaleMode, which can be set to greedy (default), MCE, or MCE-on-demand. The existing enableMoreCompleteExhale flag is kept for compatibility reasons but deprecated.

Second, it fixes the unsoundness mentioned in issue #594 and #523. I.e., when checking function preconditions in MCE mode, Silicon used to ignore permission amounts and just prove that some permission for every mentioned field and predicate was available. For consistency (until we switch everything to ignoring permission amounts in function preconditions), we now consider permission amounts in these situations as well. The implementation simply uses the existing MCE functionality and, when wildcards are used, also uses the existing summarise function, which is already used elsewhere and does not have the incompleteness problems of the existing implementation.

@jcp19

jcp19 commented Feb 14, 2023

Copy link
Copy Markdown
Contributor

On the "on-demand" mode, does silicon still retry once without mce after a failure, but after performing heap consolidation? In other words, is this the sequence of steps that silicon performs on a failure?

  1. perform heap-consolidation and retry
  2. If failed again, enable mce and retry

Comment thread src/main/scala/Config.scala Outdated
Comment thread src/main/scala/rules/ExecutionFlowController.scala Outdated
marcoeilers and others added 2 commits February 16, 2023 16:27
Co-authored-by: João Pereira <joaopereira.19@gmail.com>
Co-authored-by: João Pereira <joaopereira.19@gmail.com>
@marcoeilers

Copy link
Copy Markdown
Contributor Author

On the "on-demand" mode, does silicon still retry once without mce after a failure, but after performing heap consolidation? In other words, is this the sequence of steps that silicon performs on a failure?

  1. perform heap-consolidation and retry
  2. If failed again, enable mce and retry

On retry, it immediately performs a state consolidation and switches on MCE in one step. I agree it might be worth trying to do only a state consolidation first, but with the current system of nested retries, I don't want to generally add a second retry. If we ever change the retry system in general to avoid the nesting (I still don't know if that's possible or worth it), then this might be a thing we could change.

@jcp19 jcp19 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@marcoeilers

Copy link
Copy Markdown
Contributor Author

I added a small change that fixes issue #205, which was previously fixed for standard (greedy) Silicon, also in MCE mode.
As a result, in the test suite, only one test actually fails with MCE and another one times out (see the update in issue #557).

@jcp19

jcp19 commented Feb 20, 2023

Copy link
Copy Markdown
Contributor

I added a small change that fixes issue #205, which was previously fixed for standard (greedy) Silicon, also in MCE mode.

As a result, in the test suite, only one test actually fails with MCE and another one times out (see the update in issue #557).

@marcoeilers do you expect this change to have major performance implications when running in mce mode?

@marcoeilers

Copy link
Copy Markdown
Contributor Author

I added a small change that fixes issue #205, which was previously fixed for standard (greedy) Silicon, also in MCE mode.
As a result, in the test suite, only one test actually fails with MCE and another one times out (see the update in issue #557).

@marcoeilers do you expect this change to have major performance implications when running in mce mode?

No. In greedy mode, the exact same fix made basically no difference at all.

@mschwerhoff mschwerhoff left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

Comment thread src/main/scala/Config.scala Outdated
Comment thread src/main/scala/rules/MoreCompleteExhaleSupporter.scala Outdated
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