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

SIMD: add a model #815

Open
wants to merge 11 commits into
base: main
Choose a base branch
from
Open

SIMD: add a model #815

wants to merge 11 commits into from

Conversation

W95Psp
Copy link
Contributor

@W95Psp W95Psp commented Feb 12, 2025

This PR inits a new crate, minicore.
For now, this covers only a small subset of core::arch::x86_64 and core::arch::x86.

This PR adds:

  • abstractions for bit and bitvecs
  • some specifications written in Rust (mainly a backport of what can be found in F* in fstar-helpers)
  • tests

@W95Psp W95Psp requested a review from a team as a code owner February 12, 2025 08:15
Copy link
Member

@franziskuskiefer franziskuskiefer left a comment

Choose a reason for hiding this comment

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

Thanks. This looks like it's quite ready though.

Let's move this into a separate directory for proofs (where we can move the other proof related parts too). Having this on the top level clutters things even more. We'll move crates as well soon.

Also, the high level description needs to explain what this crate is doing and how.

minicore/src/abstractions/bit.rs Outdated Show resolved Hide resolved
minicore/src/abstractions/bit.rs Outdated Show resolved Hide resolved
minicore/src/abstractions/bit.rs Outdated Show resolved Hide resolved
minicore/src/abstractions/bit.rs Outdated Show resolved Hide resolved
minicore/src/abstractions/bit.rs Outdated Show resolved Hide resolved
#[allow(non_camel_case_types)]
type __m128i = BitVec<128>;

pub fn _mm_storeu_si128(output: *mut __m128i, a: __m128i) {
Copy link
Member

Choose a reason for hiding this comment

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

All of these functions should get a description of what they are doing when we want to use this as a source of truth.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can copy-paste docs from rust core or intel, shall I do that?

Copy link
Member

Choose a reason for hiding this comment

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

The rust docs are pretty limited and sometimes wrong I think. But the the gist from the intel docs with a link would be good.

minicore/src/arch/x86.rs Outdated Show resolved Hide resolved
minicore/src/arch/x86.rs Outdated Show resolved Hide resolved
minicore/src/arch/x86.rs Outdated Show resolved Hide resolved
minicore/src/arch/x86.rs Outdated Show resolved Hide resolved
@franziskuskiefer franziskuskiefer added the waiting-on-author Status: This is awaiting some action from the author. label Feb 12, 2025
Co-authored-by: Franziskus Kiefer <[email protected]>
@W95Psp
Copy link
Contributor Author

W95Psp commented Feb 17, 2025

Thanks for the review.
Shall I move that crate to fstar-helpers then?

@franziskuskiefer
Copy link
Member

Shall I move that crate to fstar-helpers then?

Yeah, I guess that should be a more general verification folder or something like that at some point. But we can start using it for that and rename later. There is the formal_verification directory that has all the stuff on other projects that we could also repurpose. But fstar-helpers sounds the easiest for now.

@W95Psp
Copy link
Contributor Author

W95Psp commented Feb 19, 2025

I did the changes, but I'm not sure about the docs of the intrinsics actually.
Those are specifications: so adding the imperative style spec from intel seems a bit odd.

I wonder if we should not instead expose the body of the function as docstring, somehow.
Anyway, that's not clear, I think we should think about a good style before commiting to copy/pasting 10 snippets of code from the intel doc.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
waiting-on-author Status: This is awaiting some action from the author.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants