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: complete BitVec.[getMsbD|getLsbD|msb] for shifts #5604

Merged
merged 33 commits into from
Oct 13, 2024

Conversation

luisacicolini
Copy link
Contributor

No description provided.

@luisacicolini luisacicolini marked this pull request as draft October 3, 2024 08:34
@luisacicolini luisacicolini changed the title feat: add BitVec.(getMsbD_sshiftRight, getLsbD_sshiftRight', getMsbD_sshiftRight', getMsbD_ushiftRight, msb_shiftLeft, msb_ushiftRight, msb_sshiftRight') and rename sshiftRight_msb_eq_msb to msb_sshiftRight feat: add BitVec.(getMsbD_sshiftRight, getLsbD_sshiftRight', getMsbD_sshiftRight', getMsbD_ushiftRight, msb_shiftLeft, msb_ushiftRight, msb_sshiftRight') and deprecated sshiftRight_msb_eq_msb to msb_sshiftRight Oct 3, 2024
@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 Oct 3, 2024
@leanprover-community-bot
Copy link
Collaborator

leanprover-community-bot commented Oct 3, 2024

Mathlib CI status (docs):

  • ❗ Mathlib CI can not be attempted yet, as the nightly-testing-2024-10-01 tag does not exist there yet. We will retry when you push more commits. If you rebase your branch onto nightly-with-mathlib, Mathlib CI should run now. (2024-10-03 09:04:31)
  • ❗ Batteries/Mathlib CI will not be attempted unless your PR branches off the nightly-with-mathlib branch. Try git rebase 0178f2b70df112a916efaaf8acede9ccea12b950 --onto a4fda010f3d44c02393d5afd5cf05509989daf63. (2024-10-05 21:59:30)
  • ❗ Batteries/Mathlib CI will not be attempted unless your PR branches off the nightly-with-mathlib branch. Try git rebase ec5f206d8036b08c556eb7809a038936f838cbd0 --onto 9dac514c2fc80d3f60076314ad30a993a7b2c22f. (2024-10-07 15:45:18)

@luisacicolini luisacicolini marked this pull request as ready for review October 3, 2024 09:04
@tobiasgrosser
Copy link
Contributor

tobiasgrosser commented Oct 3, 2024

Also, I feel you can have a slightly shorter title, e.g., feat: complete BitVec.[getMsbD|getLsbD|msb] for shifts

@luisacicolini luisacicolini changed the title feat: add BitVec.(getMsbD_sshiftRight, getLsbD_sshiftRight', getMsbD_sshiftRight', getMsbD_ushiftRight, msb_shiftLeft, msb_ushiftRight, msb_sshiftRight') and deprecated sshiftRight_msb_eq_msb to msb_sshiftRight feat: complete BitVec.[getMsbD|getLsbD|msb] for shifts Oct 3, 2024
Comment on lines 1352 to 1353
all_goals (simp [h, h₁, h₂, h₃, h₄]; try congr; try omega)
simp_all
Copy link
Contributor

Choose a reason for hiding this comment

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

I am afraid this simp is non-terminal, so you may want to turn it into a simp only. You can try to be strategic and only include the minimal set of theorems needed to get it through.

Also, do you really need a simp_all at the end?

@luisacicolini luisacicolini marked this pull request as draft October 3, 2024 12:57
@luisacicolini
Copy link
Contributor Author

luisacicolini commented Oct 3, 2024

I am having some trouble refactoring the proofs of getMsbD_sshiftRight, getMsbD_sshiftRight' and getMsbD_ushiftRight to use simp only when non terminal. getMsbD_sshiftRight_exp and getMsbD_ushiftRight_exp are my attempts at this, which however made the proofs quite a lot longer and less elegant. I am wondering whether there are any proving strategies that I did not think about and would make these proofs better.

@luisacicolini luisacicolini marked this pull request as ready for review October 3, 2024 16:56
by_cases h : i < w
<;> by_cases h₁ : i < n
<;> by_cases h₂ : i - n < w
all_goals (simp only [h, decide_False, ushiftRight_eq, getLsbD_ushiftRight, Bool.false_and, h₁,
Copy link
Contributor

Choose a reason for hiding this comment

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

This looks fairly aggressive, and wonder if there are higher-level lemmas to be extracted here.

Comment on lines 1176 to 1183
theorem getMsbD_ushiftRight {w} {x : BitVec w} {i n : Nat} :
getMsbD (x.ushiftRight n) i = (decide (i < w) && if i < n then false else getMsbD x (i - n)) := by
simp only [getMsbD, ushiftRight_eq, getLsbD_ushiftRight, Bool.if_false_left]
by_cases h : i < w
<;> by_cases h₁ : i < n
<;> by_cases h₂ : i - n < w
<;> simp (discharger := omega) [h, h₁, h₂]
congr; omega
Copy link
Contributor

@tobiasgrosser tobiasgrosser Oct 3, 2024

Choose a reason for hiding this comment

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

Suggested change
theorem getMsbD_ushiftRight {w} {x : BitVec w} {i n : Nat} :
getMsbD (x.ushiftRight n) i = (decide (i < w) && if i < n then false else getMsbD x (i - n)) := by
simp only [getMsbD, ushiftRight_eq, getLsbD_ushiftRight, Bool.if_false_left]
by_cases h : i < w
<;> by_cases h₁ : i < n
<;> by_cases h₂ : i - n < w
<;> simp (discharger := omega) [h, h₁, h₂]
congr; omega
theorem getMsbD_ushiftRight {x : BitVec w} {i n : Nat} :
getMsbD (x.ushiftRight n) i = (decide (i < w) && if i < n then false else getMsbD x (i - n)) := by
simp only [getMsbD, Bool.if_false_left]
by_cases h : i < n
· simp [getLsbD_ge, show w ≤ (n + (w - 1 - i)) by omega]
omega
· by_cases h₁ : i < w <;> by_cases h₂ : i - n < w
· simp only [h, , h₁, h₂, ushiftRight_eq, getLsbD_ushiftRight]
congr
omega
all_goals (simp [h, h₁, h₂]; try omega)

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe you can refactor the other cases in similar ways?

@luisacicolini
Copy link
Contributor Author

I changed the proof getMsbD_sshiftRight and it seems to me slightly better (if I understood correctly the rationale behind simp and simp only - CC @hargoniX). I left the two other versions as well (getMsbD_sshiftRight' and getMsbD_sshiftRight'') in case either of the two is preferrable.

@luisacicolini luisacicolini marked this pull request as draft October 5, 2024 21:58
@luisacicolini luisacicolini marked this pull request as ready for review October 7, 2024 06:44
@hargoniX hargoniX added this pull request to the merge queue Oct 13, 2024
Merged via the queue into leanprover:master with commit 47e0430 Oct 13, 2024
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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