-
Notifications
You must be signed in to change notification settings - Fork 0
WIP: Fix Deep Recursion by Using Bitshift Repr #71
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
WIP: Fix Deep Recursion by Using Bitshift Repr #71
Conversation
|
You can merge this if you want to. Don't worry about me. |
…ccinctlabs/sp1-lean into dtumad/deep-recursion-fixes
…into gzgz/load
| simp_all | ||
|
|
||
| constructor <;> intro sgn <;> (repeat rw [this]) <;> clear this <;> simp_all | ||
| stop |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Something is going wrong here. split_ands hangs, and then when I override that with [ skip; constructor; constructor; constructor; constructor ] the following split_ifs` hangs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh sorry, I was only searching for sorry to fix (no pun intended). Will look at this and why they are stalling
| intro h_op1_isU64 h_op2_isU64 | ||
| have ha' := Word.lt_cases_of_isU64 h_op1_isU64 | ||
| have hb' := Word.lt_cases_of_isU64 h_op2_isU64 | ||
| cases op <;> simp [execute_RTYPEW_pure] <;> congr |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This congr hangs as well, it didn't before.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems maybe related to the fact that ofNat was causing deep recursion issues when it wasn't already unfolded (and it isn't @[reducible]). Hadn't looked very closely when just refactoring the proof.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Possibly. I'm not sure how that would come up in this proof, given that the first change is here...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My thought was that one side contains e.g. a x * 2^16 and the other a x <<< 16, and Lean is both somehow seeing that, and trying to unify them in congruence which is difficult. I'll investigate more when I get home. This is definitely not the getElem change (given the location of the macro), and none of the bit-shifts should be in scope IIRC (will check when I'm back home)
…into gzgz/load
|
This branch has grown too large and still isn't fully finished. Closing in favor of cherry picking individual commits to merge incrementally |
Using e.g.
2^16in place of2 <<< 16is what is causing most of the "(kernel) deep recursion detected" issues. This PR swaps to that form, and also tries to just unfoldWord.toNatless in general. This happens in the kernel after Lean has unfolded the function calls, so just no opening upWord.toNatisn't enough, we need to actually change this.