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

eradicating LittleEndian.combine #44

Open
andres-erbsen opened this issue Sep 30, 2021 · 2 comments
Open

eradicating LittleEndian.combine #44

andres-erbsen opened this issue Sep 30, 2021 · 2 comments

Comments

@andres-erbsen
Copy link
Contributor

I have the feeling that using tuples in that file was an unfortunate choice and benefits almost nothing; getting rid of it would be a clear improvement. @samuelgruetter do you agree? If so, plan:

  1. define LittleEndian.combine and LittleEndian.split in terms of LittleEndianList.combine and LittleEndianList.split
  2. prove the lemmas we currently have about them? this might actually be more annoying than the next point
  3. change the callers to use LittleEndianList directly
  4. atomic rename LittleEndianList -> LittleEndian across all repositories (but we get most of the benefit even if we don't do this)

Thoughts?

supersedes #34

@andres-erbsen
Copy link
Contributor Author

After some discussion: it may make sense to update callers in two phases, first everything above bedrock2.exec and then everything below, duplicating bedrock2.exec in the interim (with a proof that the two versions are equivalent).

@andres-erbsen
Copy link
Contributor Author

bedrock2, rupicola, and fiat-crypto primarily use LittleEndianList after https://github.com/mit-plv/fiat-crypto/pull/1085/files ; there is still some LittleEndian in the bedrock2 compiler and more in riscv-coq.

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

No branches or pull requests

1 participant