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

feat: add BitVec.(getElem_umod_of_lt, getElem_umod, getLsbD_umod, getMsbD_umod) #6795

Draft
wants to merge 9 commits into
base: master
Choose a base branch
from

Conversation

luisacicolini
Copy link
Contributor

@luisacicolini luisacicolini commented Jan 27, 2025

This PR adds theorems BitVec.(getElem_umod_of_lt, getElem_umod, getLsbD_umod, getMsbD_umod). For the defiition of these theorems we rely on divRec, excluding the case where d=0#w, which is treated separately because there is no infrastructure to reason about this case within divRec. In particular, our implementation follows the mathlib standard where division by 0 yields 0, while in SMTLIB this yields allOnes.

Co-authored by @bollu.

@luisacicolini luisacicolini changed the title feat: add 'BitVec.(getElem_umod_of_lt, getElem_umod, getLsbD_umod, getMsbD_umod) and support BitVec.le_zero_eq_zero` feat: add BitVec.(getElem_umod_of_lt, getElem_umod, getLsbD_umod, getMsbD_umod) and support BitVec.le_zero_eq_zero Jan 27, 2025
@github-actions github-actions bot added the toolchain-available A toolchain is available for this PR, at leanprover/lean4-pr-releases:pr-release-NNNN label Jan 27, 2025
@leanprover-community-bot
Copy link
Collaborator

leanprover-community-bot commented Jan 27, 2025

Mathlib CI status (docs):

  • ❗ Batteries/Mathlib CI will not be attempted unless your PR branches off the nightly-with-mathlib branch. Try git rebase 4ca98dcca2b0995dddff444cfef1f3ccc89c7b12 --onto 69a73a18fbfa1fc045bfbf1c4cf93b155d4c9387. (2025-01-27 15:48:51)
  • ❗ Batteries/Mathlib CI will not be attempted unless your PR branches off the nightly-with-mathlib branch. Try git rebase c7c1e091c9f07ae6f8e8ff7246eb7650e2740dcb --onto 20c616503abe5ce4253c56dbcd7766a91c675ba0. (2025-01-28 11:10:40)
  • ❗ Batteries/Mathlib CI will not be attempted unless your PR branches off the nightly-with-mathlib branch. Try git rebase c7c1e091c9f07ae6f8e8ff7246eb7650e2740dcb --onto e922edfc218090ab2e54092336c67fe3b970dfc2. (2025-01-30 16:34:13)

@tobiasgrosser
Copy link
Contributor

Thank you. Could you add 1-2 sentences of motivation of why you choose these right-hand-sides that you see here.

…tMsbD_umod)` and support `BitVec.le_zero_eq_zero`

This PR adds theorems `BitVec.(getElem_umod_of_lt, getElem_umod, getLsbD_umod, getMsbD_umod)` and support theorem `BitVec.le_zero_eq_zero`. For the defiition of these theorems we rely on `divRec`, excluding the case where `d=0#w`, which is treated separately because there is no infrastructure to reason about this case within `divRec`. In particular, our implementation follows the mathlib standard [where division by 0 yields 0](https://github.com/leanprover/lean4/blob/c7c1e091c9f07ae6f8e8ff7246eb7650e2740dcb/src/Init/Data/BitVec/Basic.lean#L217), while in [SMTLIB this yields `allOnes`](https://github.com/leanprover/lean4/blob/c7c1e091c9f07ae6f8e8ff7246eb7650e2740dcb/src/Init/Data/BitVec/Basic.lean#L237).

Co-authored by @bollu.
@luisacicolini luisacicolini changed the title feat: add BitVec.(getElem_umod_of_lt, getElem_umod, getLsbD_umod, getMsbD_umod) and support BitVec.le_zero_eq_zero feat: add BitVec.(getElem_umod_of_lt, getElem_umod, getLsbD_umod, getMsbD_umod) Jan 28, 2025
Copy link
Contributor

@bollu bollu left a comment

Choose a reason for hiding this comment

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

LGTM, modulo my comments about proof reuse. It would be nice to not repeat the same proof, as that's a bit brittle.

@luisacicolini luisacicolini requested a review from bollu January 29, 2025 15:07
Copy link
Contributor

@bollu bollu left a comment

Choose a reason for hiding this comment

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

LGTM. These straightforwardly use the bitblasting theorems as getElem/getLsbD/getMsb.

Co-authored-by: Siddharth <[email protected]>
@luisacicolini
Copy link
Contributor Author

awaiting-review

@luisacicolini luisacicolini marked this pull request as ready for review January 30, 2025 15:55
@luisacicolini luisacicolini requested a review from kim-em as a code owner January 30, 2025 15:55
@github-actions github-actions bot added the awaiting-review Waiting for someone to review the PR label Jan 30, 2025
@luisacicolini
Copy link
Contributor Author

changelog-library

@github-actions github-actions bot added the changelog-library Library label Jan 30, 2025
@luisacicolini luisacicolini marked this pull request as draft January 30, 2025 16:01
src/Init/Data/BitVec/Bitblast.lean Outdated Show resolved Hide resolved
src/Init/Data/BitVec/Bitblast.lean Show resolved Hide resolved
Comment on lines 1243 to 1246
· have := (BitVec.not_le (x := d) (y := 0#w)).mp
simp only [le_zero_iff] at this
rw [← BitVec.umod_eq_divRec (by simp [hd, this])]
simp [hd]
Copy link
Contributor

Choose a reason for hiding this comment

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

Why don't we reuse getElem_umod_of_(lt/pos) here? That lemmas seems to describe exactly this case, no?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

so it's actually interesting, using that theorem in the proof seems to only make the proof longer. I guess we don't need getElem_umod_of_pos in the first place.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting-review Waiting for someone to review the PR changelog-library Library toolchain-available A toolchain is available for this PR, at leanprover/lean4-pr-releases:pr-release-NNNN
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants