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

Modify side_condition results to be only false or true in Proof Hints #1023

Merged
merged 14 commits into from
Apr 10, 2024

Conversation

Robertorosmaninho
Copy link
Collaborator

@Robertorosmaninho Robertorosmaninho commented Apr 8, 2024

This modification was proposed by the LLVM Backend and Math Proof Generation Teams from Pi2 to optimize the Hints generated by the llvm-krun as constructing and parsing the boolean values of the side conditions takes some time. Then, we choose to simplify the way we're generating these hints.

Replacing `emit_serialize_term` with `emit_bool_term` in `side_condition_event_post`
Introducing `result_` in `llvm_side_condition_end_event`
Introducing function `parse_bool`
Deleting the check of  `kore_end_sentinel` in `ProofTraceParser.h`
Copy link
Contributor

@Baltoli Baltoli left a comment

Choose a reason for hiding this comment

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

Some questions and changes. Please also make sure that you accompany any PRs you open with a body that explains the context for the changes. This is important for reviewing now, as well as in the future when we look back at historical changes to try and understand them.

docs/proof-trace.md Show resolved Hide resolved
include/kllvm/binary/ProofTraceParser.h Outdated Show resolved Hide resolved
include/kllvm/binary/ProofTraceParser.h Outdated Show resolved Hide resolved
include/kllvm/codegen/ProofEvent.h Outdated Show resolved Hide resolved
lib/codegen/ProofEvent.cpp Outdated Show resolved Hide resolved
runtime/util/ConfigurationSerializer.cpp Outdated Show resolved Hide resolved
Copy link
Collaborator

@theo25 theo25 left a comment

Choose a reason for hiding this comment

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

This looks good to me but I think you should wait for Bruce to review your latest changes before you merge.

Copy link
Contributor

@Baltoli Baltoli left a comment

Choose a reason for hiding this comment

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

One minor comment, otherwise looks good!

include/kllvm/binary/ProofTraceParser.h Outdated Show resolved Hide resolved
Co-authored-by: Bruce Collie <brucecollie82@gmail.com>
@rv-jenkins rv-jenkins merged commit 4f1a261 into master Apr 10, 2024
8 checks passed
@rv-jenkins rv-jenkins deleted the side-condition-binary-trace branch April 10, 2024 16:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants